From 49279f4030225cee166362d7222e97496022331f Mon Sep 17 00:00:00 2001 From: Jakub Filak 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 --- 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