From 8ec399c80a9c17183e07b6c53d76c802a124fcd6 Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: Jun 09 2015 15:21:31 +0000 Subject: import abrt-2.1.11-22.el7_1 --- diff --git a/SOURCES/0088-a-a-save-package-data-turn-off-reading-data-from-roo.patch b/SOURCES/0088-a-a-save-package-data-turn-off-reading-data-from-roo.patch new file mode 100644 index 0000000..af4fd62 --- /dev/null +++ b/SOURCES/0088-a-a-save-package-data-turn-off-reading-data-from-roo.patch @@ -0,0 +1,65 @@ +From fdf93685d4f3fc36fe50d34a11e24662c4cb2d8c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 12:12:59 +0200 +Subject: [ABRT PATCH] a-a-save-package-data: turn off reading data from root + directories + +Making copies of files from arbitrary root directories is not secure. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-action-save-package-data.c | 8 ++------ + 1 file changed, 2 insertions(+), 6 deletions(-) + +diff --git a/src/daemon/abrt-action-save-package-data.c b/src/daemon/abrt-action-save-package-data.c +index 6dbcfc2..97d5f5e 100644 +--- a/src/daemon/abrt-action-save-package-data.c ++++ b/src/daemon/abrt-action-save-package-data.c +@@ -223,7 +223,6 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name) + + char *cmdline = NULL; + char *executable = NULL; +- char *rootdir = NULL; + char *package_short_name = NULL; + struct pkg_envra *pkg_name = NULL; + char *component = NULL; +@@ -233,8 +232,6 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name) + + cmdline = dd_load_text_ext(dd, FILENAME_CMDLINE, DD_FAIL_QUIETLY_ENOENT); + executable = dd_load_text(dd, FILENAME_EXECUTABLE); +- rootdir = dd_load_text_ext(dd, FILENAME_ROOTDIR, +- DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE); + + /* Close dd while we query package database. It can take some time, + * don't want to keep dd locked longer than necessary */ +@@ -246,7 +243,7 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name) + goto ret; /* return 1 (failure) */ + } + +- pkg_name = rpm_get_package_nvr(executable, rootdir); ++ pkg_name = rpm_get_package_nvr(executable, NULL); + if (!pkg_name) + { + if (settings_bProcessUnpackaged) +@@ -329,7 +326,7 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name) + */ + } + +- component = rpm_get_component(executable, rootdir); ++ component = rpm_get_component(executable, NULL); + + dd = dd_opendir(dump_dir_name, /*flags:*/ 0); + if (!dd) +@@ -355,7 +352,6 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name) + ret: + free(cmdline); + free(executable); +- free(rootdir); + free(package_short_name); + free_pkg_envra(pkg_name); + free(component); +-- +1.8.3.1 + diff --git a/SOURCES/0088-event-don-t-run-the-reporter-bugzilla-h-on-RHEL-and-.patch b/SOURCES/0088-event-don-t-run-the-reporter-bugzilla-h-on-RHEL-and-.patch deleted file mode 100644 index 334986f..0000000 --- a/SOURCES/0088-event-don-t-run-the-reporter-bugzilla-h-on-RHEL-and-.patch +++ /dev/null @@ -1,29 +0,0 @@ -From 6c95ae2bf1c80530442a516f23b7cd8e82dcae12 Mon Sep 17 00:00:00 2001 -From: Matej Habrnal -Date: Thu, 22 Jan 2015 02:23:21 +0100 -Subject: [PATCH 88/91] event: don't run the 'reporter-bugzilla -h' on RHEL and - CentOS - -Running the 'reporter-bugzilla -h' makes sense only on Fedora because of bodhi. - -Signed-off-by: Matej Habrnal ---- - src/plugins/ccpp_event.conf | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/src/plugins/ccpp_event.conf b/src/plugins/ccpp_event.conf -index 62ff08a..cd75ee2 100644 ---- a/src/plugins/ccpp_event.conf -+++ b/src/plugins/ccpp_event.conf -@@ -71,7 +71,7 @@ EVENT=analyze_LocalGDB analyzer=CCpp - # Run GDB plugin to see if crash looks exploitable - abrt-action-analyze-vulnerability - # Run GDB to genereate backtrace -- abrt-action-analyze-ccpp-local --without-bodhi -+ abrt-action-analyze-ccpp-local --without-bz - - - # Bugzilla requires nonempty duphash --- -1.8.3.1 - diff --git a/SOURCES/0089-ccpp-fix-symlink-race-conditions.patch b/SOURCES/0089-ccpp-fix-symlink-race-conditions.patch new file mode 100644 index 0000000..35227f9 --- /dev/null +++ b/SOURCES/0089-ccpp-fix-symlink-race-conditions.patch @@ -0,0 +1,80 @@ +From 80408e9e24a1c10f85fd969e1853e0f192157f92 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 12:14:22 +0200 +Subject: [ABRT PATCH] ccpp: fix symlink race conditions + +Fix copy & chown race conditions + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 27 ++++++++++++++++----------- + 1 file changed, 16 insertions(+), 11 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 8e141d4..be16fab 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -397,7 +397,7 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + return user_core_fd; + } + +-static bool dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs) ++static bool dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs, uid_t uid, gid_t gid) + { + FILE *fp = fopen(dest_filename, "w"); + if (!fp) +@@ -429,6 +429,16 @@ static bool dump_fd_info(const char *dest_filename, char *source_filename, int s + } + fclose(in); + } ++ ++ const int dest_fd = fileno(fp); ++ if (fchown(dest_fd, uid, gid) < 0) ++ { ++ perror_msg("Can't change '%s' ownership to %lu:%lu", dest_filename, (long)uid, (long)gid); ++ fclose(fp); ++ unlink(dest_filename); ++ return false; ++ } ++ + fclose(fp); + return true; + } +@@ -678,27 +688,22 @@ int main(int argc, char** argv) + + // Disabled for now: /proc/PID/smaps tends to be BIG, + // and not much more informative than /proc/PID/maps: +- //copy_file(source_filename, dest_filename, 0640); +- //chown(dest_filename, dd->dd_uid, dd->dd_gid); ++ //copy_file_ext(source_filename, dest_filename, 0640, dd->dd_uid, dd->dd_gid, O_RDONLY, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL); + + strcpy(source_filename + source_base_ofs, "maps"); + strcpy(dest_base, FILENAME_MAPS); +- copy_file(source_filename, dest_filename, DEFAULT_DUMP_DIR_MODE); +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ copy_file_ext(source_filename, dest_filename, 0640, dd->dd_uid, dd->dd_gid, O_RDONLY, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL); + + strcpy(source_filename + source_base_ofs, "limits"); + strcpy(dest_base, FILENAME_LIMITS); +- copy_file(source_filename, dest_filename, DEFAULT_DUMP_DIR_MODE); +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ copy_file_ext(source_filename, dest_filename, 0640, dd->dd_uid, dd->dd_gid, O_RDONLY, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL); + + strcpy(source_filename + source_base_ofs, "cgroup"); + strcpy(dest_base, FILENAME_CGROUP); +- copy_file(source_filename, dest_filename, DEFAULT_DUMP_DIR_MODE); +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ copy_file_ext(source_filename, dest_filename, 0640, dd->dd_uid, dd->dd_gid, O_RDONLY, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL); + + strcpy(dest_base, FILENAME_OPEN_FDS); +- if (dump_fd_info(dest_filename, source_filename, source_base_ofs)) +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ dump_fd_info(dest_filename, source_filename, source_base_ofs, dd->dd_uid, dd->dd_gid); + + free(dest_filename); + +-- +1.8.3.1 + diff --git a/SOURCES/0090-ccpp-stop-reading-hs_error.log-from-tmp.patch b/SOURCES/0090-ccpp-stop-reading-hs_error.log-from-tmp.patch new file mode 100644 index 0000000..b88e095 --- /dev/null +++ b/SOURCES/0090-ccpp-stop-reading-hs_error.log-from-tmp.patch @@ -0,0 +1,39 @@ +From 17cb66b13997b0159b4253b3f5722db79f476d68 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 28 Apr 2015 14:00:18 +0200 +Subject: [ABRT PATCH] ccpp: stop reading hs_error.log from /tmp + +The file might contain anything and there is no way to verify its +contents. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index be16fab..5694f84 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -795,6 +795,8 @@ int main(int argc, char** argv) + unlink(core_basename); + } + ++/* Because of #1211835 and #1126850 */ ++#if 0 + /* Save JVM crash log if it exists. (JVM's coredump per se + * is nearly useless for JVM developers) + */ +@@ -827,6 +829,7 @@ int main(int argc, char** argv) + close(src_fd); + } + } ++#endif + + /* We close dumpdir before we start catering for crash storm case. + * Otherwise, delete_dump_dir's from other concurrent +-- +1.8.3.1 + diff --git a/SOURCES/0090-plugin-set-URL-to-retrace-server.patch b/SOURCES/0090-plugin-set-URL-to-retrace-server.patch deleted file mode 100644 index b2dc31a..0000000 --- a/SOURCES/0090-plugin-set-URL-to-retrace-server.patch +++ /dev/null @@ -1,42 +0,0 @@ -From 81181a893b91a229e05a5a915cc98347126e3834 Mon Sep 17 00:00:00 2001 -From: Matej Habrnal -Date: Fri, 30 Jan 2015 17:52:25 +0100 -Subject: [PATCH 90/91] plugin: set URL to retrace server - -Changed default retrace server URL from localhost to retrace.fedoraproject.org. - -Signed-off-by: Matej Habrnal ---- - src/plugins/analyze_CCpp.xml.in | 2 +- - src/plugins/analyze_RetraceServer.xml.in | 2 +- - 2 files changed, 2 insertions(+), 2 deletions(-) - -diff --git a/src/plugins/analyze_CCpp.xml.in b/src/plugins/analyze_CCpp.xml.in -index 6f02692..a7ce4dd 100644 ---- a/src/plugins/analyze_CCpp.xml.in -+++ b/src/plugins/analyze_CCpp.xml.in -@@ -26,7 +26,7 @@ - - -diff --git a/src/plugins/analyze_RetraceServer.xml.in b/src/plugins/analyze_RetraceServer.xml.in -index cf1d25a..e437cac 100644 ---- a/src/plugins/analyze_RetraceServer.xml.in -+++ b/src/plugins/analyze_RetraceServer.xml.in -@@ -12,7 +12,7 @@ - - --- -1.8.3.1 - diff --git a/SOURCES/0091-ccpp-do-not-read-data-from-root-directories.patch b/SOURCES/0091-ccpp-do-not-read-data-from-root-directories.patch new file mode 100644 index 0000000..f5752c6 --- /dev/null +++ b/SOURCES/0091-ccpp-do-not-read-data-from-root-directories.patch @@ -0,0 +1,31 @@ +From 4f2c1ddd3e3b81d2d5146b883115371f1cada9f9 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 12:14:52 +0200 +Subject: [ABRT PATCH] ccpp: do not read data from root directories + +Users are allowed to modify /proc/[pid]/root to any directory by running +their own MOUNT namespace. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 5694f84..0606519 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -678,7 +678,7 @@ int main(int argc, char** argv) + { + char *rootdir = get_rootdir(pid); + +- dd_create_basic_files(dd, fsuid, (rootdir && strcmp(rootdir, "/") != 0) ? rootdir : NULL); ++ dd_create_basic_files(dd, fsuid, NULL); + + char source_filename[sizeof("/proc/%lu/somewhat_long_name") + sizeof(long)*3]; + int source_base_ofs = sprintf(source_filename, "/proc/%lu/smaps", (long)pid); +-- +1.8.3.1 + diff --git a/SOURCES/0092-ccpp-open-file-for-dump_fd_info-with-O_EXCL.patch b/SOURCES/0092-ccpp-open-file-for-dump_fd_info-with-O_EXCL.patch new file mode 100644 index 0000000..a417cf2 --- /dev/null +++ b/SOURCES/0092-ccpp-open-file-for-dump_fd_info-with-O_EXCL.patch @@ -0,0 +1,30 @@ +From d6e2f6f128cef4c21cb80941ae674c9842681aa7 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 14:01:37 +0200 +Subject: [ABRT PATCH] ccpp: open file for dump_fd_info with O_EXCL + +To avoid possible races. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 0606519..ece1ece 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -399,7 +399,7 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + + static bool dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs, uid_t uid, gid_t gid) + { +- FILE *fp = fopen(dest_filename, "w"); ++ FILE *fp = fopen(dest_filename, "wx"); + if (!fp) + return false; + +-- +1.8.3.1 + diff --git a/SOURCES/0093-ccpp-postpone-changing-ownership-of-new-dump-directo.patch b/SOURCES/0093-ccpp-postpone-changing-ownership-of-new-dump-directo.patch new file mode 100644 index 0000000..eff9538 --- /dev/null +++ b/SOURCES/0093-ccpp-postpone-changing-ownership-of-new-dump-directo.patch @@ -0,0 +1,53 @@ +From a4794b39efc62c9ba92b38b419de3babbbcd8cfb Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 15:27:09 +0200 +Subject: [ABRT PATCH] ccpp: postpone changing ownership of new dump + directories + +Florian Weimer : + + Currently, dd_create changes ownership of the directory immediately, + when it is still empty. This means that any operations within the + directory (which happen as the root user) can race with changes to + the directory contents by the user. If you delay changing directory + ownership until all the files have created and written, this is no + longer a problem. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index ece1ece..7e05aa6 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -672,8 +672,12 @@ int main(int argc, char** argv) + + /* use fsuid instead of uid, so we don't expose any sensitive + * information of suided app in /var/tmp/abrt ++ * ++ * dd_create_skeleton() creates a new directory and leaves ownership to ++ * the current user, hence, we have to call dd_reset_ownership() after the ++ * directory is populated. + */ +- dd = dd_create(path, fsuid, DEFAULT_DUMP_DIR_MODE); ++ dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE); + if (dd) + { + char *rootdir = get_rootdir(pid); +@@ -831,6 +835,9 @@ int main(int argc, char** argv) + } + #endif + ++ /* And finally set the right uid and gid */ ++ dd_reset_ownership(dd); ++ + /* We close dumpdir before we start catering for crash storm case. + * Otherwise, delete_dump_dir's from other concurrent + * CCpp's won't be able to delete our dump (their delete_dump_dir +-- +1.8.3.1 + diff --git a/SOURCES/0094-ccpp-create-dump-directory-without-parents.patch b/SOURCES/0094-ccpp-create-dump-directory-without-parents.patch new file mode 100644 index 0000000..fb89031 --- /dev/null +++ b/SOURCES/0094-ccpp-create-dump-directory-without-parents.patch @@ -0,0 +1,31 @@ +From 2f948bdc09aa346616852a421ce1af2e03b39997 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 17:42:59 +0200 +Subject: [ABRT PATCH] ccpp: create dump directory without parents + +This patch makes the code more robust. +This patch ensures that abrt-hook-ccpp never creates the dump location. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 7e05aa6..85e0d35 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -677,7 +677,7 @@ int main(int argc, char** argv) + * the current user, hence, we have to call dd_reset_ownership() after the + * directory is populated. + */ +- dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE); ++ dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0); + if (dd) + { + char *rootdir = get_rootdir(pid); +-- +1.8.3.1 + diff --git a/SOURCES/0095-ccpp-do-not-override-existing-files-by-compat-cores.patch b/SOURCES/0095-ccpp-do-not-override-existing-files-by-compat-cores.patch new file mode 100644 index 0000000..37bb6e7 --- /dev/null +++ b/SOURCES/0095-ccpp-do-not-override-existing-files-by-compat-cores.patch @@ -0,0 +1,82 @@ +From af945ff58a698ce00c45059a05994ef53a13e192 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:36:45 +0200 +Subject: [ABRT PATCH] ccpp: do not override existing files by compat cores + +Implement all checks used in kernel's do_coredump() and require +non-relative path if suid_dumpable is 2. + +Related: #1212818 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 20 ++++++++++++++++---- + 1 file changed, 16 insertions(+), 4 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 85e0d35..82ff555 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -24,6 +24,8 @@ + #define DUMP_SUID_UNSAFE 1 + #define DUMP_SUID_SAFE 2 + ++static int g_user_core_flags; ++static int g_need_nonrelative; + + /* I want to use -Werror, but gcc-4.4 throws a curveball: + * "warning: ignoring return value of 'ftruncate', declared with attribute warn_unused_result" +@@ -337,7 +339,14 @@ 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) ++ { ++ 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); ++ } + + /* Open (create) compat core file. + * man core: +@@ -372,19 +381,19 @@ 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, 0600); /* kernel makes 0600 too */ ++ int user_core_fd = open(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 + || fstat(user_core_fd, &sb) != 0 + || !S_ISREG(sb.st_mode) + || sb.st_nlink != 1 +- /* kernel internal dumper checks this too: if (inode->i_uid != current->fsuid) , need to mimic? */ ++ || sb.st_uid != fsuid + ) { + if (user_core_fd < 0) + perror_msg("Can't open '%s'", full_core_basename); + else +- perror_msg("'%s' is not a regular file with link count 1", full_core_basename); ++ perror_msg("'%s' is not a regular file with link count 1 owned by UID(%d)", full_core_basename, fsuid); + return -1; + } + if (ftruncate(user_core_fd, 0) != 0) { +@@ -578,8 +587,11 @@ int main(int argc, char** argv) + /* use root for suided apps unless it's explicitly set to UNSAFE */ + fsuid = 0; + if (suid_policy == DUMP_SUID_UNSAFE) +- { + fsuid = tmp_fsuid; ++ else ++ { ++ g_user_core_flags = O_EXCL; ++ g_need_nonrelative = 1; + } + } + +-- +1.8.3.1 + diff --git a/SOURCES/0096-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch b/SOURCES/0096-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch new file mode 100644 index 0000000..c0ab080 --- /dev/null +++ b/SOURCES/0096-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch @@ -0,0 +1,218 @@ +From 806bb07571b698d90169c3b73cb65cd09c900284 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:40:20 +0200 +Subject: [ABRT 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 | 85 +++++++++++++++++++++++----------------------- + 1 file changed, 42 insertions(+), 43 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 82ff555..d600bb7 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -144,6 +144,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 char *proc_pid_status; + static struct dump_dir *dd; + static int user_core_fd = -1; +@@ -163,13 +164,6 @@ static int user_core_fd = -1; + */ + static const char percent_specifiers[] = "%scpugteh"; + 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 char* get_executable(pid_t pid, int *fd_p) + { +@@ -198,6 +192,18 @@ static char* get_executable(pid_t pid, int *fd_p) + return executable; + } + ++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 char* get_cwd(pid_t pid) + { + char buf[sizeof("/proc/%lu/cwd") + sizeof(long)*3]; +@@ -268,13 +274,9 @@ static int dump_suid_policy() + + 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; +@@ -337,15 +339,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. +@@ -381,7 +378,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 +@@ -391,15 +388,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; + } + +@@ -466,10 +463,8 @@ static int create_or_die(const char *filename) + 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); + } +@@ -573,7 +568,7 @@ int main(int argc, char** argv) + (long)pid, executable); + } + +- user_pwd = get_cwd(pid); /* may be NULL on error */ ++ user_pwd = get_cwd(pid); + log_notice("user_pwd:'%s'", user_pwd); + + sprintf(path, "/proc/%lu/status", (long)pid); +@@ -672,6 +667,8 @@ int main(int argc, char** argv) + error_msg_and_die("Error saving '%s'", path); + } + log("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size); ++ if (proc_cwd != NULL) ++ closedir(proc_cwd); + return 0; + } + +@@ -791,10 +788,7 @@ int main(int argc, char** argv) + unlink(path); + dd_delete(dd); + if (user_core_fd >= 0) +- { +- xchdir(user_pwd); +- unlink(core_basename); +- } ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); + /* copyfd_sparse logs the error including errno string, + * but it does not log file name */ + error_msg_and_die("Error writing '%s'", path); +@@ -807,8 +801,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); + } + + /* Because of #1211835 and #1126850 */ +@@ -879,6 +872,8 @@ int main(int argc, char** argv) + } + + free(rootdir); ++ if (proc_cwd != NULL) ++ closedir(proc_cwd); + return 0; + } + +@@ -890,19 +885,23 @@ int main(int argc, char** argv) + 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); ++ perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd); ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); ++ if (proc_cwd != NULL) ++ closedir(proc_cwd); + return 1; + } + if (ulimit_c == 0 || core_size > ulimit_c) + { +- xchdir(user_pwd); +- unlink(core_basename); ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); ++ if (proc_cwd != NULL) ++ closedir(proc_cwd); + return 1; + } +- log("Saved core dump of pid %lu to %s (%llu bytes)", (long)pid, full_core_basename, (long long)core_size); ++ log("Saved core dump of pid %lu to %s at %s (%llu bytes)", (long)pid, core_basename, user_pwd, (long long)core_size); + } + ++ if (proc_cwd != NULL) ++ closedir(proc_cwd); + return 0; + } +-- +1.8.3.1 + diff --git a/SOURCES/0097-ccpp-harden-dealing-with-UID-GID.patch b/SOURCES/0097-ccpp-harden-dealing-with-UID-GID.patch new file mode 100644 index 0000000..601341a --- /dev/null +++ b/SOURCES/0097-ccpp-harden-dealing-with-UID-GID.patch @@ -0,0 +1,91 @@ +From b72616471ec52a009904689592f4f69e730a6f56 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:42:13 +0200 +Subject: [ABRT PATCH] ccpp: harden dealing with UID/GID + +* Don't fall back to UID 0. +* Use fsgid. + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 39 ++++++++++++++++++++++++++------------- + 1 file changed, 26 insertions(+), 13 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index d600bb7..d9f1f5e 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -218,23 +218,27 @@ static char* get_rootdir(pid_t pid) + return malloc_readlink(buf); + } + +-static int get_fsuid(void) ++static int get_proc_fs_id(char type) + { +- int real, euid, saved; +- /* if we fail to parse the uid, then make it root only readable to be safe */ +- int fs_uid = 0; ++ const char *scanf_format = "%*cid:\t%d\t%d\t%d\t%d\n"; ++ char id_type[] = "_id"; ++ id_type[0] = type; ++ ++ int real, e_id, saved; ++ int fs_id = 0; + + char *line = proc_pid_status; /* never NULL */ + for (;;) + { +- if (strncmp(line, "Uid", 3) == 0) ++ if (strncmp(line, id_type, 3) == 0) + { +- int n = sscanf(line, "Uid:\t%d\t%d\t%d\t%d\n", &real, &euid, &saved, &fs_uid); ++ int n = sscanf(line, scanf_format, &real, &e_id, &saved, &fs_id); + if (n != 4) + { +- perror_msg_and_die("Can't parse Uid: line"); ++ perror_msg_and_die("Can't parse %cid: line", type); + } +- break; ++ ++ return fs_id; + } + line = strchr(line, '\n'); + if (!line) +@@ -242,7 +246,17 @@ static int get_fsuid(void) + line++; + } + +- return fs_uid; ++ perror_msg_and_die("Failed to get file system %cID of the crashed process", type); ++} ++ ++static int get_fsuid(void) ++{ ++ return get_proc_fs_id(/*UID*/'U'); ++} ++ ++static int get_fsgid(void) ++{ ++ return get_proc_fs_id(/*GID*/'G'); + } + + static int dump_suid_policy() +@@ -278,10 +292,9 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + if (proc_cwd == NULL) + return -1; + +- struct passwd* pw = getpwuid(uid); +- gid_t gid = pw ? pw->pw_gid : uid; +- //log("setting uid: %i gid: %i", uid, gid); +- xsetegid(gid); ++ errno = 0; ++ ++ xsetegid(get_fsgid()); + xseteuid(fsuid); + + if (strcmp(core_basename, "core") == 0) +-- +1.8.3.1 + diff --git a/SOURCES/0098-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch b/SOURCES/0098-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch new file mode 100644 index 0000000..00f94ac --- /dev/null +++ b/SOURCES/0098-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch @@ -0,0 +1,30 @@ +From 28ce40d8db91c1926a95f21ef19a980a8af88471 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:43:59 +0200 +Subject: [ABRT PATCH] ccpp: check for overflow in abrt coredump path creation + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index d9f1f5e..81f9349 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -669,7 +669,9 @@ int main(int argc, char** argv) + * and maybe crash again... + * Unlike dirs, mere files are ignored by abrtd. + */ +- snprintf(path, sizeof(path), "%s/%s-coredump", g_settings_dump_location, last_slash); ++ if (snprintf(path, sizeof(path), "%s/%s-coredump", g_settings_dump_location, last_slash) >= sizeof(path)) ++ error_msg_and_die("Error saving '%s': truncated long file path", path); ++ + int abrt_core_fd = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0600); + off_t core_size = copyfd_eof(STDIN_FILENO, abrt_core_fd, COPYFD_SPARSE); + if (core_size < 0 || fsync(abrt_core_fd) != 0) +-- +1.8.3.1 + diff --git a/SOURCES/0099-ccpp-emulate-selinux-for-creation-of-compat-cores.patch b/SOURCES/0099-ccpp-emulate-selinux-for-creation-of-compat-cores.patch new file mode 100644 index 0000000..58b2e2c --- /dev/null +++ b/SOURCES/0099-ccpp-emulate-selinux-for-creation-of-compat-cores.patch @@ -0,0 +1,188 @@ +From 2f0a18b499b9b0e1afbdab8a8bb31d38f2acc6d8 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 16:06:33 +0200 +Subject: [ABRT PATCH] ccpp: emulate selinux for creation of compat cores + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +http://article.gmane.org/gmane.comp.security.selinux/21842 + +v2: use the _raw interface and do the preparation steps as root +v3: don't fail if SELinux is disabled + https://github.com/abrt/abrt/commit/c4f06d4198658c82550e93bb2617b96022c06cf4#commitcomment-11021276 + +Signed-off-by: Jakub Filak +--- + configure.ac | 1 + + src/hooks/Makefile.am | 4 ++- + src/hooks/abrt-hook-ccpp.c | 85 ++++++++++++++++++++++++++++++++++++++++++++-- + 3 files changed, 86 insertions(+), 4 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 9ff616d..6c6d2e8 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -106,6 +106,7 @@ PKG_CHECK_MODULES([LIBREPORT_GTK], [libreport-gtk]) + PKG_CHECK_MODULES([POLKIT], [polkit-gobject-1]) + PKG_CHECK_MODULES([GIO], [gio-2.0]) + PKG_CHECK_MODULES([SATYR], [satyr]) ++PKG_CHECK_MODULES([LIBSELINUX], [libselinux]) + + PKG_PROG_PKG_CONFIG + AC_ARG_WITH([systemdsystemunitdir], +diff --git a/src/hooks/Makefile.am b/src/hooks/Makefile.am +index e536089..9a527f4 100644 +--- a/src/hooks/Makefile.am ++++ b/src/hooks/Makefile.am +@@ -33,10 +33,12 @@ abrt_hook_ccpp_CPPFLAGS = \ + -DDEFAULT_DUMP_DIR_MODE=$(DEFAULT_DUMP_DIR_MODE) \ + $(GLIB_CFLAGS) \ + $(LIBREPORT_CFLAGS) \ ++ $(LIBSELINUX_CFLAGS) \ + -D_GNU_SOURCE + abrt_hook_ccpp_LDADD = \ + ../lib/libabrt.la \ +- $(LIBREPORT_LIBS) ++ $(LIBREPORT_LIBS) \ ++ $(LIBSELINUX_LIBS) + + # abrt-merge-pstoreoops + abrt_merge_pstoreoops_SOURCES = \ +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 81f9349..00ae621 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -20,6 +20,7 @@ + */ + #include + #include "libabrt.h" ++#include + + #define DUMP_SUID_UNSAFE 1 + #define DUMP_SUID_SAFE 2 +@@ -286,6 +287,54 @@ static int dump_suid_policy() + return suid_dump_policy; + } + ++/* Computes a security context of new file created by the given process with ++ * pid in the given directory represented by file descriptor. ++ * ++ * On errors returns negative number. Returns 0 if the function succeeds and ++ * computes the context and returns positive number and assigns NULL to newcon ++ * if the security context is not needed (SELinux disabled). ++ */ ++static int compute_selinux_con_for_new_file(pid_t pid, int dir_fd, security_context_t *newcon) ++{ ++ security_context_t srccon; ++ security_context_t dstcon; ++ ++ const int r = is_selinux_enabled(); ++ if (r == 0) ++ { ++ *newcon = NULL; ++ return 1; ++ } ++ else if (r == -1) ++ { ++ perror_msg("Couldn't get state of SELinux"); ++ return -1; ++ } ++ else if (r != 1) ++ error_msg_and_die("Unexpected SELinux return value: %d", r); ++ ++ ++ if (getpidcon_raw(pid, &srccon) < 0) ++ { ++ perror_msg("getpidcon_raw(%d)", pid); ++ return -1; ++ } ++ ++ if (fgetfilecon_raw(dir_fd, &dstcon) < 0) ++ { ++ perror_msg("getfilecon_raw(%s)", user_pwd); ++ return -1; ++ } ++ ++ if (security_compute_create_raw(srccon, dstcon, string_to_security_class("file"), newcon) < 0) ++ { ++ perror_msg("security_compute_create_raw(%s, %s, 'file')", srccon, dstcon); ++ return -1; ++ } ++ ++ return 0; ++} ++ + static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_values) + { + proc_cwd = open_cwd(pid); +@@ -294,6 +343,14 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + + errno = 0; + ++ /* http://article.gmane.org/gmane.comp.security.selinux/21842 */ ++ security_context_t newcon; ++ if (compute_selinux_con_for_new_file(pid, dirfd(proc_cwd), &newcon) < 0) ++ { ++ log_notice("Not going to create a user core due to SELinux errors"); ++ return -1; ++ } ++ + xsetegid(get_fsgid()); + xseteuid(fsuid); + +@@ -388,10 +445,25 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + * (However, see the description of the prctl(2) PR_SET_DUMPABLE operation, + * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).) + */ ++ ++ /* Set SELinux context like kernel when creating core dump file */ ++ if (newcon != NULL && setfscreatecon_raw(newcon) < 0) ++ { ++ perror_msg("setfscreatecon_raw(%s)", newcon); ++ return -1; ++ } ++ + 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 = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ ++ ++ if (newcon != NULL && setfscreatecon_raw(NULL) < 0) ++ { ++ error_msg("setfscreatecon_raw(NULL)"); ++ goto user_core_fail; ++ } ++ + xsetegid(0); + xseteuid(0); + if (user_core_fd < 0 +@@ -404,16 +476,23 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd); + else + 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; ++ goto user_core_fail; + } + if (ftruncate(user_core_fd, 0) != 0) { + /* perror first, otherwise unlink may trash errno */ + 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; ++ goto user_core_fail; + } + + return user_core_fd; ++ ++user_core_fail: ++ if (user_core_fd >= 0) ++ { ++ close(user_core_fd); ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); ++ } ++ return -1; + } + + static bool dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs, uid_t uid, gid_t gid) +-- +1.8.3.1 + diff --git a/SOURCES/0100-make-the-dump-directories-owned-by-root-by-default.patch b/SOURCES/0100-make-the-dump-directories-owned-by-root-by-default.patch new file mode 100644 index 0000000..6fd9548 --- /dev/null +++ b/SOURCES/0100-make-the-dump-directories-owned-by-root-by-default.patch @@ -0,0 +1,233 @@ +From 8939398b82006ba1fec4ed491339fc075f43fc7c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 20 Apr 2015 07:56:34 +0200 +Subject: [ABRT PATCH] make the dump directories owned by root by default + +It was discovered that the abrt event scripts create a user-readable +copy of a sosreport file in abrt problem directories, and include +excerpts of /var/log/messages selected by the user-controlled process +name, leading to an information disclosure. + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Related: #1212868 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 34 ++++++++++++++++++++++++++++++++-- + src/daemon/abrt.conf | 5 +++++ + src/hooks/abrt-hook-ccpp.c | 10 +++++++--- + src/include/libabrt.h | 2 ++ + src/lib/abrt_conf.c | 8 ++++++++ + src/lib/hooklib.c | 7 ++++++- + src/plugins/abrt-dump-oops.c | 8 ++++++++ + src/plugins/abrt-dump-xorg.c | 8 ++++++++ + 8 files changed, 76 insertions(+), 6 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 307b41b..5789075 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -15,6 +15,7 @@ + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ ++#include "problem_api.h" + #include "libabrt.h" + + /* Maximal length of backtrace. */ +@@ -153,7 +154,36 @@ static int run_post_create(const char *dirname) + error_msg("Bad problem directory name '%s', should start with: '%s'", dirname, g_settings_dump_location); + return 400; /* Bad Request */ + } +- if (!dump_dir_accessible_by_uid(dirname, client_uid)) ++ if (g_settings_privatereports) ++ { ++ struct stat statbuf; ++ if (lstat(dirname, &statbuf) != 0 || !S_ISDIR(statbuf.st_mode)) ++ { ++ error_msg("Path '%s' isn't directory", dirname); ++ return 404; /* Not Found */ ++ } ++ /* Get ABRT's group gid */ ++ struct group *gr = getgrnam("abrt"); ++ if (!gr) ++ { ++ error_msg("Group 'abrt' does not exist"); ++ return 500; ++ } ++ if (statbuf.st_uid != 0 || !(statbuf.st_gid == 0 || statbuf.st_gid == gr->gr_gid) || statbuf.st_mode & 07) ++ { ++ error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dirname); ++ return 403; ++ } ++ struct dump_dir *dd = dd_opendir(dirname, DD_OPEN_READONLY); ++ const bool complete = dd && problem_dump_dir_is_complete(dd); ++ dd_close(dd); ++ if (complete) ++ { ++ error_msg("Problem directory '%s' has already been processed", dirname); ++ return 403; ++ } ++ } ++ else if (!dump_dir_accessible_by_uid(dirname, client_uid)) + { + if (errno == ENOTDIR) + { +@@ -377,7 +407,7 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid) + /* No need to check the path length, as all variables used are limited, + * and dd_create() fails if the path is too long. + */ +- struct dump_dir *dd = dd_create(path, client_uid, DEFAULT_DUMP_DIR_MODE); ++ struct dump_dir *dd = dd_create(path, g_settings_privatereports ? 0 : client_uid, DEFAULT_DUMP_DIR_MODE); + if (!dd) + { + error_msg_and_die("Error creating problem directory '%s'", path); +diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf +index 59d1831..6c0d6b0 100644 +--- a/src/daemon/abrt.conf ++++ b/src/daemon/abrt.conf +@@ -43,3 +43,8 @@ AutoreportingEnabled = no + # session; otherwise No. + # + # ShortenedReporting = yes ++ ++# Disable this if you want to regular users to own the problem data colleted by ++# abrt. ++# ++PrivateReports = yes +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 00ae621..3a6d002 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -682,6 +682,9 @@ int main(int argc, char** argv) + } + } + ++ /* If PrivateReports is on, root owns all problem directories */ ++ const uid_t dduid = g_settings_privatereports ? 0 : fsuid; ++ + /* Open a fd to compat coredump, if requested and is possible */ + if (setting_MakeCompatCore && ulimit_c != 0) + /* note: checks "user_pwd == NULL" inside; updates core_basename */ +@@ -773,18 +776,19 @@ int main(int argc, char** argv) + goto create_user_core; + } + +- /* use fsuid instead of uid, so we don't expose any sensitive +- * information of suided app in /var/tmp/abrt ++ /* use dduid (either fsuid or 0) instead of uid, so we don't expose any ++ * sensitive information of suided app in /var/tmp/abrt + * + * dd_create_skeleton() creates a new directory and leaves ownership to + * the current user, hence, we have to call dd_reset_ownership() after the + * directory is populated. + */ +- dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0); ++ dd = dd_create_skeleton(path, dduid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0); + if (dd) + { + char *rootdir = get_rootdir(pid); + ++ /* This function uses fsuid only for its value. The function stores fsuid in a file name 'uid'*/ + dd_create_basic_files(dd, fsuid, NULL); + + char source_filename[sizeof("/proc/%lu/somewhat_long_name") + sizeof(long)*3]; +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 85a5a5c..0320c5b 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -62,6 +62,8 @@ extern bool g_settings_autoreporting; + extern char * g_settings_autoreporting_event; + #define g_settings_shortenedreporting abrt_g_settings_shortenedreporting + extern bool g_settings_shortenedreporting; ++#define g_settings_privatereports abrt_g_settings_privatereports ++extern bool g_settings_privatereports; + + + #define load_abrt_conf abrt_load_abrt_conf +diff --git a/src/lib/abrt_conf.c b/src/lib/abrt_conf.c +index 5eb69e2..c6aba58 100644 +--- a/src/lib/abrt_conf.c ++++ b/src/lib/abrt_conf.c +@@ -27,6 +27,7 @@ bool g_settings_delete_uploaded = 0; + bool g_settings_autoreporting = 0; + char * g_settings_autoreporting_event = NULL; + bool g_settings_shortenedreporting = 0; ++bool g_settings_privatereports = true; + + void free_abrt_conf_data() + { +@@ -102,6 +103,13 @@ static void ParseCommon(map_string_t *settings, const char *conf_filename) + else + g_settings_shortenedreporting = 0; + ++ value = get_map_string_item_or_NULL(settings, "PrivateReports"); ++ if (value) ++ { ++ g_settings_privatereports = string_to_bool(value); ++ remove_map_string_item(settings, "PrivateReports"); ++ } ++ + GHashTableIter iter; + const char *name; + /*char *value; - already declared */ +diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c +index 1d45cdd..fb7750d 100644 +--- a/src/lib/hooklib.c ++++ b/src/lib/hooklib.c +@@ -410,7 +410,12 @@ char* problem_data_save(problem_data_t *pd) + { + load_abrt_conf(); + +- struct dump_dir *dd = create_dump_dir_from_problem_data(pd, g_settings_dump_location); ++ struct dump_dir *dd = NULL; ++ ++ if (g_settings_privatereports) ++ dd = create_dump_dir_from_problem_data_ext(pd, g_settings_dump_location, 0); ++ else ++ dd = create_dump_dir_from_problem_data(pd, g_settings_dump_location); + + char *problem_id = NULL; + if (dd) +diff --git a/src/plugins/abrt-dump-oops.c b/src/plugins/abrt-dump-oops.c +index 9f0dc87..05cb728 100644 +--- a/src/plugins/abrt-dump-oops.c ++++ b/src/plugins/abrt-dump-oops.c +@@ -189,6 +189,14 @@ static unsigned create_oops_dump_dirs(GList *oops_list, unsigned oops_cnt) + mode = DEFAULT_DUMP_DIR_MODE; + my_euid = geteuid(); + } ++ if (g_settings_privatereports) ++ { ++ if (world_readable_dump) ++ log("Not going to make dump directories world readable because PrivateReports is on"); ++ ++ mode = DEFAULT_DUMP_DIR_MODE; ++ my_euid = 0; ++ } + + pid_t my_pid = getpid(); + unsigned idx = 0; +diff --git a/src/plugins/abrt-dump-xorg.c b/src/plugins/abrt-dump-xorg.c +index 3500629..434dc76 100644 +--- a/src/plugins/abrt-dump-xorg.c ++++ b/src/plugins/abrt-dump-xorg.c +@@ -82,6 +82,14 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea + mode = DEFAULT_DUMP_DIR_MODE; + my_euid = geteuid(); + } ++ if (g_settings_privatereports) ++ { ++ if ((g_opts & OPT_x)) ++ log("Not going to make dump directories world readable because PrivateReports is on"); ++ ++ mode = DEFAULT_DUMP_DIR_MODE; ++ my_euid = 0; ++ } + + pid_t my_pid = getpid(); + +-- +1.8.3.1 + diff --git a/SOURCES/0101-configure-move-the-default-dump-location-to-var-spoo.patch b/SOURCES/0101-configure-move-the-default-dump-location-to-var-spoo.patch new file mode 100644 index 0000000..9554b09 --- /dev/null +++ b/SOURCES/0101-configure-move-the-default-dump-location-to-var-spoo.patch @@ -0,0 +1,47 @@ +From 7d023c32a565e83306cddf34c894477b7aaf33d1 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 20 Apr 2015 08:06:09 +0200 +Subject: [ABRT PATCH] configure: move the default dump location to /var/spool + +Signed-off-by: Jakub Filak +--- + configure.ac | 4 ++-- + src/daemon/abrt.conf | 4 ++-- + 2 files changed, 4 insertions(+), 4 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 6c6d2e8..d95fc4a 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -146,8 +146,8 @@ PROBLEMS_CONFIG_INTERFACES_DIR=${dbusinterfacedir} + + AC_ARG_WITH([defaultdumplocation], + AS_HELP_STRING([--with-defaultdumplocation=DIR], +- [Default dump location ('LOCALSTATEDIR/tmp/abrt')]), +- [], [with_defaultdumplocation=${localstatedir}/tmp/abrt]) ++ [Default dump location ('LOCALSTATEDIR/spool/abrt')]), ++ [], [with_defaultdumplocation=${localstatedir}/spool/abrt]) + AC_SUBST([DEFAULT_DUMP_LOCATION], [$with_defaultdumplocation]) + + AC_ARG_WITH(augeaslenslibdir, +diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf +index 6c0d6b0..171ee4c 100644 +--- a/src/daemon/abrt.conf ++++ b/src/daemon/abrt.conf +@@ -10,11 +10,11 @@ + MaxCrashReportsSize = 1000 + + # Specify where you want to store coredumps and all files which are needed for +-# reporting. (default:/var/tmp/abrt) ++# reporting. (default:/var/spool/abrt) + # + # Changing dump location could cause problems with SELinux. See man abrt_selinux(8). + # +-#DumpLocation = /var/tmp/abrt ++#DumpLocation = /var/spool/abrt + + # If you want to automatically clean the upload directory you have to tweak the + # selinux policy. +-- +1.8.3.1 + diff --git a/SOURCES/0103-ccpp-avoid-overriding-system-files-by-coredump.patch b/SOURCES/0103-ccpp-avoid-overriding-system-files-by-coredump.patch new file mode 100644 index 0000000..fa860fc --- /dev/null +++ b/SOURCES/0103-ccpp-avoid-overriding-system-files-by-coredump.patch @@ -0,0 +1,28 @@ +From cdb4c5b0855d910132e61d71afbd445b0271fcb4 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 21 Apr 2015 07:54:17 +0200 +Subject: [ABRT PATCH] ccpp: avoid overriding system files by coredump + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 3a6d002..02f15d5 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -544,7 +544,7 @@ static bool dump_fd_info(const char *dest_filename, char *source_filename, int s + /* Like xopen, but on error, unlocks and deletes dd and user core */ + static int create_or_die(const char *filename) + { +- int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, DEFAULT_DUMP_DIR_MODE); ++ int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, DEFAULT_DUMP_DIR_MODE); + if (fd >= 0) + { + IGNORE_RESULT(fchown(fd, dd->dd_uid, dd->dd_gid)); +-- +1.8.3.1 + diff --git a/SOURCES/0105-daemon-use-libreport-s-function-checking-file-name.patch b/SOURCES/0105-daemon-use-libreport-s-function-checking-file-name.patch new file mode 100644 index 0000000..04df87f --- /dev/null +++ b/SOURCES/0105-daemon-use-libreport-s-function-checking-file-name.patch @@ -0,0 +1,54 @@ +From c796c76341ee846cfb897ed645bac211d7d0a932 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 13:12:01 +0200 +Subject: [ABRT PATCH] daemon: use libreport's function checking file name + +Move the functions to libreport because we need the same functionality +there too. + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 18 +----------------- + 1 file changed, 1 insertion(+), 17 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 5789075..4d486d4 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -476,22 +476,6 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid) + exit(0); + } + +-/* Checks if a string contains only printable characters. */ +-static gboolean printable_str(const char *str) +-{ +- do { +- if ((unsigned char)(*str) < ' ' || *str == 0x7f) +- return FALSE; +- str++; +- } while (*str); +- return TRUE; +-} +- +-static gboolean is_correct_filename(const char *value) +-{ +- return printable_str(value) && !strchr(value, '/') && !strchr(value, '.'); +-} +- + static gboolean key_value_ok(gchar *key, gchar *value) + { + char *i; +@@ -510,7 +494,7 @@ static gboolean key_value_ok(gchar *key, gchar *value) + || strcmp(key, FILENAME_TYPE) == 0 + ) + { +- if (!is_correct_filename(value)) ++ if (!str_is_correct_filename(value)) + { + error_msg("Value of '%s' ('%s') is not a valid directory name", + key, value); +-- +1.8.3.1 + diff --git a/SOURCES/0106-lib-add-functions-validating-dump-dir.patch b/SOURCES/0106-lib-add-functions-validating-dump-dir.patch new file mode 100644 index 0000000..83e5647 --- /dev/null +++ b/SOURCES/0106-lib-add-functions-validating-dump-dir.patch @@ -0,0 +1,287 @@ +From b7f8bd20b7fb5b72f003ae3fa647c1d75f4218b7 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 14:40:18 +0200 +Subject: [ABRT PATCH] lib: add functions validating dump dir + +Move the code from abrt-server to shared library and fix the condition +validating dump dir's path. + +As of now, abrt is allowed to process only direct sub-directories of the +dump locations. + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 42 ++++++------------------ + src/include/libabrt.h | 4 +++ + src/lib/hooklib.c | 56 +++++++++++++++++++++++++++++++ + tests/Makefile.am | 3 +- + tests/hooklib.at | 85 ++++++++++++++++++++++++++++++++++++++++++++++++ + tests/testsuite.at | 1 + + 6 files changed, 158 insertions(+), 33 deletions(-) + create mode 100644 tests/hooklib.at + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 4d486d4..1030461 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -76,20 +76,6 @@ static unsigned total_bytes_read = 0; + static uid_t client_uid = (uid_t)-1L; + + +-static bool dir_is_in_dump_location(const char *dump_dir_name) +-{ +- unsigned len = strlen(g_settings_dump_location); +- +- if (strncmp(dump_dir_name, g_settings_dump_location, len) == 0 +- && dump_dir_name[len] == '/' +- /* must not contain "/." anywhere (IOW: disallow ".." component) */ +- && !strstr(dump_dir_name + len, "/.") +- ) { +- return 1; +- } +- return 0; +-} +- + /* Remove dump dir */ + static int delete_path(const char *dump_dir_name) + { +@@ -100,6 +86,11 @@ static int delete_path(const char *dump_dir_name) + error_msg("Bad problem directory name '%s', should start with: '%s'", dump_dir_name, g_settings_dump_location); + return 400; /* Bad Request */ + } ++ if (!dir_has_correct_permissions(dump_dir_name)) ++ { ++ error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dump_dir_name); ++ return 400; /* */ ++ } + if (!dump_dir_accessible_by_uid(dump_dir_name, client_uid)) + { + if (errno == ENOTDIR) +@@ -154,26 +145,13 @@ static int run_post_create(const char *dirname) + error_msg("Bad problem directory name '%s', should start with: '%s'", dirname, g_settings_dump_location); + return 400; /* Bad Request */ + } ++ if (!dir_has_correct_permissions(dirname)) ++ { ++ error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dirname); ++ return 400; /* */ ++ } + if (g_settings_privatereports) + { +- struct stat statbuf; +- if (lstat(dirname, &statbuf) != 0 || !S_ISDIR(statbuf.st_mode)) +- { +- error_msg("Path '%s' isn't directory", dirname); +- return 404; /* Not Found */ +- } +- /* Get ABRT's group gid */ +- struct group *gr = getgrnam("abrt"); +- if (!gr) +- { +- error_msg("Group 'abrt' does not exist"); +- return 500; +- } +- if (statbuf.st_uid != 0 || !(statbuf.st_gid == 0 || statbuf.st_gid == gr->gr_gid) || statbuf.st_mode & 07) +- { +- error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dirname); +- return 403; +- } + struct dump_dir *dd = dd_opendir(dirname, DD_OPEN_READONLY); + const bool complete = dd && problem_dump_dir_is_complete(dd); + dd_close(dd); +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 0320c5b..5bf2397 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -47,6 +47,10 @@ char *run_unstrip_n(const char *dump_dir_name, unsigned timeout_sec); + #define get_backtrace abrt_get_backtrace + char *get_backtrace(const char *dump_dir_name, unsigned timeout_sec, const char *debuginfo_dirs); + ++#define dir_is_in_dump_location abrt_dir_is_in_dump_location ++bool dir_is_in_dump_location(const char *dir_name); ++#define dir_has_correct_permissions abrt_dir_has_correct_permissions ++bool dir_has_correct_permissions(const char *dir_name); + + #define g_settings_nMaxCrashReportsSize abrt_g_settings_nMaxCrashReportsSize + extern unsigned int g_settings_nMaxCrashReportsSize; +diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c +index fb7750d..4b20025 100644 +--- a/src/lib/hooklib.c ++++ b/src/lib/hooklib.c +@@ -427,3 +427,59 @@ char* problem_data_save(problem_data_t *pd) + log_info("problem id: '%s'", problem_id); + return problem_id; + } ++ ++bool dir_is_in_dump_location(const char *dir_name) ++{ ++ unsigned len = strlen(g_settings_dump_location); ++ ++ /* The path must start with "g_settings_dump_location" */ ++ if (strncmp(dir_name, g_settings_dump_location, len) != 0) ++ { ++ log_debug("Bad parent directory: '%s' not in '%s'", g_settings_dump_location, dir_name); ++ return false; ++ } ++ ++ /* and must be a sub-directory of the g_settings_dump_location dir */ ++ const char *base_name = dir_name + len; ++ while (*base_name && *base_name == '/') ++ ++base_name; ++ ++ if (*(base_name - 1) != '/' || !str_is_correct_filename(base_name)) ++ { ++ log_debug("Invalid dump directory name: '%s'", base_name); ++ return false; ++ } ++ ++ /* and we are sure it is a directory */ ++ struct stat sb; ++ if (lstat(dir_name, &sb) < 0) ++ { ++ VERB2 perror_msg("stat('%s')", dir_name); ++ return errno== ENOENT; ++ } ++ ++ return S_ISDIR(sb.st_mode); ++} ++ ++bool dir_has_correct_permissions(const char *dir_name) ++{ ++ if (g_settings_privatereports) ++ { ++ struct stat statbuf; ++ if (lstat(dir_name, &statbuf) != 0 || !S_ISDIR(statbuf.st_mode)) ++ { ++ error_msg("Path '%s' isn't directory", dir_name); ++ return false; ++ } ++ /* Get ABRT's group gid */ ++ struct group *gr = getgrnam("abrt"); ++ if (!gr) ++ { ++ error_msg("Group 'abrt' does not exist"); ++ return false; ++ } ++ if (statbuf.st_uid != 0 || !(statbuf.st_gid == 0 || statbuf.st_gid == gr->gr_gid) || statbuf.st_mode & 07) ++ return false; ++ } ++ return true; ++} +diff --git a/tests/Makefile.am b/tests/Makefile.am +index 5ef08a0..416f579 100644 +--- a/tests/Makefile.am ++++ b/tests/Makefile.am +@@ -29,7 +29,8 @@ TESTSUITE_AT = \ + testsuite.at \ + pyhook.at \ + koops-parser.at \ +- ignored_problems.at ++ ignored_problems.at \ ++ hooklib.at + + EXTRA_DIST += $(TESTSUITE_AT) + TESTSUITE = $(srcdir)/testsuite +diff --git a/tests/hooklib.at b/tests/hooklib.at +new file mode 100644 +index 0000000..70631c6 +--- /dev/null ++++ b/tests/hooklib.at +@@ -0,0 +1,85 @@ ++# -*- Autotest -*- ++ ++AT_BANNER([hooklib]) ++ ++AT_TESTFUN([dir_is_in_dump_location], ++[[ ++#include "libabrt.h" ++#include ++ ++void test(char *name, bool expected) ++{ ++ if (dir_is_in_dump_location(name) != expected) ++ { ++ fprintf(stderr, "Bad: %s", name); ++ abort(); ++ } ++ ++ free(name); ++} ++ ++int main(void) ++{ ++ g_verbose = 3; ++ load_abrt_conf(); ++ ++ g_verbose = 3; ++ ++ char *name; ++ ++ assert(dir_is_in_dump_location("/") == false); ++ ++ asprintf(&name, "%s", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s..evil", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s///", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/.", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s///.", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/./", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/.///", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/..", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s///..", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/../", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/..///", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/good/../../../evil", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/good..still", g_settings_dump_location); ++ test(name, true); ++ ++ asprintf(&name, "%s/good.new", g_settings_dump_location); ++ test(name, true); ++ ++ asprintf(&name, "%s/.meta", g_settings_dump_location); ++ test(name, true); ++ ++ asprintf(&name, "%s/..data", g_settings_dump_location); ++ test(name, true); ++ ++ return 0; ++} ++]]) +diff --git a/tests/testsuite.at b/tests/testsuite.at +index b8f363d..765de2a 100644 +--- a/tests/testsuite.at ++++ b/tests/testsuite.at +@@ -4,3 +4,4 @@ + m4_include([koops-parser.at]) + m4_include([pyhook.at]) + m4_include([ignored_problems.at]) ++m4_include([hooklib.at]) +-- +1.8.3.1 + diff --git a/SOURCES/0107-dbus-process-only-valid-sub-directories-of-the-dump-.patch b/SOURCES/0107-dbus-process-only-valid-sub-directories-of-the-dump-.patch new file mode 100644 index 0000000..f1fb687 --- /dev/null +++ b/SOURCES/0107-dbus-process-only-valid-sub-directories-of-the-dump-.patch @@ -0,0 +1,70 @@ +From 6e811d78e2719988ae291181f5b133af32ce62d8 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 14:46:27 +0200 +Subject: [ABRT PATCH] dbus: process only valid sub-directories of the dump + location + +Must have correct rights and must be a direct sub-directory of the dump +location. + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 36 ++++++++++++++++++++++++++---------- + 1 file changed, 26 insertions(+), 10 deletions(-) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 308a9af..7400dff 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -132,18 +132,34 @@ static uid_t get_caller_uid(GDBusConnection *connection, GDBusMethodInvocation * + return caller_uid; + } + +-static bool allowed_problem_dir(const char *dir_name) ++bool allowed_problem_dir(const char *dir_name) + { +-//HACK HACK HACK! Disabled for now until we fix clients (abrt-gui) to not pass /home/user/.cache/abrt/spool ++ if (!dir_is_in_dump_location(dir_name)) ++ { ++ error_msg("Bad problem directory name '%s', should start with: '%s'", dir_name, g_settings_dump_location); ++ return false; ++ } ++ ++ /* We cannot test correct permissions yet because we still need to chown ++ * dump directories before reporting and Chowing changes the file owner to ++ * the reporter, which causes this test to fail and prevents users from ++ * getting problem data after reporting it. ++ * ++ * Fortunately, libreport has been hardened against hard link and symbolic ++ * link attacks and refuses to work with such files, so this test isn't ++ * really necessary, however, we will use it once we get rid of the ++ * chowning files. ++ * ++ * abrt-server refuses to run post-create on directories that have ++ * incorrect owner (not "root:(abrt|root)"), incorrect permissions (other ++ * bits are not 0) and are complete (post-create finished). So, there is no ++ * way to run security sensitive event scripts (post-create) on crafted ++ * problem directories. ++ */ + #if 0 +- unsigned len = strlen(g_settings_dump_location); +- +- /* If doesn't start with "g_settings_dump_location[/]"... */ +- if (strncmp(dir_name, g_settings_dump_location, len) != 0 +- || (dir_name[len] != '/' && dir_name[len] != '\0') +- /* or contains "/." anywhere (-> might contain ".." component) */ +- || strstr(dir_name + len, "/.") +- ) { ++ if (!dir_has_correct_permissions(dir_name)) ++ { ++ error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dir_name); + return false; + } + #endif +-- +1.8.3.1 + diff --git a/SOURCES/0108-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch b/SOURCES/0108-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch new file mode 100644 index 0000000..c90d1e1 --- /dev/null +++ b/SOURCES/0108-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch @@ -0,0 +1,223 @@ +From 7814554e0827ece778ca88fd90832bd4d05520b1 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 24 Apr 2015 13:48:32 +0200 +Subject: [ABRT PATCH] dbus: avoid race-conditions in tests for dum dir + availability + +Florian Weimer + + dump_dir_accessible_by_uid() is fundamentally insecure because it + opens up a classic time-of-check-time-of-use race between this + function and and dd_opendir(). + +Related: #1214745 + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 66 ++++++++++++++++++++++++++++++++++++++++++++------- + src/lib/problem_api.c | 15 ++++++++++-- + 2 files changed, 71 insertions(+), 10 deletions(-) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 7400dff..9e1844a 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -245,7 +245,15 @@ static struct dump_dir *open_directory_for_modification_of_element( + } + } + +- if (!dump_dir_accessible_by_uid(problem_id, caller_uid)) ++ int dir_fd = dd_openfd(problem_id); ++ if (dir_fd < 0) ++ { ++ perror_msg("can't open problem directory '%s'", problem_id); ++ return_InvalidProblemDir_error(invocation, problem_id); ++ return NULL; ++ } ++ ++ if (!fdump_dir_accessible_by_uid(dir_fd, caller_uid)) + { + if (errno == ENOTDIR) + { +@@ -260,10 +268,11 @@ static struct dump_dir *open_directory_for_modification_of_element( + _("Not Authorized")); + } + ++ close(dir_fd); + return NULL; + } + +- struct dump_dir *dd = dd_opendir(problem_id, /* flags : */ 0); ++ struct dump_dir *dd = dd_fdopendir(dir_fd, problem_id, /* flags : */ 0); + if (!dd) + { /* This should not happen because of the access check above */ + log_notice("Can't access the problem '%s' for modification", problem_id); +@@ -429,7 +438,15 @@ static void handle_method_call(GDBusConnection *connection, + return; + } + +- int ddstat = dump_dir_stat_for_uid(problem_dir, caller_uid); ++ int dir_fd = dd_openfd(problem_dir); ++ if (dir_fd < 0) ++ { ++ perror_msg("can't open problem directory '%s'", problem_dir); ++ return_InvalidProblemDir_error(invocation, problem_dir); ++ return; ++ } ++ ++ int ddstat = fdump_dir_stat_for_uid(dir_fd, caller_uid); + if (ddstat < 0) + { + if (errno == ENOTDIR) +@@ -443,6 +460,7 @@ static void handle_method_call(GDBusConnection *connection, + + return_InvalidProblemDir_error(invocation, problem_dir); + ++ close(dir_fd); + return; + } + +@@ -450,6 +468,7 @@ static void handle_method_call(GDBusConnection *connection, + { //caller seems to be in group with access to this dir, so no action needed + log_notice("caller has access to the requested directory %s", problem_dir); + g_dbus_method_invocation_return_value(invocation, NULL); ++ close(dir_fd); + return; + } + +@@ -460,10 +479,11 @@ static void handle_method_call(GDBusConnection *connection, + g_dbus_method_invocation_return_dbus_error(invocation, + "org.freedesktop.problems.AuthFailure", + _("Not Authorized")); ++ close(dir_fd); + return; + } + +- struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES); ++ struct dump_dir *dd = dd_fdopendir(dir_fd, problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES); + if (!dd) + { + return_InvalidProblemDir_error(invocation, problem_dir); +@@ -497,12 +517,21 @@ static void handle_method_call(GDBusConnection *connection, + return; + } + +- if (!dump_dir_accessible_by_uid(problem_dir, caller_uid)) ++ int dir_fd = dd_openfd(problem_dir); ++ if (dir_fd < 0) ++ { ++ perror_msg("can't open problem directory '%s'", problem_dir); ++ return_InvalidProblemDir_error(invocation, problem_dir); ++ return; ++ } ++ ++ if (!fdump_dir_accessible_by_uid(dir_fd, caller_uid)) + { + if (errno == ENOTDIR) + { + log_notice("Requested directory does not exist '%s'", problem_dir); + return_InvalidProblemDir_error(invocation, problem_dir); ++ close(dir_fd); + return; + } + +@@ -512,11 +541,12 @@ static void handle_method_call(GDBusConnection *connection, + g_dbus_method_invocation_return_dbus_error(invocation, + "org.freedesktop.problems.AuthFailure", + _("Not Authorized")); ++ close(dir_fd); + return; + } + } + +- struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES); ++ struct dump_dir *dd = dd_fdopendir(dir_fd, problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES); + if (!dd) + { + return_InvalidProblemDir_error(invocation, problem_dir); +@@ -677,20 +707,40 @@ static void handle_method_call(GDBusConnection *connection, + for (GList *l = problem_dirs; l; l = l->next) + { + const char *dir_name = (const char*)l->data; +- if (!dump_dir_accessible_by_uid(dir_name, caller_uid)) ++ ++ int dir_fd = dd_openfd(dir_name); ++ if (dir_fd < 0) ++ { ++ perror_msg("can't open problem directory '%s'", dir_name); ++ return_InvalidProblemDir_error(invocation, dir_name); ++ return; ++ } ++ ++ if (!fdump_dir_accessible_by_uid(dir_fd, caller_uid)) + { + if (errno == ENOTDIR) + { + log_notice("Requested directory does not exist '%s'", dir_name); ++ close(dir_fd); + continue; + } + + if (polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) + { // if user didn't provide correct credentials, just move to the next dir ++ close(dir_fd); + continue; + } + } +- delete_dump_dir(dir_name); ++ ++ struct dump_dir *dd = dd_fdopendir(dir_fd, dir_name, /*flags:*/ 0); ++ if (dd) ++ { ++ if (dd_delete(dd) != 0) ++ { ++ error_msg("Failed to delete problem directory '%s'", dir_name); ++ dd_close(dd); ++ } ++ } + } + + g_dbus_method_invocation_return_value(invocation, NULL); +diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c +index c2b4b1c..b343882 100644 +--- a/src/lib/problem_api.c ++++ b/src/lib/problem_api.c +@@ -46,7 +46,15 @@ int for_each_problem_in_dir(const char *path, + continue; /* skip "." and ".." */ + + char *full_name = concat_path_file(path, dent->d_name); +- if (caller_uid == -1 || dump_dir_accessible_by_uid(full_name, caller_uid)) ++ ++ int dir_fd = dd_openfd(full_name); ++ if (dir_fd < 0) ++ { ++ VERB2 perror_msg("can't open problem directory '%s'", full_name); ++ continue; ++ } ++ ++ if (caller_uid == -1 || fdump_dir_accessible_by_uid(dir_fd, caller_uid)) + { + /* Silently ignore *any* errors, not only EACCES. + * We saw "lock file is locked by process PID" error +@@ -54,7 +62,7 @@ int for_each_problem_in_dir(const char *path, + */ + int sv_logmode = logmode; + logmode = 0; +- struct dump_dir *dd = dd_opendir(full_name, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES | DD_DONT_WAIT_FOR_LOCK); ++ struct dump_dir *dd = dd_fdopendir(dir_fd, full_name, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES | DD_DONT_WAIT_FOR_LOCK); + logmode = sv_logmode; + if (dd) + { +@@ -62,6 +70,9 @@ int for_each_problem_in_dir(const char *path, + dd_close(dd); + } + } ++ else ++ close(dir_fd); ++ + free(full_name); + if (brk) + break; +-- +1.8.3.1 + diff --git a/SOURCES/0109-dbus-report-invalid-element-names.patch b/SOURCES/0109-dbus-report-invalid-element-names.patch new file mode 100644 index 0000000..d0a9394 --- /dev/null +++ b/SOURCES/0109-dbus-report-invalid-element-names.patch @@ -0,0 +1,49 @@ +From f3c2a6af3455b2882e28570e8a04f1c2d4500d5b Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 27 Apr 2015 07:52:00 +0200 +Subject: [ABRT PATCH] dbus: report invalid element names + +Return D-Bus error in case of invalid problem element name. + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 14 +++++++++++++- + 1 file changed, 13 insertions(+), 1 deletion(-) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 9e1844a..6de15e9 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -599,7 +599,7 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s&s)", &problem_id, &element, &value); + +- if (element == NULL || element[0] == '\0' || strlen(element) > 64) ++ if (!str_is_correct_filename(element)) + { + log_notice("'%s' is not a valid element name of '%s'", element, problem_id); + char *error = xasprintf(_("'%s' is not a valid element name"), element); +@@ -658,6 +658,18 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + ++ if (!str_is_correct_filename(element)) ++ { ++ log_notice("'%s' is not a valid element name of '%s'", element, problem_id); ++ char *error = xasprintf(_("'%s' is not a valid element name"), element); ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.InvalidElement", ++ error); ++ ++ free(error); ++ return; ++ } ++ + struct dump_dir *dd = open_directory_for_modification_of_element( + invocation, caller_uid, problem_id, element); + if (!dd) +-- +1.8.3.1 + diff --git a/SOURCES/0110-a-a-i-d-t-a-cache-sanitize-arguments.patch b/SOURCES/0110-a-a-i-d-t-a-cache-sanitize-arguments.patch new file mode 100644 index 0000000..0939cc6 --- /dev/null +++ b/SOURCES/0110-a-a-i-d-t-a-cache-sanitize-arguments.patch @@ -0,0 +1,170 @@ +From 9943a77bca37a0829ccd3784d1dfab37f8c24e7b Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 29 Apr 2015 13:57:39 +0200 +Subject: [ABRT PATCH] a-a-i-d-t-a-cache: sanitize arguments + +Parse command lines arguments and use them to create new arguments for +exec(). No black listing algorithm would be safe enough. The only +allowed arguments are the following: +* v - verbose +* y - noninteractive +* repo - enable only repositories whose names match the pattern +* exact - download packages for the specified files +* ids - passed as magic proc fd path to the wrapped executable + +The wrapper opens the list of needed build ids passes /proc/self/fd/[fd] +to the wrapped process. This allows us to open the file with caller's +UID/GID in order to avoid information disclosures. + +Forbidden arguments: +* cache - allows regular users to create a user writable dump directory +* tmpdir - the same as above +* size_mb - no need to allow users to fiddle with the cache size + +Related: #1216962 + +Signed-off-by: Jakub Filak +--- + po/POTFILES.in | 1 + + .../abrt-action-install-debuginfo-to-abrt-cache.c | 106 ++++++++++++++++----- + 2 files changed, 85 insertions(+), 22 deletions(-) + +diff --git a/po/POTFILES.in b/po/POTFILES.in +index cbe89fa..1248c46 100644 +--- a/po/POTFILES.in ++++ b/po/POTFILES.in +@@ -31,6 +31,7 @@ src/plugins/abrt-action-check-oops-for-hw-error.in + src/plugins/abrt-action-generate-backtrace.c + src/plugins/abrt-action-generate-core-backtrace.c + src/plugins/abrt-action-install-debuginfo.in ++src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c + src/plugins/abrt-action-perform-ccpp-analysis.in + src/plugins/abrt-action-trim-files.c + src/plugins/abrt-action-ureport +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 e0eccc0..eb2f7c5 100644 +--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c ++++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +@@ -29,28 +29,90 @@ + */ + int main(int argc, char **argv) + { +- /* +- * We disallow passing of arguments which point to writable dirs +- * and other files possibly not accessible to calling user. +- * This way, the script will always use default values for these arguments. +- */ +- char **pp = argv; +- char *arg; +- while ((arg = *++pp) != NULL) ++ /* I18n */ ++ setlocale(LC_ALL, ""); ++#if ENABLE_NLS ++ bindtextdomain(PACKAGE, LOCALEDIR); ++ textdomain(PACKAGE); ++#endif ++ ++ abrt_init(argv); ++ ++ /* Can't keep these strings/structs static: _() doesn't support that */ ++ const char *program_usage_string = _( ++ "& [-y] [-i BUILD_IDS_FILE|-i -] [-e PATH[:PATH]...]\n" ++ "\t[-r REPO]\n" ++ "\n" ++ "Installs debuginfo packages for all build-ids listed in BUILD_IDS_FILE to\n" ++ "ABRT system cache." ++ ); ++ ++ enum { ++ OPT_v = 1 << 0, ++ OPT_y = 1 << 1, ++ OPT_i = 1 << 2, ++ OPT_e = 1 << 3, ++ OPT_r = 1 << 4, ++ OPT_s = 1 << 5, ++ }; ++ ++ const char *build_ids = "build_ids"; ++ const char *exact = NULL; ++ const char *repo = NULL; ++ const char *size_mb = NULL; ++ ++ struct options program_options[] = { ++ OPT__VERBOSE(&g_verbose), ++ OPT_BOOL ('y', "yes", NULL, _("Noninteractive, assume 'Yes' to all questions")), ++ OPT_STRING('i', "ids", &build_ids, "BUILD_IDS_FILE", _("- means STDIN, default: build_ids")), ++ OPT_STRING('e', "exact", &exact, "EXACT", _("Download only specified files")), ++ OPT_STRING('r', "repo", &repo, "REPO", _("Pattern to use when searching for repos, default: *debug*")), ++ OPT_STRING('s', "size_mb", &size_mb, "SIZE_MB", _("Ignored option")), ++ OPT_END() ++ }; ++ const unsigned opts = parse_opts(argc, argv, program_options, program_usage_string); ++ ++ /* We need to open the build ids file under the caller's UID/GID to avoid ++ * information disclosures when reading files with changed UID. ++ * Unfortunately, we cannot replace STDIN with the new fd because ABRT uses ++ * STDIN to communicate with the caller. So, the following code opens a ++ * dummy file descriptor to the build ids file and passes the new fd's proc ++ * path to the wrapped program in the ids argument. ++ * The new fd remains opened, the OS will close it for us. */ ++ char *build_ids_self_fd = NULL; ++ if (strcmp("-", build_ids) != 0) ++ { ++ const int build_ids_fd = open(build_ids, O_RDONLY); ++ if (build_ids_fd < 0) ++ perror_msg_and_die("Failed to open file '%s'", build_ids); ++ ++ /* 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]; + { +- /* Allow taking ids from stdin */ +- if (strcmp(arg, "--ids=-") == 0) +- continue; +- +- if (strncmp(arg, "--exact", 7) == 0) +- continue; +- +- if (strncmp(arg, "--cache", 7) == 0) +- error_msg_and_die("bad option %s", arg); +- if (strncmp(arg, "--tmpdir", 8) == 0) +- error_msg_and_die("bad option %s", arg); +- if (strncmp(arg, "--ids", 5) == 0) +- error_msg_and_die("bad option %s", arg); ++ const char *verbs[] = { "", "-v", "-vv", "-vvv" }; ++ unsigned i = 0; ++ args[i++] = EXECUTABLE; ++ args[i++] = "--ids"; ++ args[i++] = (build_ids_self_fd != NULL) ? build_ids_self_fd : "-"; ++ args[i++] = verbs[g_verbose <= 3 ? g_verbose : 3]; ++ if ((opts & OPT_y)) ++ args[i++] = "-y"; ++ if ((opts & OPT_e)) ++ { ++ args[i++] = "--exact"; ++ args[i++] = exact; ++ } ++ if ((opts & OPT_r)) ++ { ++ args[i++] = "--repo"; ++ args[i++] = repo; ++ } ++ args[i++] = "--"; ++ args[i] = NULL; + } + + /* Switch real user/group to effective ones. +@@ -122,6 +184,6 @@ int main(int argc, char **argv) + putenv(path_env); + } + +- execvp(EXECUTABLE, argv); ++ execvp(EXECUTABLE, (char **)args); + error_msg_and_die("Can't execute %s", EXECUTABLE); + } +-- +1.8.3.1 + diff --git a/SOURCES/0111-a-a-i-d-t-a-cache-sanitize-umask.patch b/SOURCES/0111-a-a-i-d-t-a-cache-sanitize-umask.patch new file mode 100644 index 0000000..2037588 --- /dev/null +++ b/SOURCES/0111-a-a-i-d-t-a-cache-sanitize-umask.patch @@ -0,0 +1,31 @@ +From 9a4100678fea4d60ec93d35f4c5de2e9ad054f3a Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 29 Apr 2015 14:13:57 +0200 +Subject: [ABRT PATCH] a-a-i-d-t-a-cache: sanitize umask + +We cannot trust anything when running suided program. + +Related: #1216962 + +Signed-off-by: Jakub Filak +--- + src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c | 3 +++ + 1 file changed, 3 insertions(+) + +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 eb2f7c5..cd9ee7a 100644 +--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c ++++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +@@ -182,6 +182,9 @@ int main(int argc, char **argv) + if (u != 0) + strcpy(path_env, "PATH=/usr/bin:/bin:"BIN_DIR); + putenv(path_env); ++ ++ /* Use safe umask */ ++ umask(0022); + } + + execvp(EXECUTABLE, (char **)args); +-- +1.8.3.1 + diff --git a/SOURCES/0112-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch b/SOURCES/0112-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch new file mode 100644 index 0000000..8b89567 --- /dev/null +++ b/SOURCES/0112-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch @@ -0,0 +1,102 @@ +From 7269a2cc88735aee0d1fa62491b9efe73ab5c6e8 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 4 May 2015 13:23:43 +0200 +Subject: [ABRT PATCH] ccpp: revert the UID/GID changes if user core fails + +Thanks Florian Weimer + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 58 ++++++++++++++++++++++++++++------------------ + 1 file changed, 36 insertions(+), 22 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 02f15d5..fdd9b06 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -351,9 +351,6 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + return -1; + } + +- xsetegid(get_fsgid()); +- xseteuid(fsuid); +- + if (strcmp(core_basename, "core") == 0) + { + /* Mimic "core.PID" if requested */ +@@ -446,36 +443,53 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).) + */ + +- /* Set SELinux context like kernel when creating core dump file */ +- if (newcon != NULL && setfscreatecon_raw(newcon) < 0) +- { +- perror_msg("setfscreatecon_raw(%s)", newcon); +- return -1; +- } ++ int user_core_fd = -1; ++ int selinux_fail = 1; + +- 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 = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ ++ /* ++ * These calls must be reverted as soon as possible. ++ */ ++ xsetegid(get_fsgid()); ++ xseteuid(fsuid); + +- if (newcon != NULL && setfscreatecon_raw(NULL) < 0) ++ /* Set SELinux context like kernel when creating core dump file. ++ * This condition is TRUE if */ ++ if (/* SELinux is disabled */ newcon == NULL ++ || /* or the call succeeds */ setfscreatecon_raw(newcon) >= 0) + { +- error_msg("setfscreatecon_raw(NULL)"); +- goto user_core_fail; ++ /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */ ++ user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ ++ ++ /* Do the error check here and print the error message in order to ++ * avoid interference in 'errno' usage caused by SELinux functions */ ++ if (user_core_fd < 0) ++ perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd); ++ ++ /* Fail if SELinux is enabled and the call fails */ ++ if (newcon != NULL && setfscreatecon_raw(NULL) < 0) ++ perror_msg("setfscreatecon_raw(NULL)"); ++ else ++ selinux_fail = 0; + } ++ else ++ perror_msg("setfscreatecon_raw(%s)", newcon); + ++ /* ++ * DON'T JUMP OVER THIS REVERT OF THE UID/GID CHANGES ++ */ + xsetegid(0); + xseteuid(0); +- if (user_core_fd < 0 +- || fstat(user_core_fd, &sb) != 0 ++ ++ if (user_core_fd < 0 || selinux_fail) ++ goto user_core_fail; ++ ++ struct stat sb; ++ if (fstat(user_core_fd, &sb) != 0 + || !S_ISREG(sb.st_mode) + || sb.st_nlink != 1 + || sb.st_uid != fsuid + ) { +- if (user_core_fd < 0) +- perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd); +- else +- perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid); ++ perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid); + goto user_core_fail; + } + if (ftruncate(user_core_fd, 0) != 0) { +-- +1.8.3.1 + diff --git a/SOURCES/0113-upload-validate-and-sanitize-uploaded-dump-directori.patch b/SOURCES/0113-upload-validate-and-sanitize-uploaded-dump-directori.patch new file mode 100644 index 0000000..57b917a --- /dev/null +++ b/SOURCES/0113-upload-validate-and-sanitize-uploaded-dump-directori.patch @@ -0,0 +1,142 @@ +From a4e47c753e9d7988f4d938ed2e0fd690a909ce68 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 20 Apr 2015 15:15:40 +0200 +Subject: [ABRT PATCH] upload: validate and sanitize uploaded dump directories + +It was discovered that, when moving problem reports from +/var/spool/abrt-upload to /var/spool/abrt or /var/tmp/abrt, +abrt-handle-upload does not verify that the new problem directory +has appropriate permissions and does not contain symbolic links. A +crafted problem report exposes other parts of abrt to attack, and +the abrt-handle-upload script allows to overwrite arbitrary files. + +Acknowledgement: + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Related: #1212953 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-handle-upload.in | 78 +++++++++++++++++++++++++++++++++++----- + 1 file changed, 70 insertions(+), 8 deletions(-) + +diff --git a/src/daemon/abrt-handle-upload.in b/src/daemon/abrt-handle-upload.in +index dbc4534..7720da4 100755 +--- a/src/daemon/abrt-handle-upload.in ++++ b/src/daemon/abrt-handle-upload.in +@@ -10,6 +10,7 @@ import getopt + import tempfile + import shutil + import datetime ++import grp + + from reportclient import set_verbosity, error_msg_and_die, error_msg, log + +@@ -36,12 +37,77 @@ def init_gettext(): + + import problem + +-def write_str_to(filename, s): +- fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IROTH) ++def write_str_to(filename, s, uid, gid, mode): ++ fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode) + if fd >= 0: ++ os.fchown(fd, uid, gid) + os.write(fd, s) + os.close(fd) + ++ ++def validate_transform_move_and_notify(uploaded_dir_path, problem_dir_path, dest=None): ++ fsuid = 0 ++ fsgid = 0 ++ ++ try: ++ gabrt = grp.getgrnam("abrt") ++ fsgid = gabrt.gr_gid ++ except KeyError as ex: ++ error_msg("Failed to get GID of 'abrt' (using 0 instead): {0}'".format(str(ex))) ++ ++ try: ++ # give the uploaded directory to 'root:abrt' or 'root:root' ++ os.chown(uploaded_dir_path, fsuid, fsgid) ++ # set the right permissions for this machine ++ # (allow the owner and the group to access problem elements, ++ # the default dump dir mode lacks x bit for both) ++ os.chmod(uploaded_dir_path, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IXUSR | stat.S_IXGRP) ++ ++ # sanitize problem elements ++ for item in os.listdir(uploaded_dir_path): ++ apath = os.path.join(uploaded_dir_path, item) ++ if os.path.islink(apath): ++ # remove symbolic links ++ os.remove(apath) ++ elif os.path.isdir(apath): ++ # remove directories ++ shutil.rmtree(apath) ++ elif os.path.isfile(apath): ++ # set file ownership to 'root:abrt' or 'root:root' ++ os.chown(apath, fsuid, fsgid) ++ # set the right file permissions for this machine ++ os.chmod(apath, @DEFAULT_DUMP_DIR_MODE@) ++ else: ++ # remove things that are neither files, symlinks nor directories ++ os.remove(apath) ++ except OSError as ex: ++ error_msg("Removing uploaded dir '{0}': '{1}'".format(uploaded_dir_path, str(ex))) ++ try: ++ shutil.rmtree(uploaded_dir_path) ++ except OSError as ex2: ++ error_msg_and_die("Failed to clean up dir '{0}': '{1}'".format(uploaded_dir_path, str(ex2))) ++ return ++ ++ # overwrite remote if it exists ++ remote_path = os.path.join(uploaded_dir_path, "remote") ++ write_str_to(remote_path, "1", fsuid, fsgid, @DEFAULT_DUMP_DIR_MODE@) ++ ++ # abrtd would increment count value and abrt-server refuses to process ++ # problem directories containing 'count' element when PrivateReports is on. ++ count_path = os.path.join(uploaded_dir_path, "count") ++ if os.path.exists(count_path): ++ # overwrite remote_count if it exists ++ remote_count_path = os.path.join(uploaded_dir_path, "remote_count") ++ os.rename(count_path, remote_count_path) ++ ++ if not dest: ++ dest = problem_dir_path ++ ++ shutil.move(uploaded_dir_path, dest) ++ ++ problem.notify_new_path(problem_dir_path) ++ ++ + if __name__ == "__main__": + + # Helper: exit with cleanup +@@ -177,21 +243,17 @@ if __name__ == "__main__": + # or one or more complete problem data directories. + # Checking second possibility first. + if os.path.exists(tempdir+"/analyzer") and os.path.exists(tempdir+"/time"): +- write_str_to(tempdir+"/remote", "1") +- shutil.move(tempdir, abrt_dir) +- problem.notify_new_path(abrt_dir+"/"+os.path.basename(tempdir)) ++ validate_transform_move_and_notify(tempdir, abrt_dir+"/"+os.path.basename(tempdir), dest=abrt_dir) + else: + for d in os.listdir(tempdir): + if not os.path.isdir(tempdir+"/"+d): + continue +- write_str_to(tempdir+"/"+d+"/remote", "1") + dst = abrt_dir+"/"+d + if os.path.exists(dst): + dst += "."+str(os.getpid()) + if os.path.exists(dst): + continue +- shutil.move(tempdir+"/"+d, dst) +- problem.notify_new_path(dst) ++ validate_transform_move_and_notify(tempdir+"/"+d, dst) + + die_exitcode = 0 + # This deletes working_dir (== delete_on_exit) +-- +1.8.3.1 + diff --git a/SOURCES/0114-daemon-harden-against-race-conditions-in-DELETE.patch b/SOURCES/0114-daemon-harden-against-race-conditions-in-DELETE.patch new file mode 100644 index 0000000..7061feb --- /dev/null +++ b/SOURCES/0114-daemon-harden-against-race-conditions-in-DELETE.patch @@ -0,0 +1,58 @@ +From 10bea037a2ad82616b3698d07d07d287481e1bed Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 6 May 2015 14:04:42 +0200 +Subject: [ABRT PATCH] daemon: harden against race conditions in DELETE + +There is a race between checking dump dir accessibility and deleting it +in abrt-server. + +Related: #1214457. + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 21 +++++++++++++++++++-- + 1 file changed, 19 insertions(+), 2 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 1030461..130c24a 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -91,8 +91,16 @@ static int delete_path(const char *dump_dir_name) + error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dump_dir_name); + return 400; /* */ + } +- if (!dump_dir_accessible_by_uid(dump_dir_name, client_uid)) ++ ++ int dir_fd = dd_openfd(dump_dir_name); ++ if (dir_fd < 0) ++ { ++ perror_msg("Can't open problem directory '%s'", dump_dir_name); ++ return 400; ++ } ++ if (!fdump_dir_accessible_by_uid(dir_fd, client_uid)) + { ++ close(dir_fd); + if (errno == ENOTDIR) + { + error_msg("Path '%s' isn't problem directory", dump_dir_name); +@@ -102,7 +110,16 @@ static int delete_path(const char *dump_dir_name) + return 403; /* Forbidden */ + } + +- delete_dump_dir(dump_dir_name); ++ struct dump_dir *dd = dd_fdopendir(dir_fd, dump_dir_name, /*flags:*/ 0); ++ if (dd) ++ { ++ if (dd_delete(dd) != 0) ++ { ++ error_msg("Failed to delete problem directory '%s'", dump_dir_name); ++ dd_close(dd); ++ return 400; ++ } ++ } + + return 0; /* success */ + } +-- +1.8.3.1 + diff --git a/SOURCES/0115-daemon-allow-only-root-user-to-trigger-the-post-crea.patch b/SOURCES/0115-daemon-allow-only-root-user-to-trigger-the-post-crea.patch new file mode 100644 index 0000000..f3dc626 --- /dev/null +++ b/SOURCES/0115-daemon-allow-only-root-user-to-trigger-the-post-crea.patch @@ -0,0 +1,65 @@ +From 3287aa12eb205cff95cdd00d6d6c5c9a4f8f0eca Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 6 May 2015 14:39:44 +0200 +Subject: [ABRT PATCH] daemon: allow only root user to trigger the post-create + +There is no reason to allow non-root users to trigger this +functionality. Regular users can create abrt problems only through +abrtd or abrt-dbus and both triggers the post-create. + +Other hooks run under root user (CCpp, Koops, VMCore, Xorg). + +Related: #1212861 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 19 ++++++++----------- + 1 file changed, 8 insertions(+), 11 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 130c24a..d3fa1b5 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -178,16 +178,6 @@ static int run_post_create(const char *dirname) + return 403; + } + } +- else if (!dump_dir_accessible_by_uid(dirname, client_uid)) +- { +- if (errno == ENOTDIR) +- { +- error_msg("Path '%s' isn't problem directory", dirname); +- return 404; /* Not Found */ +- } +- error_msg("Problem directory '%s' can't be accessed by user with uid %ld", dirname, (long)client_uid); +- return 403; /* Forbidden */ +- } + + int child_stdout_fd; + int child_pid = spawn_event_handler_child(dirname, "post-create", &child_stdout_fd); +@@ -741,14 +731,21 @@ static int perform_http_xact(void) + /* Body received, EOF was seen. Don't let alarm to interrupt after this. */ + alarm(0); + ++ int ret = 0; + if (url_type == CREATION_NOTIFICATION) + { ++ if (client_uid != 0) ++ { ++ error_msg("UID=%ld is not authorized to trigger post-create processing", (long)client_uid); ++ ret = 403; /* Forbidden */ ++ goto out; ++ } ++ + messagebuf_data[messagebuf_len] = '\0'; + return run_post_create(messagebuf_data); + } + + /* Save problem dir */ +- int ret = 0; + unsigned pid = convert_pid(problem_info); + die_if_data_is_missing(problem_info); + +-- +1.8.3.1 + diff --git a/SOURCES/0116-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch b/SOURCES/0116-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch new file mode 100644 index 0000000..24bd244 --- /dev/null +++ b/SOURCES/0116-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch @@ -0,0 +1,127 @@ +From 7417505e1d93cc95ec648b74e3c801bc67aacb9f Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 7 May 2015 11:07:12 +0200 +Subject: [ABRT PATCH] daemon, dbus: allow only root to create CCpp, Koops, + vmcore and xorg +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Florian Weimer : + This prevents users from feeding things that are not actually + coredumps and excerpts from /proc to these analyzers. + + For example, it should not be possible to trigger a rule with + “EVENT=post-create analyzer=CCpp” using NewProblem + +Related: #1212861 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 2 +- + src/dbus/abrt-dbus.c | 10 +++++++++- + src/include/libabrt.h | 2 ++ + src/lib/hooklib.c | 24 ++++++++++++++++++++++++ + 4 files changed, 36 insertions(+), 2 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index d3fa1b5..afd9fd3 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -487,7 +487,7 @@ static gboolean key_value_ok(gchar *key, gchar *value) + } + } + +- return TRUE; ++ return allowed_new_user_problem_entry(client_uid, key, value); + } + + /* Handles a message received from client over socket. */ +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 6de15e9..bef95bd 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -168,6 +168,7 @@ bool allowed_problem_dir(const char *dir_name) + + static char *handle_new_problem(GVariant *problem_info, uid_t caller_uid, char **error) + { ++ char *problem_id = NULL; + problem_data_t *pd = problem_data_new(); + + GVariantIter *iter; +@@ -175,6 +176,12 @@ static char *handle_new_problem(GVariant *problem_info, uid_t caller_uid, char * + gchar *key, *value; + while (g_variant_iter_loop(iter, "{ss}", &key, &value)) + { ++ if (allowed_new_user_problem_entry(caller_uid, key, value) == false) ++ { ++ *error = xasprintf("You are not allowed to create element '%s' containing '%s'", key, value); ++ goto finito; ++ } ++ + problem_data_add_text_editable(pd, key, value); + } + +@@ -189,12 +196,13 @@ static char *handle_new_problem(GVariant *problem_info, uid_t caller_uid, char * + /* At least it should generate local problem identifier UUID */ + problem_data_add_basics(pd); + +- char *problem_id = problem_data_save(pd); ++ problem_id = problem_data_save(pd); + if (problem_id) + notify_new_path(problem_id); + else if (error) + *error = xasprintf("Cannot create a new problem"); + ++finito: + problem_data_free(pd); + return problem_id; + } +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 5bf2397..3749a31 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -51,6 +51,8 @@ char *get_backtrace(const char *dump_dir_name, unsigned timeout_sec, const char + bool dir_is_in_dump_location(const char *dir_name); + #define dir_has_correct_permissions abrt_dir_has_correct_permissions + bool dir_has_correct_permissions(const char *dir_name); ++#define allowed_new_user_problem_entry abrt_allowed_new_user_problem_entry ++bool allowed_new_user_problem_entry(uid_t uid, const char *name, const char *value); + + #define g_settings_nMaxCrashReportsSize abrt_g_settings_nMaxCrashReportsSize + extern unsigned int g_settings_nMaxCrashReportsSize; +diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c +index 4b20025..8e93663 100644 +--- a/src/lib/hooklib.c ++++ b/src/lib/hooklib.c +@@ -483,3 +483,27 @@ bool dir_has_correct_permissions(const char *dir_name) + } + return true; + } ++ ++bool allowed_new_user_problem_entry(uid_t uid, const char *name, const char *value) ++{ ++ /* Allow root to create everything */ ++ if (uid == 0) ++ return true; ++ ++ /* Permit non-root users to create everything except: analyzer and type */ ++ if (strcmp(name, FILENAME_ANALYZER) != 0 ++ && strcmp(name, FILENAME_TYPE) != 0 ++ /* compatibility value used in abrt-server */ ++ && strcmp(name, "basename") != 0) ++ return true; ++ ++ /* Permit non-root users to create all types except: C/C++, Koops, vmcore and xorg */ ++ if (strcmp(value, "CCpp") != 0 ++ && strcmp(value, "Kerneloops") != 0 ++ && strcmp(value, "vmcore") != 0 ++ && strcmp(value, "xorg") != 0) ++ return true; ++ ++ error_msg("Only root is permitted to create element '%s' containing '%s'", name, value); ++ return false; ++} +-- +1.8.3.1 + diff --git a/SOURCES/0117-dumpers-avoid-AVC-when-creating-dump-directories.patch b/SOURCES/0117-dumpers-avoid-AVC-when-creating-dump-directories.patch new file mode 100644 index 0000000..f3de437 --- /dev/null +++ b/SOURCES/0117-dumpers-avoid-AVC-when-creating-dump-directories.patch @@ -0,0 +1,65 @@ +From d59475b77eb47e8270557f5828acf786cffcf8f8 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 7 May 2015 14:22:27 +0200 +Subject: [ABRT PATCH] dumpers: avoid AVC when creating dump directories + +dump-oops and dump-xorg forces libreport to create a new dump directory +owned by root and the group abrt. That requires querying passwd and +group which is not yet allowed by selinux-policy: +https://bugzilla.redhat.com/show_bug.cgi?id=1219464 + +This is a temporary patch for rhel-7.1.z + +Signed-off-by: Jakub Filak +--- + src/plugins/abrt-dump-oops.c | 11 ++++++++++- + src/plugins/abrt-dump-xorg.c | 11 ++++++++++- + 2 files changed, 20 insertions(+), 2 deletions(-) + +diff --git a/src/plugins/abrt-dump-oops.c b/src/plugins/abrt-dump-oops.c +index 05cb728..2dc93c9 100644 +--- a/src/plugins/abrt-dump-oops.c ++++ b/src/plugins/abrt-dump-oops.c +@@ -195,7 +195,16 @@ static unsigned create_oops_dump_dirs(GList *oops_list, unsigned oops_cnt) + log("Not going to make dump directories world readable because PrivateReports is on"); + + mode = DEFAULT_DUMP_DIR_MODE; +- my_euid = 0; ++ /* Keep my_euid=-1, it produces dump directories owned by the user root ++ * and the group root. ++ * ++ * Using my_euid!=-1 forces libreport to read /etc/passwd and ++ * /etc/group which generates SELinux AVC. ++ */ ++ /* my_euid = 0; */ ++ ++ if (geteuid() != 0) ++ error_msg_and_die("PrivateReports is on, you must run this tool as root."); + } + + pid_t my_pid = getpid(); +diff --git a/src/plugins/abrt-dump-xorg.c b/src/plugins/abrt-dump-xorg.c +index 434dc76..545db7f 100644 +--- a/src/plugins/abrt-dump-xorg.c ++++ b/src/plugins/abrt-dump-xorg.c +@@ -88,7 +88,16 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea + log("Not going to make dump directories world readable because PrivateReports is on"); + + mode = DEFAULT_DUMP_DIR_MODE; +- my_euid = 0; ++ /* Keep my_euid=-1, it produces dump directories owned by the user root ++ * and the group root. ++ * ++ * Using my_euid!=-1 forces libreport to read /etc/passwd and ++ * /etc/group which generates SELinux AVC. ++ */ ++ /* my_euid = 0; */ ++ ++ if (geteuid() != 0) ++ error_msg_and_die("PrivateReports is on, you must run this tool as root."); + } + + pid_t my_pid = getpid(); +-- +1.8.3.1 + diff --git a/SOURCES/0118-dbus-validate-parameters-of-all-calls.patch b/SOURCES/0118-dbus-validate-parameters-of-all-calls.patch new file mode 100644 index 0000000..18a3ea7 --- /dev/null +++ b/SOURCES/0118-dbus-validate-parameters-of-all-calls.patch @@ -0,0 +1,69 @@ +From 7a47f57975be0d285a2f20758e4572dca6d9cdd3 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 13 May 2015 11:10:23 +0200 +Subject: [ABRT PATCH] dbus: validate parameters of all calls + +SetElement and DeleteElement were missing check for valid dump directory +path. + +FindProblemByElementInTimeRange was not reporting invalid element names. + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 24 ++++++++++++++++++++++++ + 1 file changed, 24 insertions(+) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index bef95bd..f2f742b 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -607,6 +607,12 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s&s)", &problem_id, &element, &value); + ++ if (!allowed_problem_dir(problem_id)) ++ { ++ return_InvalidProblemDir_error(invocation, problem_id); ++ return; ++ } ++ + if (!str_is_correct_filename(element)) + { + log_notice("'%s' is not a valid element name of '%s'", element, problem_id); +@@ -666,6 +672,12 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + ++ if (!allowed_problem_dir(problem_id)) ++ { ++ return_InvalidProblemDir_error(invocation, problem_id); ++ return; ++ } ++ + if (!str_is_correct_filename(element)) + { + log_notice("'%s' is not a valid element name of '%s'", element, problem_id); +@@ -783,6 +795,18 @@ static void handle_method_call(GDBusConnection *connection, + g_variant_get_child(parameters, 3, "x", ×tamp_to); + g_variant_get_child(parameters, 4, "b", &all); + ++ if (!str_is_correct_filename(element)) ++ { ++ log_notice("'%s' is not a valid element name", element); ++ char *error = xasprintf(_("'%s' is not a valid element name"), element); ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.InvalidElement", ++ error); ++ ++ free(error); ++ return; ++ } ++ + if (all && polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") == PolkitYes) + caller_uid = 0; + +-- +1.8.3.1 + diff --git a/SOURCES/0119-ccpp-do-not-unlink-failed-and-big-user-cores.patch b/SOURCES/0119-ccpp-do-not-unlink-failed-and-big-user-cores.patch new file mode 100644 index 0000000..83c958b --- /dev/null +++ b/SOURCES/0119-ccpp-do-not-unlink-failed-and-big-user-cores.patch @@ -0,0 +1,167 @@ +From 7bd77a63f226a572946f30db3e76f23f971f46d5 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 20 May 2015 06:07:15 +0200 +Subject: [ABRT PATCH] ccpp: do not unlink failed and big user cores + +* We might end up deleting an already existing file. +* Kernel does not delete nor truncate core files. Admittedly, kernel + knows how process's memory is structured, dumps it per logical + segments and checks whether a next segment can be written. +* 'ulimit -c' does not seem to be a hard limit. Kernel wrote 8192 bytes + despite $(ulimit -c) == 6. + +Related: #1212818 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 70 +++++++++++++++++++--------------------------- + 1 file changed, 29 insertions(+), 41 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index fdd9b06..9b38ed7 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -129,8 +129,8 @@ static off_t copyfd_sparse(int src_fd, int dst_fd1, int dst_fd2, off_t size2) + size2 -= rd; + if (size2 < 0) + dst_fd2 = -1; +-//TODO: truncate to 0 or even delete the second file +-//(currently we delete the file later) ++// truncate to 0 or even delete the second file? ++// No, kernel does not delete nor truncate core files. + } + out: + +@@ -502,13 +502,20 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + + user_core_fail: + if (user_core_fd >= 0) +- { + close(user_core_fd); +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); +- } + return -1; + } + ++static int close_user_core(int user_core_fd, off_t core_size) ++{ ++ if (user_core_fd >= 0 && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0)) ++ { ++ perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd); ++ return -1; ++ } ++ return 0; ++} ++ + static bool dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs, uid_t uid, gid_t gid) + { + FILE *fp = fopen(dest_filename, "wx"); +@@ -569,7 +576,7 @@ static int create_or_die(const char *filename) + if (dd) + dd_delete(dd); + if (user_core_fd >= 0) +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); ++ close(user_core_fd); + + errno = sv_errno; + perror_msg_and_die("Can't open '%s'", filename); +@@ -577,6 +584,7 @@ static int create_or_die(const char *filename) + + int main(int argc, char** argv) + { ++ int err = 1; + /* Kernel starts us with all fd's closed. + * But it's dangerous: + * fprintf(stderr) can dump messages into random fds, etc. +@@ -778,9 +786,8 @@ int main(int argc, char** argv) + error_msg_and_die("Error saving '%s'", path); + } + log("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size); +- if (proc_cwd != NULL) +- closedir(proc_cwd); +- return 0; ++ err = 0; ++ goto finito; + } + + unsigned path_len = snprintf(path, sizeof(path), "%s/ccpp-%s-%lu.new", +@@ -895,26 +902,17 @@ int main(int argc, char** argv) + * ls: cannot access core*: No such file or directory <=== BAD + */ + off_t core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c); ++ ++ close_user_core(user_core_fd, core_size); ++ + if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0) + { + unlink(path); + dd_delete(dd); +- if (user_core_fd >= 0) +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); + /* copyfd_sparse logs the error including errno string, + * but it does not log file name */ + error_msg_and_die("Error writing '%s'", path); + } +- if (user_core_fd >= 0 +- /* error writing user coredump? */ +- && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 +- /* user coredump is too big? */ +- || (ulimit_c == 0 /* paranoia */ || core_size > ulimit_c) +- ) +- ) { +- /* nuke it (silently) */ +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); +- } + + /* Because of #1211835 and #1126850 */ + #if 0 +@@ -984,9 +982,9 @@ int main(int argc, char** argv) + } + + free(rootdir); +- if (proc_cwd != NULL) +- closedir(proc_cwd); +- return 0; ++ ++ err = 0; ++ goto finito; + } + + /* We didn't create abrt dump, but may need to create compat coredump */ +@@ -994,26 +992,16 @@ int main(int argc, char** argv) + 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' at '%s'", core_basename, user_pwd); +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); +- if (proc_cwd != NULL) +- closedir(proc_cwd); +- return 1; +- } +- if (ulimit_c == 0 || core_size > ulimit_c) +- { +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); +- if (proc_cwd != NULL) +- closedir(proc_cwd); +- return 1; +- } ++ if (close_user_core(user_core_fd, core_size) != 0) ++ goto finito; ++ ++ err = 0; + log("Saved core dump of pid %lu to %s at %s (%llu bytes)", (long)pid, core_basename, user_pwd, (long long)core_size); + } + ++ finito: + if (proc_cwd != NULL) + closedir(proc_cwd); +- return 0; ++ ++ return err; + } +-- +1.8.3.1 + diff --git a/SOURCES/0120-a-a-i-d-t-a-cache-don-t-open-the-build_ids-file-as-a.patch b/SOURCES/0120-a-a-i-d-t-a-cache-don-t-open-the-build_ids-file-as-a.patch new file mode 100644 index 0000000..75d0737 --- /dev/null +++ b/SOURCES/0120-a-a-i-d-t-a-cache-don-t-open-the-build_ids-file-as-a.patch @@ -0,0 +1,82 @@ +From 3bdf6305f6a8501a692e1a98f98e0be9d3922a1d Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 20 May 2015 08:08:58 +0200 +Subject: [ABRT PATCH] a-a-i-d-t-a-cache: don't open the build_ids file as abrt + +Opening the build_ids file as abrt may lead to information disclosure. + +Related: #1216962 + +Signed-off-by: Jakub Filak +--- + .../abrt-action-install-debuginfo-to-abrt-cache.c | 30 +++++++++++++++++----- + 1 file changed, 23 insertions(+), 7 deletions(-) + +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 cd9ee7a..fafb0c4 100644 +--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c ++++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +@@ -72,6 +72,11 @@ int main(int argc, char **argv) + }; + const unsigned opts = parse_opts(argc, argv, program_options, program_usage_string); + ++ const gid_t egid = getegid(); ++ const gid_t rgid = getgid(); ++ const uid_t euid = geteuid(); ++ const gid_t ruid = getuid(); ++ + /* We need to open the build ids file under the caller's UID/GID to avoid + * information disclosures when reading files with changed UID. + * Unfortunately, we cannot replace STDIN with the new fd because ABRT uses +@@ -82,7 +87,20 @@ int main(int argc, char **argv) + char *build_ids_self_fd = NULL; + if (strcmp("-", build_ids) != 0) + { ++ if (setregid(egid, rgid) < 0) ++ perror_msg_and_die("setregid(egid, rgid)"); ++ ++ if (setreuid(euid, ruid) < 0) ++ perror_msg_and_die("setreuid(euid, ruid)"); ++ + const int build_ids_fd = open(build_ids, O_RDONLY); ++ ++ if (setregid(rgid, egid) < 0) ++ perror_msg_and_die("setregid(rgid, egid)"); ++ ++ if (setreuid(ruid, euid) < 0 ) ++ perror_msg_and_die("setreuid(ruid, euid)"); ++ + if (build_ids_fd < 0) + perror_msg_and_die("Failed to open file '%s'", build_ids); + +@@ -118,14 +136,12 @@ int main(int argc, char **argv) + /* Switch real user/group to effective ones. + * Otherwise yum library gets confused - gets EPERM (why??). + */ +- gid_t g = getegid(); + /* do setregid only if we have to, to not upset selinux needlessly */ +- if (g != getgid()) +- IGNORE_RESULT(setregid(g, g)); +- uid_t u = geteuid(); +- if (u != getuid()) ++ if (egid != rgid) ++ IGNORE_RESULT(setregid(egid, egid)); ++ if (euid != ruid) + { +- IGNORE_RESULT(setreuid(u, u)); ++ IGNORE_RESULT(setreuid(euid, euid)); + /* We are suid'ed! */ + /* Prevent malicious user from messing up with suid'ed process: */ + #if 1 +@@ -179,7 +195,7 @@ int main(int argc, char **argv) + // abrt-action-install-debuginfo doesn't fail when spawning + // abrt-action-trim-files + char path_env[] = "PATH=/usr/sbin:/sbin:/usr/bin:/bin:"BIN_DIR":"SBIN_DIR; +- if (u != 0) ++ if (euid != 0) + strcpy(path_env, "PATH=/usr/bin:/bin:"BIN_DIR); + putenv(path_env); + +-- +1.8.3.1 + diff --git a/SOURCES/0121-a-a-i-d-t-a-cache-fix-command-line-argument-generati.patch b/SOURCES/0121-a-a-i-d-t-a-cache-fix-command-line-argument-generati.patch new file mode 100644 index 0000000..6a04adc --- /dev/null +++ b/SOURCES/0121-a-a-i-d-t-a-cache-fix-command-line-argument-generati.patch @@ -0,0 +1,32 @@ +From ff67428ed1685b1d5b12e2893396d6acecf3a123 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 20 May 2015 15:22:58 +0200 +Subject: [ABRT PATCH] a-a-i-d-t-a-cache: fix command line argument generation + +Empty string in the list of arguments for execvp causes problems (-y +was ignored). + +Related: #1216962 + +Signed-off-by: Jakub Filak +--- + src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +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 fafb0c4..81b1486 100644 +--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c ++++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +@@ -116,7 +116,8 @@ int main(int argc, char **argv) + args[i++] = EXECUTABLE; + args[i++] = "--ids"; + args[i++] = (build_ids_self_fd != NULL) ? build_ids_self_fd : "-"; +- args[i++] = verbs[g_verbose <= 3 ? g_verbose : 3]; ++ if (g_verbose > 0) ++ args[i++] = verbs[g_verbose <= 3 ? g_verbose : 3]; + if ((opts & OPT_y)) + args[i++] = "-y"; + if ((opts & OPT_e)) +-- +1.8.3.1 + diff --git a/SOURCES/1000-turn-sosreport-off.patch b/SOURCES/1000-turn-sosreport-off.patch deleted file mode 100644 index 4b7166a..0000000 --- a/SOURCES/1000-turn-sosreport-off.patch +++ /dev/null @@ -1,24 +0,0 @@ -From 2b02dc85753e4b11f10bfa2d660aa493ae80c52b Mon Sep 17 00:00:00 2001 -From: Jakub Filak -Date: Thu, 20 Nov 2014 11:24:39 +0100 -Subject: [PATCH] turn sosreport off - ---- - src/daemon/abrt_event.conf | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/src/daemon/abrt_event.conf b/src/daemon/abrt_event.conf -index 380b312..eafee17 100644 ---- a/src/daemon/abrt_event.conf -+++ b/src/daemon/abrt_event.conf -@@ -67,7 +67,7 @@ EVENT=post-create runlevel= - # Example: if you want to save sosreport immediately at the moment of a crash: - # (alternatively, you can add similar command to EVENT=analyze_foo's, - # if you would rather perform this collection later): --EVENT=post-create -+#EVENT=post-create - nice sosreport --tmp-dir "$DUMP_DIR" --batch \ - --only=anaconda --only=boot --only=devicemapper \ - --only=filesys --only=hardware --only=kernel --only=libraries \ --- -1.8.3.1 diff --git a/SPECS/abrt.spec b/SPECS/abrt.spec index ff0f90d..e657db1 100644 --- a/SPECS/abrt.spec +++ b/SPECS/abrt.spec @@ -27,13 +27,13 @@ %define desktopvendor fedora %endif -%define libreport_ver 2.1.11-21%{?dist}.0.3 +%define libreport_ver 2.1.11-22 %define satyr_ver 0.13-7 Summary: Automatic bug detection and reporting tool Name: abrt Version: 2.1.11 -Release: 19%{?dist}.0.3 +Release: 22%{?dist} License: GPLv2+ Group: Applications/System URL: https://fedorahosted.org/abrt/ @@ -54,6 +54,7 @@ Patch12: 0012-configui-show-Close-button-in-the-dialog.patch Patch13: 0013-applet-do-not-say-the-report-is-anonymous-when-urepo.patch #Patch14: 0014-spec-abrt-cli-requires-a-pkg-providing-workflows.patch #Patch15: 0015-testsuite-encourage-users-to-create-a-case-in-RHTS.patch +Patch16: 0016-cli-list-show-a-hint-about-creating-a-case-in-RHTS.patch Patch17: 0017-harvest-vmcore-properly-handle-inaccessible-dir-erro.patch Patch18: 0018-don-t-break-the-event-run-by-failures-of-abrt-action.patch Patch19: 0019-Fix-handling-of-Machine-Check-Exceptions.patch @@ -116,7 +117,7 @@ Patch75: 0075-Translation-updates.patch Patch76: 0076-Revert-gdb-disable-loading-of-auto-loaded-files.patch Patch77: 0077-gdb-make-gdb-aware-of-the-abrt-s-debuginfo-dir.patch #Patch78: 0078-spec-update-the-required-gdb-version.patch -#Patch79: 0079-cli-mark-the-suggestion-text-for-translation.patch +Patch79: 0079-cli-mark-the-suggestion-text-for-translation.patch Patch80: 0080-auto-reporting-add-options-to-specify-auth-type.patch #Patch81: 0081-testsuite-abrt-auto-reporting-uReport-authentication.patch Patch82: 0082-translations-pull-the-newest-PO-files.patch @@ -125,12 +126,44 @@ Patch82: 0082-translations-pull-the-newest-PO-files.patch #Patch85: 0085-zanata-add-gettext-mappings.patch Patch86: 0086-translations-update-the-PO-files.patch Patch87: 0087-abrt-auto-reporting-make-the-code-more-safer.patch -Patch88: 0088-event-don-t-run-the-reporter-bugzilla-h-on-RHEL-and-.patch -#Patch89: 0089-spec-added-dependency-to-libreport-centos.patch -Patch90: 0090-plugin-set-URL-to-retrace-server.patch -#Patch91: 0091-spec-add-dependenci-on-abrt-retrace-client.patch - -Patch1000: 1000-turn-sosreport-off.patch +# git format-patch 2.1.11-19.el7 -N --start-number 88 --topo-order +Patch88: 0088-a-a-save-package-data-turn-off-reading-data-from-roo.patch +Patch89: 0089-ccpp-fix-symlink-race-conditions.patch +Patch90: 0090-ccpp-stop-reading-hs_error.log-from-tmp.patch +Patch91: 0091-ccpp-do-not-read-data-from-root-directories.patch +Patch92: 0092-ccpp-open-file-for-dump_fd_info-with-O_EXCL.patch +Patch93: 0093-ccpp-postpone-changing-ownership-of-new-dump-directo.patch +Patch94: 0094-ccpp-create-dump-directory-without-parents.patch +Patch95: 0095-ccpp-do-not-override-existing-files-by-compat-cores.patch +Patch96: 0096-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch +Patch97: 0097-ccpp-harden-dealing-with-UID-GID.patch +Patch98: 0098-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch +Patch99: 0099-ccpp-emulate-selinux-for-creation-of-compat-cores.patch +Patch100: 0100-make-the-dump-directories-owned-by-root-by-default.patch +Patch101: 0101-configure-move-the-default-dump-location-to-var-spoo.patch +#Patch102: 0102-spec-create-vat-spool-abrt.patch +Patch103: 0103-ccpp-avoid-overriding-system-files-by-coredump.patch +#Patch104: 0104-spec-add-libselinux-devel-to-BRs.patch +Patch105: 0105-daemon-use-libreport-s-function-checking-file-name.patch +Patch106: 0106-lib-add-functions-validating-dump-dir.patch +Patch107: 0107-dbus-process-only-valid-sub-directories-of-the-dump-.patch +Patch108: 0108-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch +Patch109: 0109-dbus-report-invalid-element-names.patch +Patch110: 0110-a-a-i-d-t-a-cache-sanitize-arguments.patch +Patch111: 0111-a-a-i-d-t-a-cache-sanitize-umask.patch +Patch112: 0112-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch +Patch113: 0113-upload-validate-and-sanitize-uploaded-dump-directori.patch +Patch114: 0114-daemon-harden-against-race-conditions-in-DELETE.patch +Patch115: 0115-daemon-allow-only-root-user-to-trigger-the-post-crea.patch +Patch116: 0116-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch +# Temporary RHEL-7.1.z patch #1219464 +Patch117: 0117-dumpers-avoid-AVC-when-creating-dump-directories.patch +# git format-patch 2.1.11-20.el7 -N --start-number 118 --topo-order +Patch118: 0118-dbus-validate-parameters-of-all-calls.patch +# git format-patch 2.1.11-21.el7 -N --start-number 119 --topo-order +Patch119: 0119-ccpp-do-not-unlink-failed-and-big-user-cores.patch +Patch120: 0120-a-a-i-d-t-a-cache-don-t-open-the-build_ids-file-as-a.patch +Patch121: 0121-a-a-i-d-t-a-cache-fix-command-line-argument-generati.patch # git is need for '%%autosetup -S git' which automatically applies all the # patches above. Please, be aware that the patches must be generated @@ -156,6 +189,7 @@ BuildRequires: libreport-devel >= %{libreport_ver} BuildRequires: satyr-devel >= %{satyr_ver} BuildRequires: systemd-python BuildRequires: augeas +BuildRequires: libselinux-devel Requires: abrt-python = %{version}-%{release} Requires: libreport >= %{libreport_ver} Requires: satyr >= %{satyr_ver} @@ -238,8 +272,10 @@ Group: System Environment/Libraries Requires: cpio Requires: gdb >= 7.6.1-63 Requires: elfutils +%if 0%{!?rhel:1} # abrt-action-perform-ccpp-analysis wants to run analyze_RetraceServer: Requires: %{name}-retrace-client +%endif Requires: %{name} = %{version}-%{release} Requires: abrt-libs = %{version}-%{release} Requires: libreport-python @@ -352,13 +388,8 @@ Requires: abrt-addon-ccpp Requires: abrt-addon-python Requires: abrt-addon-xorg %if 0%{?rhel} -%if 0%{?centos_ver} -Requires: libreport-centos >= %{libreport_ver} -Requires: libreport-plugin-mantisbt >= %{libreport_ver} -%else Requires: libreport-rhel >= %{libreport_ver} Requires: libreport-plugin-rhtsupport >= %{libreport_ver} -%endif %else Requires: abrt-retrace-client Requires: libreport-plugin-bugzilla >= %{libreport_ver} @@ -391,13 +422,8 @@ Requires: elfutils Requires: abrt-gui Requires: gnome-abrt %if 0%{?rhel} -%if 0%{?centos_ver} -Requires: libreport-centos >= %{libreport_ver} -Requires: libreport-plugin-mantisbt >= %{libreport_ver} -%else Requires: libreport-rhel >= %{libreport_ver} Requires: libreport-plugin-rhtsupport >= %{libreport_ver} -%endif %else Requires: abrt-retrace-client Requires: libreport-plugin-bugzilla >= %{libreport_ver} @@ -491,7 +517,7 @@ find $RPM_BUILD_ROOT -name '*.la' -or -name '*.a' | xargs rm -f mkdir -p ${RPM_BUILD_ROOT}/%{_initrddir} mkdir -p $RPM_BUILD_ROOT/var/cache/abrt-di mkdir -p $RPM_BUILD_ROOT/var/run/abrt -mkdir -p $RPM_BUILD_ROOT/var/tmp/abrt +mkdir -p $RPM_BUILD_ROOT/var/spool/abrt mkdir -p $RPM_BUILD_ROOT/var/spool/abrt-upload desktop-file-install \ @@ -515,64 +541,18 @@ make check getent group abrt >/dev/null || groupadd -f -g %{abrt_gid_uid} --system abrt getent passwd abrt >/dev/null || useradd --system -g abrt -u %{abrt_gid_uid} -d /etc/abrt -s /sbin/nologin abrt -OLD_LOCATION="/var/spool/abrt" -NEW_LOCATION="/var/tmp/abrt" +OLD_LOCATION="/var/tmp/abrt" # $1 == 1 if install; 2 if upgrade if [ "$1" -eq 2 ] then test -d "$OLD_LOCATION" || exit 0 - mkdir -p "$NEW_LOCATION" - - #restorecon "$NEW_LOCATION" - find "$OLD_LOCATION" -maxdepth 1 -type f -exec cp '{}' "$NEW_LOCATION" \; + # remove old dump directories for DD in `find "$OLD_LOCATION" -maxdepth 1 -type d` do - # skip dump location self - if [ "$DD" == "$OLD_LOCATION" ]; then - continue - fi - - # skip symlinks - if [ -L "$DD" ]; then - continue - fi - - # check if time element exists, if so then consider a directory as dump - # directory and move it to the new location + # in order to not remove user stuff remove only directories containing 'time' file if [ -f "$DD/time" ]; then - NEW_DD="$NEW_LOCATION/"`basename $DD` - - # to be sure we do not override anything - if [ -d "$NEW_DD" ]; then - continue - fi - - if cp --recursive -- "$DD" "$NEW_DD"; then - # owner of dump dir is identified by his group - # so get the owner of dump dir - OWNER_GID=`stat -c "%g" "$DD"` - - # use group's user in case where group contains only single user - if [ $(getent passwd | cut -f3,4,5 -d: | grep -c ":$OWNER_GID:") -eq 1 ]; then - OWNER_UID=`getent passwd | cut -f3,4,5 -d: | grep ":$OWNER_GID:" | cut -f1 -d:` - else - # otherwise get owner from dd's uid element - OWNER_UID=`cat "$NEW_DD/uid"` - fi - - getent passwd "$OWNER_UID" >/dev/null || { - # if user doesn't exist pass the ownership to root - OWNER_UID=0 - } - - # set new schema of ownership - chown --recursive "$OWNER_UID":abrt "$NEW_DD" && { - chmod 770 "$NEW_DD" - find "$NEW_DD" -type d -exec chmod 770 '{}' \; - find "$NEW_DD" -type f -exec chmod 660 '{}' \; - } - fi + rm -rf $DD fi done fi @@ -670,23 +650,10 @@ if [ $1 -eq 0 ] ; then fi %posttrans -# update the old problem dirs to contain "type" element -abrtdir=$(grep "DumpLocation" /etc/abrt/abrt.conf | cut -d'=' -f2 | tr -d ' '); cd $abrtdir 2>/dev/null && for i in `find . -name "analyzer" 2>/dev/null`; do len=${#i};cp "$i" "${i:0:$len-9}/type"; done; for i in `find "$abrtdir" -mindepth 1 -maxdepth 1 -type d`; do chown `stat --format=%U:abrt $i` $i/*; done service abrtd condrestart >/dev/null 2>&1 || : %posttrans addon-ccpp service abrt-ccpp condrestart >/dev/null 2>&1 || : -# Regenerate core_bactraces because of missing crash threads -abrtdir=$(grep "DumpLocation" /etc/abrt/abrt.conf | cut -d'=' -f2 | tr -d ' ') -if test -d "$abrtdir"; then - for DD in `find "$abrtdir" -mindepth 1 -maxdepth 1 -type d` - do - if test -f "$DD/analyzer" && grep -q "^CCpp$" "$DD/analyzer"; then - /usr/bin/abrt-action-generate-core-backtrace -d "$DD" -- >/dev/null 2>&1 || : - test -f "$DD/core_backtrace" && chown `stat --format=%U:abrt $DD` "$DD/core_backtrace" || : - fi - done -fi %posttrans addon-kerneloops service abrt-oops condrestart >/dev/null 2>&1 || : @@ -745,7 +712,7 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : %{_mandir}/man5/abrt_event.conf.5.gz %config(noreplace) %{_sysconfdir}/libreport/events.d/smart_event.conf %{_mandir}/man5/smart_event.conf.5.gz -%dir %attr(0755, abrt, abrt) %{_localstatedir}/tmp/%{name} +%dir %attr(0755, abrt, abrt) %{_localstatedir}/spool/%{name} %dir %attr(0700, abrt, abrt) %{_localstatedir}/spool/%{name}-upload # abrtd runs as root %dir %attr(0755, root, root) %{_localstatedir}/run/%{name} @@ -1005,16 +972,26 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : %config(noreplace) %{_sysconfdir}/profile.d/abrt-console-notification.sh %changelog -* Mon Mar 09 2015 Matej Habrnal - 2.1.11-19.el7.centos -- Drop RHTS hint -- Change by David Mansfield -- Per http://bugs.centos.org/view.php?id=7192 -- update to not run sosreport -- Per http://bugs.centos.org/view.php?id=7913 -- Remove patch 79 +* Thu May 21 2015 Jakub Filak - 2.1.11-22 +- do not open the build_ids file as the user abrt +- do not unlink failed and big user core files +- Related: #1212819, #1216973 + +* Wed May 13 2015 Jakub Filak - 2.1.11-21 +- validate all D-Bus method arguments +- Related: #1214610 + +* Tue May 05 2015 Jakub Filak - 2.1.11-20 +- remove the old dump directories during upgrade +- abrt-action-install-debuginfo-to-abrt-cache: sanitize arguments and umask +- fix race conditions and directory traversal issues in abrt-dbus +- use /var/spool/abrt instead of /var/tmp/abrt +- make the problem directories owned by root and the group abrt +- validate uploaded problem directories in abrt-handle-upload +- don't override files with user core dump files +- fix symbolic link and race condition flaws +- Resolves: #1211969, #1212819, #1212863, #1212869 +- Resolves: #1214453, #1214610, #1216973, #1218583 * Fri Jan 09 2015 Jakub Filak - 2.1.11-19 - abrt-auto-reporting: add ureport authentication command line arguments