Blob Blame Raw
From 49279f4030225cee166362d7222e97496022331f Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Fri, 17 Apr 2015 14:40:20 +0200
Subject: [PATCH] ccpp: do not use value of /proc/PID/cwd for chdir

Avoid symlink resolutions.

This issue was discovered by Florian Weimer of Red Hat Product Security.

Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
 src/hooks/abrt-hook-ccpp.c | 116 ++++++++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 55 deletions(-)

diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
index b481421..8f3b2b0 100644
--- a/src/hooks/abrt-hook-ccpp.c
+++ b/src/hooks/abrt-hook-ccpp.c
@@ -131,6 +131,7 @@ static off_t copyfd_sparse(int src_fd, int dst_fd1, int dst_fd2, off_t size2)
 
 /* Global data */
 static char *user_pwd;
+static DIR *proc_cwd;
 static struct dump_dir *dd;
 
 /*
@@ -149,22 +150,24 @@ static struct dump_dir *dd;
  */
 static const char percent_specifiers[] = "%scpugtei";
 static char *core_basename = (char*) "core";
-/*
- * Used for error messages only.
- * It is either the same as core_basename if it is absolute,
- * or $PWD/core_basename.
- */
-static char *full_core_basename;
+
+static DIR *open_cwd(pid_t pid)
+{
+    char buf[sizeof("/proc/%lu/cwd") + sizeof(long)*3];
+    sprintf(buf, "/proc/%lu/cwd", (long)pid);
+
+    DIR *cwd = opendir(buf);
+    if (cwd == NULL)
+        perror_msg("Can't open process's CWD for CompatCore");
+
+    return cwd;
+}
 
 static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_values)
 {
-    errno = 0;
-    if (user_pwd == NULL
-     || chdir(user_pwd) != 0
-    ) {
-        perror_msg("Can't cd to '%s'", user_pwd);
+    proc_cwd = open_cwd(pid);
+    if (proc_cwd == NULL)
         return -1;
-    }
 
     struct passwd* pw = getpwuid(uid);
     gid_t gid = pw ? pw->pw_gid : uid;
@@ -227,15 +230,10 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu
         }
     }
 
-    full_core_basename = core_basename;
-    if (core_basename[0] != '/')
+    if (g_need_nonrelative && core_basename[0] != '/')
     {
-        if (g_need_nonrelative)
-        {
-            error_msg("Current suid_dumpable policy prevents from saving core dumps according to relative core_pattern");
-            return -1;
-        }
-        core_basename = concat_path_file(user_pwd, core_basename);
+        error_msg("Current suid_dumpable policy prevents from saving core dumps according to relative core_pattern");
+        return -1;
     }
 
     /* Open (create) compat core file.
@@ -271,7 +269,7 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu
     struct stat sb;
     errno = 0;
     /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */
-    int user_core_fd = open(core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */
+    int user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */
     xsetegid(0);
     xseteuid(0);
     if (user_core_fd < 0
@@ -281,15 +279,15 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu
      || sb.st_uid != fsuid
     ) {
         if (user_core_fd < 0)
-            perror_msg("Can't open '%s'", full_core_basename);
+            perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd);
         else
-            perror_msg("'%s' is not a regular file with link count 1 owned by UID(%d)", full_core_basename, fsuid);
+            perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid);
         return -1;
     }
     if (ftruncate(user_core_fd, 0) != 0) {
         /* perror first, otherwise unlink may trash errno */
-        perror_msg("Can't truncate '%s' to size 0", full_core_basename);
-        unlink(core_basename);
+        perror_msg("Can't truncate '%s' at '%s' to size 0", core_basename, user_pwd);
+        unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
         return -1;
     }
 
@@ -310,10 +308,8 @@ static int create_or_die(const char *filename, int user_core_fd)
     if (dd)
         dd_delete(dd);
     if (user_core_fd >= 0)
-    {
-        xchdir(user_pwd);
-        unlink(core_basename);
-    }
+        unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
+
     errno = sv_errno;
     perror_msg_and_die("Can't open '%s'", filename);
 }
@@ -341,27 +337,34 @@ static void create_core_backtrace(pid_t tid, const char *executable, int signal_
 
 static int create_user_core(int user_core_fd, pid_t pid, off_t ulimit_c)
 {
+    int err = 1;
     if (user_core_fd >= 0)
     {
         off_t core_size = copyfd_size(STDIN_FILENO, user_core_fd, ulimit_c, COPYFD_SPARSE);
         if (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0)
         {
             /* perror first, otherwise unlink may trash errno */
-            perror_msg("Error writing '%s'", full_core_basename);
-            xchdir(user_pwd);
-            unlink(core_basename);
-            return 1;
+            perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd);
+            unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0);
+            goto finito;
         }
         if (ulimit_c == 0 || core_size > ulimit_c)
         {
-            xchdir(user_pwd);
-            unlink(core_basename);
-            return 1;
+            unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0);
+            goto finito;
         }
-        log_notice("Saved core dump of pid %lu to %s (%llu bytes)", (long)pid, full_core_basename, (long long)core_size);
+        log_notice("Saved core dump of pid %lu to %s at '%s' (%llu bytes)", (long)pid, core_basename, user_pwd, (long long)core_size);
     }
+    err = 0;
 
-    return 0;
+finito:
+    if (proc_cwd != NULL)
+    {
+        closedir(proc_cwd);
+        proc_cwd = NULL;
+    }
+
+    return err;
 }
 
 static int test_configuration(bool setting_SaveFullCore, bool setting_CreateCoreBacktrace)
@@ -417,6 +420,7 @@ int main(int argc, char** argv)
     if (fd > 2)
         close(fd);
 
+    int err = 1;
     logmode = LOGMODE_JOURNAL;
 
     /* Parse abrt.conf */
@@ -598,7 +602,8 @@ int main(int argc, char** argv)
             error_msg_and_die("Error saving '%s'", path);
         }
         log_notice("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
-        return 0;
+        err = 0;
+        goto cleanup_and_exit;
     }
 
     unsigned path_len = snprintf(path, sizeof(path), "%s/ccpp-%s-%lu.new",
@@ -669,6 +674,7 @@ int main(int argc, char** argv)
             if (strcmp(rootdir, "/") != 0)
                 dd_save_text(dd, FILENAME_ROOTDIR, rootdir);
         }
+        free(rootdir);
 
         char *reason = xasprintf("%s killed by SIG%s",
                                  last_slash, signame ? signame : signal_str);
@@ -701,7 +707,7 @@ int main(int argc, char** argv)
             {
                 error_msg("Error saving '%s'", path);
 
-                goto error_exit;
+                goto cleanup_and_exit;
             }
         }
 
@@ -730,7 +736,7 @@ int main(int argc, char** argv)
                  * but it does not log file name */
                 error_msg("Error writing '%s'", path);
 
-                goto error_exit;
+                goto cleanup_and_exit;
             }
             if (user_core_fd >= 0
                 /* error writing user coredump? */
@@ -740,8 +746,7 @@ int main(int argc, char** argv)
                 )
             ) {
                 /* nuke it (silently) */
-                xchdir(user_pwd);
-                unlink(core_basename);
+                unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
             }
         }
         else
@@ -787,7 +792,7 @@ int main(int argc, char** argv)
                 {
                     error_msg("Error saving '%s'", path);
 
-                    goto error_exit;
+                    goto cleanup_and_exit;
                 }
                 close(src_fd);
             }
@@ -833,22 +838,23 @@ int main(int argc, char** argv)
             trim_problem_dirs(g_settings_dump_location, maxsize * (double)(1024*1024), path);
         }
 
-        free(rootdir);
-        return 0;
+        err = 0;
+    }
+    else
+    {
+        /* We didn't create abrt dump, but may need to create compat coredump */
+        return create_user_core(user_core_fd, pid, ulimit_c);
     }
 
-    /* We didn't create abrt dump, but may need to create compat coredump */
-    return create_user_core(user_core_fd, pid, ulimit_c);
-
-error_exit:
+cleanup_and_exit:
     if (dd)
         dd_delete(dd);
 
     if (user_core_fd >= 0)
-    {
-        xchdir(user_pwd);
-        unlink(core_basename);
-    }
+        unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0);
+
+    if (proc_cwd != NULL)
+        closedir(proc_cwd);
 
-    xfunc_die();
+    return err;
 }
-- 
2.1.0