Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sanitizer] Disable writes to log files for binaries in a secure context. #92593

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,11 @@ inline u32 GetNumberOfCPUsCached() {
return NumberOfCPUsCached;
}

// Returns true if the our runtime has special privileges (e.g. setuid) and
// should be treated with caution when dealing with untrusted user input.
// For example, when writing files or executing user-specified binaries.
bool ShouldTreatRuntimeSecurely();

} // namespace __sanitizer

inline void *operator new(__sanitizer::usize size,
Expand Down
2 changes: 2 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_common_nolibc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ void LogMessageOnPrintf(const char *str) {}
void WriteToSyslog(const char *buffer) {}
void Abort() { internal__exit(1); }
bool CreateDir(const char *pathname) { return false; }
// TODO: Revisit using insecure by default approach for no-libc binaries.
bool ShouldTreatRuntimeSecurely() { return false; }
bigb4ng marked this conversation as resolved.
Show resolved Hide resolved
#endif // !SANITIZER_WINDOWS

#if !SANITIZER_WINDOWS && !SANITIZER_APPLE
Expand Down
10 changes: 10 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@ void ReportFile::SetReportPath(const char *path) {
}
}

if (path && ShouldTreatRuntimeSecurely() &&
internal_strcmp(path, "stderr") != 0 &&
internal_strcmp(path, "stdout") != 0) {
Report(
"ERROR: log_path must be 'stderr' or 'stdout' for AT_SECURE and/or "
"setuid binaries, is '%s'\n",
path);
Die();
}

SpinMutexLock l(mu);
if (fd != kStdoutFd && fd != kStderrFd && fd != kInvalidFd)
CloseFile(fd);
Expand Down
13 changes: 13 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
#define MAP_NORESERVE 0
#endif

# if SANITIZER_LINUX
# include "sanitizer_getauxval.h"
# endif

typedef void (*sa_sigaction_t)(int, siginfo_t *, void *);

namespace __sanitizer {
Expand Down Expand Up @@ -518,6 +522,15 @@ bool IsStateDetached(int state) {
return state == PTHREAD_CREATE_DETACHED;
}

bool ShouldTreatRuntimeSecurely() {
# if SANITIZER_LINUX
// So we can use weak reference from sanitizer_getauxval.h
if (&getauxval)
return getauxval(/* AT_SECURE */ 23) != 0;
# endif
return getuid() != geteuid() || getgid() != getegid();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just return false?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for older versions of glibc that don't have getauxval, or other posix OS that don't have it.

}

} // namespace __sanitizer

#endif // SANITIZER_POSIX
4 changes: 4 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,10 @@ void LogFullErrorReport(const char *buffer) {

void InitializePlatformCommonFlags(CommonFlags *cf) {}

// FIXME: implement on this platform.
// Windows analog to such runtime would be auto-elevated binaries.
bool ShouldTreatRuntimeSecurely() { return false; }

} // namespace __sanitizer

#endif // _WIN32
Loading