baab13
From e721bc775d9270ac8d9d8daf2fe3f83bffe5d761 Mon Sep 17 00:00:00 2001
baab13
From: Jakub Filak <jfilak@redhat.com>
baab13
Date: Wed, 30 Sep 2015 11:50:18 +0200
baab13
Subject: [PATCH] a-a-i-d-to-abrt-cache: make own random temporary directory
baab13
baab13
The set-user-ID wrapper must use own new temporary directory in order to
baab13
avoid security issues with unpacking specially crafted debuginfo
baab13
packages that might be used to create files or symlinks anywhere on the
baab13
file system as the abrt user.
baab13
baab13
Withot the forking code the temporary directory would remain on the
baab13
filesystem in the case where all debuginfo data are already available.
baab13
This is caused by the fact that the underlying libreport functionality
baab13
accepts path to a desired temporary directory and creates it only if
baab13
necessary. Otherwise, the directory is not touched at all.
baab13
baab13
This commit addresses CVE-2015-5273
baab13
Related: #1262252
baab13
baab13
Signed-off-by: Jakub Filak <jfilak@redhat.com>
baab13
---
baab13
 src/plugins/Makefile.am                            |  1 +
baab13
 .../abrt-action-install-debuginfo-to-abrt-cache.c  | 41 +++++++++++++++++++---
baab13
 2 files changed, 38 insertions(+), 4 deletions(-)
baab13
baab13
diff --git a/src/plugins/Makefile.am b/src/plugins/Makefile.am
baab13
index 326bb6e..6dde4b7 100644
baab13
--- a/src/plugins/Makefile.am
baab13
+++ b/src/plugins/Makefile.am
baab13
@@ -261,6 +261,7 @@ abrt_action_install_debuginfo_to_abrt_cache_CPPFLAGS = \
baab13
     -D_GNU_SOURCE \
baab13
     -DBIN_DIR=\"$(bindir)\" \
baab13
     -DSBIN_DIR=\"$(sbindir)\" \
baab13
+    -DLARGE_DATA_TMP_DIR=\"$(LARGE_DATA_TMP_DIR)\" \
baab13
     $(LIBREPORT_CFLAGS) \
baab13
     -Wall -Wwrite-strings \
baab13
     -fPIE
baab13
diff --git a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
baab13
index 81b1486..52d00de 100644
baab13
--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
baab13
+++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
baab13
@@ -108,8 +108,14 @@ int main(int argc, char **argv)
baab13
         build_ids_self_fd = xasprintf("/proc/self/fd/%d", build_ids_fd);
baab13
     }
baab13
 
baab13
-    /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */
baab13
-    const char *args[11];
baab13
+    char tmp_directory[] = LARGE_DATA_TMP_DIR"/abrt-tmp-debuginfo.XXXXXX";
baab13
+    if (mkdtemp(tmp_directory) == NULL)
baab13
+        perror_msg_and_die("Failed to create working directory");
baab13
+
baab13
+    log_info("Created working directory: %s", tmp_directory);
baab13
+
baab13
+    /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, -t, PATH, --, NULL */
baab13
+    const char *args[13];
baab13
     {
baab13
         const char *verbs[] = { "", "-v", "-vv", "-vvv" };
baab13
         unsigned i = 0;
baab13
@@ -130,6 +136,8 @@ int main(int argc, char **argv)
baab13
             args[i++] = "--repo";
baab13
             args[i++] = repo;
baab13
         }
baab13
+        args[i++] = "--tmpdir";
baab13
+        args[i++] = tmp_directory;
baab13
         args[i++] = "--";
baab13
         args[i] = NULL;
baab13
     }
baab13
@@ -204,6 +212,31 @@ int main(int argc, char **argv)
baab13
         umask(0022);
baab13
     }
baab13
 
baab13
-    execvp(EXECUTABLE, (char **)args);
baab13
-    error_msg_and_die("Can't execute %s", EXECUTABLE);
baab13
+    pid_t pid = fork();
baab13
+    if (pid < 0)
baab13
+        perror_msg_and_die("fork");
baab13
+
baab13
+    if (pid == 0)
baab13
+    {
baab13
+        execvp(EXECUTABLE, (char **)args);
baab13
+        error_msg_and_die("Can't execute %s", EXECUTABLE);
baab13
+    }
baab13
+
baab13
+    int status;
baab13
+    if (safe_waitpid(pid, &status, 0) < 0)
baab13
+        perror_msg_and_die("waitpid");
baab13
+
baab13
+    if (rmdir(tmp_directory) >= 0)
baab13
+        log_info("Removed working directory: %s", tmp_directory);
baab13
+    else if (errno != ENOENT)
baab13
+        perror_msg("Failed to remove working directory");
baab13
+
baab13
+    /* Normal execution should exit here. */
baab13
+    if (WIFEXITED(status))
baab13
+        return WEXITSTATUS(status);
baab13
+
baab13
+    if (WIFSIGNALED(status))
baab13
+        error_msg_and_die("Child terminated with signal %d", WTERMSIG(status));
baab13
+
baab13
+    error_msg_and_die("Child exit failed");
baab13
 }
baab13
-- 
baab13
1.8.3.1
baab13