From 3e0812fe9d3f4712638a1c4c49bf2b2a7dc4311b Mon Sep 17 00:00:00 2001 From: Ben Gamari 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 #include +#if !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32)) +#include +#include +#endif #ifdef HAVE_FCNTL_H #include