CVE-2015-5273: Improper Link Resolution Before File Access ('Link Following') (CWE-59)
value: The abrt-action-install-debuginfo-to-abrt-cache help program in Automatic Bug Reporting Tool (ABRT) before 2.7.1 allows local users to write to arbitrary files via a symlink attack on unpacked.cpio in a pre-created directory with a predictable name in /var/tmp.
CWE description: The product attempts to access a file based on the filename, but it does not properly prevent that filename from identifying a link or shortcut that resolves to an unintended resource.
Published: 2015-12-07T18:59Z
Repository: https://github.com/abrt/abrt
Language: Other
Files changed: src/plugins/Makefile.am, src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
Commit date: 2015-11-23 14:52:53 +0100
Commit message: a-a-i-d-to-abrt-cache: make own random temporary directory

The set-user-ID wrapper must use own new temporary directory in order to
avoid security issues with unpacking specially crafted debuginfo
packages that might be used to create files or symlinks anywhere on the
file system as the abrt user.

Withot the forking code the temporary directory would remain on the
filesystem in the case where all debuginfo data are already available.
This is caused by the fact that the underlying libreport functionality
accepts path to a desired temporary directory and creates it only if
necessary. Otherwise, the directory is not touched at all.

This commit addresses CVE-2015-5273

Signed-off-by: Jakub Filak <jfilak@redhat.com>

Diff: diff --git a/src/plugins/Makefile.am b/src/plugins/Makefile.am
index 85e54ab95..005cc9da1 100644
--- a/src/plugins/Makefile.am
+++ b/src/plugins/Makefile.am
@@ -369,10 +369,11 @@ abrt_action_install_debuginfo_to_abrt_cache_CPPFLAGS = \
     -I$(srcdir)/../include \
     -I$(srcdir)/../lib \
     -D_GNU_SOURCE \
     -DBIN_DIR=\"$(bindir)\" \
     -DSBIN_DIR=\"$(sbindir)\" \
+    -DLARGE_DATA_TMP_DIR=\"$(LARGE_DATA_TMP_DIR)\" \
     $(LIBREPORT_CFLAGS) \
     -Wall -Wwrite-strings \
     -fPIE
  abrt_action_install_debuginfo_to_abrt_cache_LDADD = \
     $(LIBREPORT_LIBS) \
diff --git a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
index 81b14864a..52d00de62 100644
--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
+++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
@@ -106,12 +106,18 @@ int main(int argc, char **argv)
 
         /* We are not going to free this memory. There is no place to do so. */
         build_ids_self_fd = xasprintf("/proc/self/fd/%d", build_ids_fd);
     }
 
-    /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */
-    const char *args[11];
+    char tmp_directory[] = LARGE_DATA_TMP_DIR"/abrt-tmp-debuginfo.XXXXXX";
+    if (mkdtemp(tmp_directory) == NULL)
+        perror_msg_and_die("Failed to create working directory");
+
+    log_info("Created working directory: %s", tmp_directory);
+
+    /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, -t, PATH, --, NULL */
+    const char *args[13];
     {
         const char *verbs[] = { "", "-v", "-vv", "-vvv" };
         unsigned i = 0;
         args[i++] = EXECUTABLE;
         args[i++] = "--ids";
@@ -128,10 +134,12 @@ int main(int argc, char **argv)
         if ((opts & OPT_r))
         {
             args[i++] = "--repo";
             args[i++] = repo;
         }
+        args[i++] = "--tmpdir";
+        args[i++] = tmp_directory;
         args[i++] = "--";
         args[i] = NULL;
     }
 
     /* Switch real user/group to effective ones.
@@ -202,8 +210,33 @@ int main(int argc, char **argv)
 
         /* Use safe umask */
         umask(0022);
     }
 
-    execvp(EXECUTABLE, (char **)args);
-    error_msg_and_die("Can't execute %s", EXECUTABLE);
+    pid_t pid = fork();
+    if (pid < 0)
+        perror_msg_and_die("fork");
+
+    if (pid == 0)
+    {
+        execvp(EXECUTABLE, (char **)args);
+        error_msg_and_die("Can't execute %s", EXECUTABLE);
+    }
+
+    int status;
+    if (safe_waitpid(pid, &status, 0) < 0)
+        perror_msg_and_die("waitpid");
+
+    if (rmdir(tmp_directory) >= 0)
+        log_info("Removed working directory: %s", tmp_directory);
+    else if (errno != ENOENT)
+        perror_msg("Failed to remove working directory");
+
+    /* Normal execution should exit here. */
+    if (WIFEXITED(status))
+        return WEXITSTATUS(status);
+
+    if (WIFSIGNALED(status))
+        error_msg_and_die("Child terminated with signal %d", WTERMSIG(status));
+
+    error_msg_and_die("Child exit failed");
 }

Vulnerable code:     /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */
    const char *args[11];
    execvp(EXECUTABLE, (char **)args);
    error_msg_and_die("Can't execute %s", EXECUTABLE);
Fixed code:     -DLARGE_DATA_TMP_DIR=\"$(LARGE_DATA_TMP_DIR)\" \
    char tmp_directory[] = LARGE_DATA_TMP_DIR"/abrt-tmp-debuginfo.XXXXXX";
    if (mkdtemp(tmp_directory) == NULL)
        perror_msg_and_die("Failed to create working directory");

    log_info("Created working directory: %s", tmp_directory);

    /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, -t, PATH, --, NULL */
    const char *args[13];
        args[i++] = "--tmpdir";
        args[i++] = tmp_directory;
    pid_t pid = fork();
    if (pid < 0)
        perror_msg_and_die("fork");

    if (pid == 0)
    {
        execvp(EXECUTABLE, (char **)args);
        error_msg_and_die("Can't execute %s", EXECUTABLE);
    }

    int status;
    if (safe_waitpid(pid, &status, 0) < 0)
        perror_msg_and_die("waitpid");

    if (rmdir(tmp_directory) >= 0)
        log_info("Removed working directory: %s", tmp_directory);
    else if (errno != ENOENT)
        perror_msg("Failed to remove working directory");

    /* Normal execution should exit here. */
    if (WIFEXITED(status))
        return WEXITSTATUS(status);

    if (WIFSIGNALED(status))
        error_msg_and_die("Child terminated with signal %d", WTERMSIG(status));

    error_msg_and_die("Child exit failed");