Jens Petersen
6 years ago
parent
e55a957f1b
commit
bbb20669df
@ -0,0 +1,92 @@
|
||||
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>
|
||||
@ -0,0 +1,24 @@
|
||||
From 73ea41b3622e2e578d928f7513941aac9d873279 Mon Sep 17 00:00:00 2001
|
||||
From: Ben Gamari <ben@smart-cactus.org>
|
||||
Date: Mon, 1 Jul 2019 11:02:45 -0400
|
||||
Subject: [PATCH] Fix incorrect case fallthrough
|
||||
|
||||
The error message lookup logic would fallthrough from the
|
||||
forkSetuidFailed case into the default case, meaning that the error
|
||||
message of the former would never be returned.
|
||||
---
|
||||
cbits/runProcess.c | 1 +
|
||||
1 file changed, 1 insertion(+)
|
||||
|
||||
diff --git a/cbits/runProcess.c b/cbits/runProcess.c
|
||||
index c621158..10794bc 100644
|
||||
--- a/cbits/runProcess.c
|
||||
+++ b/cbits/runProcess.c
|
||||
@@ -329,6 +329,7 @@ runInteractiveProcess (char *const args[],
|
||||
break;
|
||||
case forkSetuidFailed:
|
||||
*failed_doing = "runInteractiveProcess: setuid";
|
||||
+ break;
|
||||
default:
|
||||
*failed_doing = "runInteractiveProcess: unknown";
|
||||
break;
|
||||
Loading…
Reference in new issue