Blob Blame Raw
From 3e0812fe9d3f4712638a1c4c49bf2b2a7dc4311b Mon Sep 17 00:00:00 2001
From: Ben Gamari <ben@smart-cactus.org>
Date: Mon, 1 Jul 2019 11:03:33 -0400
Subject: [PATCH] Call initgroups before setuid

Previously we would fail to call initgroups before setuid'ing. This
meant that our groups we not be reset to reflect those our new user
belongs to. Fix this.
---
 cbits/runProcess.c   | 32 +++++++++++++++++++++++++++++---
 include/runProcess.h |  4 ++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/cbits/runProcess.c b/cbits/runProcess.c
index 10794bc..84d5fd4 100644
--- a/cbits/runProcess.c
+++ b/cbits/runProcess.c
@@ -33,6 +33,10 @@ static long max_fd = 0;
 extern void blockUserSignals(void);
 extern void unblockUserSignals(void);
 
+// These are arbitrarily chosen -- JP
+#define forkSetgidFailed 124
+#define forkSetuidFailed 125
+
 // See #1593.  The convention for the exit code when
 // exec() fails seems to be 127 (gleened from C's
 // system()), but there's no equivalent convention for
@@ -40,9 +44,8 @@ extern void unblockUserSignals(void);
 #define forkChdirFailed 126
 #define forkExecFailed  127
 
-// These are arbitrarily chosen -- JP
-#define forkSetgidFailed 124
-#define forkSetuidFailed 125
+#define forkGetpwuidFailed 128
+#define forkInitgroupsFailed 129
 
 __attribute__((__noreturn__))
 static void childFailed(int pipe, int failCode) {
@@ -182,6 +185,23 @@ runInteractiveProcess (char *const args[],
         }
 
         if ( childUser) {
+            // Using setuid properly first requires that we initgroups.
+            // However, to do this we must know the username of the user we are
+            // switching to.
+            struct passwd pw;
+            struct passwd *res = NULL;
+            int buf_len = sysconf(_SC_GETPW_R_SIZE_MAX);
+            char *buf = malloc(buf_len);
+            gid_t suppl_gid = childGroup ? *childGroup : getgid();
+            if ( getpwuid_r(*childUser, &pw, buf, buf_len, &res) != 0) {
+                childFailed(forkCommunicationFds[1], forkGetpwuidFailed);
+            }
+            if ( res == NULL ) {
+                childFailed(forkCommunicationFds[1], forkGetpwuidFailed);
+            }
+            if ( initgroups(res->pw_name, suppl_gid) != 0) {
+                childFailed(forkCommunicationFds[1], forkInitgroupsFailed);
+            }
             if ( setuid( *childUser) != 0) {
                 // ERROR
                 childFailed(forkCommunicationFds[1], forkSetuidFailed);
@@ -330,6 +350,12 @@ runInteractiveProcess (char *const args[],
         case forkSetuidFailed:
             *failed_doing = "runInteractiveProcess: setuid";
             break;
+        case forkGetpwuidFailed:
+            *failed_doing = "runInteractiveProcess: getpwuid";
+            break;
+        case forkInitgroupsFailed:
+            *failed_doing = "runInteractiveProcess: initgroups";
+            break;
         default:
             *failed_doing = "runInteractiveProcess: unknown";
             break;
diff --git a/include/runProcess.h b/include/runProcess.h
index 3807389..dff3905 100644
--- a/include/runProcess.h
+++ b/include/runProcess.h
@@ -21,6 +21,10 @@
 
 #include <unistd.h>
 #include <sys/types.h>
+#if !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32))
+#include <pwd.h>
+#include <grp.h>
+#endif
 
 #ifdef HAVE_FCNTL_H
 #include <fcntl.h>