add paranoia checks on setuid/setgid

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

4 files changed, 31 insertions, 37 deletions

diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp
index 22566a73..e4c007a9 100644
--- a/lib/Plugins/CCpp.cpp
+++ b/lib/Plugins/CCpp.cpp
@@ -101,13 +101,6 @@ static int ExecVP(char **pArgs, uid_t uid, string& pOutput)
     int pipeout[2];
     pid_t child;
 
-    gid_t gid = uid;
-    struct passwd* pw = getpwuid(uid);
-    if (pw)
-    {
-        gid = pw->pw_gid;
-    }
-
     xpipe(pipeout);
     child = fork();
     if (child == -1)
@@ -124,9 +117,11 @@ static int ExecVP(char **pArgs, uid_t uid, string& pOutput)
         /* Not a good idea, we won't see any error messages */
         /* close(STDERR_FILENO); */
 
+        struct passwd* pw = getpwuid(uid);
+        gid_t gid = pw ? pw->pw_gid : uid;
         setgroups(1, &gid);
-        setregid(gid, gid);
-        setreuid(uid, uid);
+        xsetregid(gid, gid);
+        xsetreuid(uid, uid);
         setsid();
 
         /* Nuke everything which may make setlocale() switch to non-POSIX locale:
@@ -558,12 +553,6 @@ string CAnalyzerCCpp::GetGlobalUUID(const char *pDebugDumpDir)
     args[4] = (char*)backtrace_path.c_str();
     args[5] = NULL;
 
-    uid_t uid = atoi(uid_str.c_str());
-    gid_t gid = uid;
-    struct passwd* pw = getpwuid(uid);
-    if (pw)
-        gid = pw->pw_gid;
-
     int pipeout[2];
     xpipe(pipeout); /* stdout of abrt-backtrace */
     pid_t child = fork();
@@ -576,12 +565,13 @@ string CAnalyzerCCpp::GetGlobalUUID(const char *pDebugDumpDir)
         xmove_fd(pipeout[1], STDOUT_FILENO);
         close(pipeout[0]); /* read side of the pipe */
 
-        /* abrt-backtrace is executed under the user's
-           uid and gid. */
+        /* abrt-backtrace is executed under the user's uid and gid. */
+        uid_t uid = atoi(uid_str.c_str());
+        struct passwd* pw = getpwuid(uid);
+        gid_t gid = pw ? pw->pw_gid : uid;
         setgroups(1, &gid);
-        setregid(gid, gid);
-        setreuid(uid, uid);
-        setsid();
+        xsetregid(gid, gid);
+        xsetreuid(uid, uid);
 
         execvp(args[0], args);
         VERB1 perror_msg("Can't execute '%s'", args[0]);
@@ -632,8 +622,7 @@ static bool DebuginfoCheckPolkit(int uid)
     if (child_pid == 0)
     {
         //child
-        if (setuid(uid))
-            exit(1); //paranoia
+        xsetreuid(uid, uid);
         PolkitResult result = polkit_check_authorization(getpid(),
                  "org.fedoraproject.abrt.install-debuginfos");
         exit(result != PolkitYes); //exit 1 (failure) if not allowed
diff --git a/lib/Plugins/Firefox.cpp b/lib/Plugins/Firefox.cpp
index 9c102041..d9e6153a 100644
--- a/lib/Plugins/Firefox.cpp
+++ b/lib/Plugins/Firefox.cpp
@@ -97,12 +97,6 @@ static pid_t ExecVP(char** pArgs, uid_t uid, std::string& pOutput)
     int pipeout[2];
     pid_t child;
 
-    struct passwd* pw = getpwuid(uid);
-    if (!pw)
-    {
-        throw CABRTException(EXCEP_PLUGIN, "%s: can't get GID for UID", __func__);
-    }
-
     xpipe(pipeout);
     child = fork();
     if (child == -1)
@@ -119,10 +113,11 @@ static pid_t ExecVP(char** pArgs, uid_t uid, std::string& pOutput)
         /* Not a good idea, we won't see any error messages */
         /* close(STDERR_FILENO); */
 
-        setgroups(1, &pw->pw_gid);
-        setregid(pw->pw_gid, pw->pw_gid);
-        setreuid(uid, uid);
-        setsid();
+        struct passwd* pw = getpwuid(uid);
+        gid_t gid = pw ? pw->pw_gid : uid;
+        setgroups(1, &gid);
+        xsetregid(gid, gid);
+        xsetreuid(uid, uid);
 
         /* Nuke everything which may make setlocale() switch to non-POSIX locale:
          * we need to avoid having gdb output in some obscure language.
@@ -856,8 +851,7 @@ static bool DebuginfoCheckPolkit(int uid)
     if (child_pid == 0)
     {
         //child
-        if (setuid(uid))
-            exit(1); //paranoia
+        xsetreuid(uid, uid);
         PolkitResult result = polkit_check_authorization(getpid(),
                  "org.fedoraproject.abrt.install-debuginfos");
         exit(result != PolkitYes); //exit 1 (failure) if not allowed
diff --git a/lib/Plugins/Mailx.cpp b/lib/Plugins/Mailx.cpp
index 26b6ec41..b06edebb 100644
--- a/lib/Plugins/Mailx.cpp
+++ b/lib/Plugins/Mailx.cpp
@@ -57,9 +57,8 @@ static void exec_and_feed_input(uid_t uid, const char* pText, char **pArgs)
         struct passwd* pw = getpwuid(uid);
         gid_t gid = pw ? pw->pw_gid : uid;
         setgroups(1, &gid);
-        setregid(gid, gid);
-        setreuid(uid, uid);
-        setsid(); /* why? I propose removing this */
+        xsetregid(gid, gid);
+        xsetreuid(uid, uid);
 
         execvp(pArgs[0], pArgs);
         exit(1); /* exec failed */
diff --git a/lib/Utils/xfuncs.cpp b/lib/Utils/xfuncs.cpp
index 0e57639b..8621b5f4 100644
--- a/lib/Utils/xfuncs.cpp
+++ b/lib/Utils/xfuncs.cpp
@@ -362,6 +362,18 @@ bool string_to_bool(const char *s)
 	return false;
 }
 
+void xsetreuid(uid_t ruid, uid_t euid)
+{
+	if (setreuid(ruid, euid) != 0)
+		perror_msg_and_die("can't set %cid %d", 'u', (int)ruid);
+}
+
+void xsetregid(gid_t rgid, uid_t egid)
+{
+	if (setregid(rgid, egid) != 0)
+		perror_msg_and_die("can't set %cid %d", 'g', (int)rgid);
+}
+
 uid_t getuidbyname(const char* login)
 {
     struct passwd* pwd = getpwnam(login);