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