Security review: make privilege dropping-restoring optional.

This is/was necessary only on macOS, because you needed root in order
to read the process list. This was never necessary on Linux, and
it also raises security concerns, so now it needs to be enabled
explicitly at build time.

7 files changed, 92 insertions, 21 deletions

diff --git a/CRT.c b/CRT.c
index 0060cbfc..237e30ef 100644
--- a/CRT.c
+++ b/CRT.c
@@ -5,6 +5,7 @@ Released under the GNU GPL, see the COPYING file
 in the source distribution for its full text.
 */
 
+#include "config.h"
 #include "CRT.h"
 
 #include "StringUtils.h"
@@ -17,6 +18,10 @@ in the source distribution for its full text.
 #include <string.h>
 #include <locale.h>
 #include <langinfo.h>
+#if HAVE_SETUID_ENABLED
+#include <unistd.h>
+#include <sys/types.h>
+#endif
 
 #define ColorIndex(i,j) ((7-i)*8+j)
 
@@ -545,6 +550,48 @@ static void CRT_handleSIGTERM(int sgn) {
    exit(0);
 }
 
+#if HAVE_SETUID_ENABLED
+
+static int CRT_euid = -1;
+
+static int CRT_egid = -1;
+
+#define DIE(msg) do { CRT_done(); fprintf(stderr, msg); exit(1); } while(0)
+
+void CRT_dropPrivileges() {
+   CRT_egid = getegid();
+   CRT_euid = geteuid();
+   if (setegid(getgid()) == -1) {
+      DIE("Fatal error: failed dropping group privileges.\n");
+   }
+   if (seteuid(getuid()) == -1) {
+      DIE("Fatal error: failed dropping user privileges.\n");
+   }
+}
+
+void CRT_restorePrivileges() {
+   if (CRT_egid == -1 || CRT_euid == -1) {
+      DIE("Fatal error: internal inconsistency.\n");
+   }
+   if (setegid(CRT_egid) == -1) {
+      DIE("Fatal error: failed restoring group privileges.\n");
+   }
+   if (seteuid(CRT_euid) == -1) {
+      DIE("Fatal error: failed restoring user privileges.\n");
+   }
+}
+
+#else
+
+/* Turn setuid operations into NOPs */
+
+#ifndef CRT_dropPrivileges
+#define CRT_dropPrivileges()
+#define CRT_restorePrivileges()
+#endif
+
+#endif
+
 // TODO: pass an instance of Settings instead.
 
 void CRT_init(int delay, int colorScheme) {
diff --git a/CRT.h b/CRT.h
index 6391e890..590fff6e 100644
--- a/CRT.h
+++ b/CRT.h
@@ -9,6 +9,9 @@ Released under the GNU GPL, see the COPYING file
 in the source distribution for its full text.
 */
 
+#if HAVE_SETUID_ENABLED
+#endif
+
 #define ColorIndex(i,j) ((7-i)*8+j)
 
 #define ColorPair(i,j) COLOR_PAIR(ColorIndex(i,j))
@@ -154,6 +157,25 @@ extern int CRT_colorScheme;
 
 void *backtraceArray[128];
 
+#if HAVE_SETUID_ENABLED
+
+#define DIE(msg) do { CRT_done(); fprintf(stderr, msg); exit(1); } while(0)
+
+void CRT_dropPrivileges();
+
+void CRT_restorePrivileges();
+
+#else
+
+/* Turn setuid operations into NOPs */
+
+#ifndef CRT_dropPrivileges
+#define CRT_dropPrivileges()
+#define CRT_restorePrivileges()
+#endif
+
+#endif
+
 // TODO: pass an instance of Settings instead.
 
 void CRT_init(int delay, int colorScheme);
diff --git a/EnvScreen.c b/EnvScreen.c
index 06f329c1..855023a4 100644
--- a/EnvScreen.c
+++ b/EnvScreen.c
@@ -48,10 +48,9 @@ void EnvScreen_scan(InfoScreen* this) {
 
    Panel_prune(panel);
 
-   uid_t euid = geteuid();
-   (void) seteuid(getuid());
-   char *env = Platform_getProcessEnv(this->process->pid);
-   (void) seteuid(euid);
+   CRT_dropPrivileges();
+   char* env = Platform_getProcessEnv(this->process->pid);
+   CRT_restorePrivileges();
    if (env) {
       for (char *p = env; *p; p = strrchr(p, 0)+1)
          InfoScreen_addLine(this, p);
diff --git a/Process.c b/Process.c
index f31cb705..b02761c3 100644
--- a/Process.c
+++ b/Process.c
@@ -522,11 +522,10 @@ void Process_toggleTag(Process* this) {
 }
 
 bool Process_setPriority(Process* this, int priority) {
-   uid_t euid = geteuid();
-   (void) seteuid(getuid());
+   CRT_dropPrivileges();
    int old_prio = getpriority(PRIO_PROCESS, this->pid);
    int err = setpriority(PRIO_PROCESS, this->pid, priority);
-   (void) seteuid(euid);
+   CRT_restorePrivileges();
    if (err == 0 && old_prio != getpriority(PRIO_PROCESS, this->pid)) {
       this->nice = priority;
    }
@@ -538,10 +537,9 @@ bool Process_changePriorityBy(Process* this, size_t delta) {
 }
 
 void Process_sendSignal(Process* this, size_t sgn) {
-   uid_t euid = geteuid();
-   (void) seteuid(getuid());
+   CRT_dropPrivileges();
    kill(this->pid, (int) sgn);
-   (void) seteuid(euid);
+   CRT_restorePrivileges();
 }
 
 long Process_pidCompare(const void* v1, const void* v2) {
diff --git a/Settings.c b/Settings.c
index 0e296c10..6a7a9b0a 100644
--- a/Settings.c
+++ b/Settings.c
@@ -165,11 +165,10 @@ static void readFields(ProcessField* fields, int* flags, const char* line) {
 
 static bool Settings_read(Settings* this, const char* fileName) {
    FILE* fd;
-   uid_t euid = geteuid();
-
-   (void) seteuid(getuid());
+   
+   CRT_dropPrivileges();
    fd = fopen(fileName, "r");
-   (void) seteuid(euid);
+   CRT_restorePrivileges();
    if (!fd)
       return false;
    
@@ -278,11 +277,11 @@ static void writeMeterModes(Settings* this, FILE* fd, int column) {
 
 bool Settings_write(Settings* this) {
    FILE* fd;
-   uid_t euid = geteuid();
 
-   (void) seteuid(getuid());
+   CRT_dropPrivileges();
    fd = fopen(this->filename, "w");
-   (void) seteuid(euid);
+   CRT_restorePrivileges();
+
    if (fd == NULL) {
       return false;
    }
@@ -368,8 +367,8 @@ Settings* Settings_new(int cpuCount) {
          htopDir = String_cat(home, "/.config/htop");
       }
       legacyDotfile = String_cat(home, "/.htoprc");
-      uid_t euid = geteuid();
-      (void) seteuid(getuid());
+      
+      CRT_dropPrivileges();
       (void) mkdir(configDir, 0700);
       (void) mkdir(htopDir, 0700);
       free(htopDir);
@@ -382,7 +381,7 @@ Settings* Settings_new(int cpuCount) {
          free(legacyDotfile);
          legacyDotfile = NULL;
       }
-      (void) seteuid(euid);
+      CRT_restorePrivileges();
    }
    this->colorScheme = 0;
    this->changed = false;
diff --git a/TraceScreen.c b/TraceScreen.c
index 6bb2e041..318b9091 100644
--- a/TraceScreen.c
+++ b/TraceScreen.c
@@ -96,7 +96,7 @@ bool TraceScreen_forkTracer(TraceScreen* this) {
    this->child = fork();
    if (this->child == -1) return false;
    if (this->child == 0) {
-      (void) seteuid(getuid());
+      CRT_dropPrivileges();
       dup2(this->fdpair[1], STDERR_FILENO);
       int ok = fcntl(this->fdpair[1], F_SETFL, O_NONBLOCK);
       if (ok != -1) {
diff --git a/configure.ac b/configure.ac
index 225c6d85..9931f3a4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -253,6 +253,12 @@ then
    AC_CHECK_HEADERS([hwloc.h],[:], [missing_headers="$missing_headers $ac_header"])
 fi
 
+AC_ARG_ENABLE(setuid, [AS_HELP_STRING([--enable-setuid], [enable setuid support for platforms that need it])],, enable_setuid="no")
+if test "x$enable_setuid" = xyes
+then
+   AC_DEFINE(HAVE_SETUID_ENABLED, 1, [Define if setuid support should be enabled.])
+fi
+
 # Bail out on errors.
 # ----------------------------------------------------------------------
 if test ! -z "$missing_libraries"; then