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