CVE-2015-5273, CVE-2015-5287, coredumpctl

Resolves: #1262252, #1284557
This commit is contained in:
Jakub Filak 2015-11-25 11:30:00 +01:00
commit d596add397
13 changed files with 706 additions and 2 deletions

View file

@ -0,0 +1,32 @@
From 97633ab28bef2e65032fb81063922e3106dd135b Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 28 Oct 2015 00:21:12 +0100
Subject: [PATCH] dbus: ensure expected bytes width of DBus numbers
t - UINT64 - guint64 (unsigned long is not wide enough on 32bit architectures)
Resolves: rhbz#1256456
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/dbus/abrt-dbus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
index 44778a2..500ddb2 100644
--- a/src/dbus/abrt-dbus.c
+++ b/src/dbus/abrt-dbus.c
@@ -625,8 +625,8 @@ static void handle_method_call(GDBusConnection *connection,
g_variant_builder_add(response_builder, "{s(its)}",
element_name,
- element_info->flags,
- size,
+ (gint32)element_info->flags,
+ (guint64)size,
element_info->content);
}
--
2.6.3

View file

@ -0,0 +1,98 @@
From 001066cf6238ee3a61f1a87607272e9736d3e371 Mon Sep 17 00:00:00 2001
From: Matej Habrnal <mhabrnal@redhat.com>
Date: Mon, 26 Oct 2015 13:57:51 +0100
Subject: [PATCH] doc: add missing man page for abrt-dump-journal-core
Signed-off-by: Matej Habrnal <mhabrnal@redhat.com>
---
doc/Makefile.am | 1 +
doc/abrt-dump-journal-core.txt | 65 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
create mode 100644 doc/abrt-dump-journal-core.txt
diff --git a/doc/Makefile.am b/doc/Makefile.am
index fdd08cf..ec6b3c5 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -19,6 +19,7 @@ MAN1_TXT += abrt-action-perform-ccpp-analysis.txt
MAN1_TXT += abrt-action-notify.txt
MAN1_TXT += abrt-applet.txt
MAN1_TXT += abrt-dump-oops.txt
+MAN1_TXT += abrt-dump-journal-core.txt
MAN1_TXT += abrt-dump-journal-oops.txt
MAN1_TXT += abrt-dump-xorg.txt
MAN1_TXT += abrt-auto-reporting.txt
diff --git a/doc/abrt-dump-journal-core.txt b/doc/abrt-dump-journal-core.txt
new file mode 100644
index 0000000..25d5e57
--- /dev/null
+++ b/doc/abrt-dump-journal-core.txt
@@ -0,0 +1,65 @@
+abrt-dump-journal-core(1)
+=========================
+
+NAME
+----
+abrt-dump-journal-core - Extract coredumps from systemd-journal
+
+SYNOPSIS
+--------
+'abrt-dump-journal-core' [-vsf] [-e]/[-c CURSOR] [-t INT]/[-T] [-d DIR]/[-D]
+
+DESCRIPTION
+-----------
+This tool creates problem directory from coredumps extracted from
+systemd-journal.
+The tool can follow systemd-journal and extract coredumps in time of their
+occurrence.
+
+The following start from the last seen cursor. If the last seen cursor file
+does not exist, the following start by scanning the entire sytemd-journal or
+from the end if '-e' option is specified.
+
+-c and -e options conflicts because both specifies the first read message.
+
+-e is useful only for -f because the following of journal starts by reading
+the entire journal if the last seen possition is not available.
+
+FILES
+-----
+/var/lib/abrt/abrt-dump-journal-core.state::
+ State file where systemd-journal cursor to the last seen message is saved
+
+OPTIONS
+-------
+-v, --verbose::
+ Be more verbose. Can be given multiple times.
+
+-s::
+ Log to syslog
+
+-d DIR::
+ Create new problem directory in DIR for every coredump
+
+-D::
+ Same as -d DumpLocation, DumpLocation is specified in abrt.conf
+
+-e::
+ Starts following systemd-journal from the end
+
+-t INT::
+ Throttle problem directory creation to 1 per INT second
+
+-T::
+ Same as -t INT, INT is specified in plugins/CCpp.conf
+
+-f::
+ Follow systemd-journal from the last seen position (if available)
+
+SEE ALSO
+--------
+abrt.conf(5), journalctl(1)
+
+AUTHORS
+-------
+* ABRT team
--
2.6.3

View file

@ -0,0 +1,31 @@
From cf9e5388ac7d94ab5aa34b274c32d735b702c48a Mon Sep 17 00:00:00 2001
From: Matej Habrnal <mhabrnal@redhat.com>
Date: Mon, 26 Oct 2015 15:21:07 +0100
Subject: [PATCH] a-d-journal-core: set root owner for created dump directory
Without this commit abrt-server fails during check the right owner and group of
dump dirs.
Related to rhbz#1269827
Signed-off-by: Matej Habrnal <mhabrnal@redhat.com>
---
src/plugins/abrt-dump-journal-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/plugins/abrt-dump-journal-core.c b/src/plugins/abrt-dump-journal-core.c
index 7ad69e8..02d5afe 100644
--- a/src/plugins/abrt-dump-journal-core.c
+++ b/src/plugins/abrt-dump-journal-core.c
@@ -302,7 +302,7 @@ save_systemd_coredump_in_dump_directory(struct dump_dir *dd, struct crash_info *
static int
abrt_journal_core_to_abrt_problem(struct crash_info *info, const char *dump_location)
{
- struct dump_dir *dd = create_dump_dir(dump_location, "ccpp", info->ci_uid,
+ struct dump_dir *dd = create_dump_dir(dump_location, "ccpp", /*fs owner*/0,
(save_data_call_back)save_systemd_coredump_in_dump_directory, info);
if (dd != NULL)
--
2.6.3

View file

@ -0,0 +1,31 @@
From 8b6d1511bb89548c60a472ddbaefbf00743be1ea Mon Sep 17 00:00:00 2001
From: Matej Habrnal <mhabrnal@redhat.com>
Date: Tue, 3 Nov 2015 12:31:38 +0100
Subject: [PATCH] doc: a-a-analyze-xorg fix path to conf file
xorg.conf is no longer located in /etc/abrt/xorg.conf but in
/etc/abrt/plugins/xorg.conf.
Signed-off-by: Matej Habrnal <mhabrnal@redhat.com>
---
doc/abrt-action-analyze-xorg.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/abrt-action-analyze-xorg.txt b/doc/abrt-action-analyze-xorg.txt
index 880c6fb..aa72249 100644
--- a/doc/abrt-action-analyze-xorg.txt
+++ b/doc/abrt-action-analyze-xorg.txt
@@ -39,8 +39,8 @@ OPTIONS
FILES
-----
-/etc/abrt/xorg.conf
- List of modules which, when loaded, should make Xorg crashes non-reportable.
+/etc/abrt/plugins/xorg.conf
+ Configuration file for ABRT's tools which work with Xorg crashes
AUTHORS
-------
--
2.6.3

View file

@ -0,0 +1,26 @@
From 02fa8cc3f0c63b61b4994ac66e5c7e304013b25f Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Tue, 10 Nov 2015 13:22:21 +0100
Subject: [PATCH] a-a-s-p-data: fix segfault if GPGKeysDir isn't configured
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/daemon/abrt-action-save-package-data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/daemon/abrt-action-save-package-data.c b/src/daemon/abrt-action-save-package-data.c
index 7054f06..72c9878 100644
--- a/src/daemon/abrt-action-save-package-data.c
+++ b/src/daemon/abrt-action-save-package-data.c
@@ -88,7 +88,7 @@ static void load_gpg_keys(void)
}
const char *gpg_keys_dir = get_map_string_item_or_NULL(settings, "GPGKeysDir");
- if (strcmp(gpg_keys_dir, "") != 0)
+ if (gpg_keys_dir != NULL && strcmp(gpg_keys_dir, "") != 0)
{
log_debug("Reading gpg keys from '%s'", gpg_keys_dir);
GHashTable *done_set = g_hash_table_new(g_str_hash, g_str_equal);
--
2.6.3

View file

@ -0,0 +1,56 @@
From a16d12d27e5c12f3e4ab5defaf775a692c405206 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 11 Nov 2015 13:19:35 +0100
Subject: [PATCH] ccpp: make crashes of processes with locked memory
not-reportable
Lets begin with a simply policy preventing users from accidental
publication of problem data with security sensitive data.
"not-reportable" problems can still be auto-reported. That is not an
security issue because uReports does not contain any user data stored in
process' memory (only stack-trace without values local|global variables
and function arguments).
Related to #796.
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/hooks/abrt-hook-ccpp.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
index 809b45e..4b79900 100644
--- a/src/hooks/abrt-hook-ccpp.c
+++ b/src/hooks/abrt-hook-ccpp.c
@@ -868,6 +868,27 @@ int main(int argc, char** argv)
dd_save_text(dd, FILENAME_ABRT_VERSION, VERSION);
+ /* In case of errors, treat the process as if it has locked memory */
+ long unsigned lck_bytes = ULONG_MAX;
+ const char *vmlck = strstr(proc_pid_status, "VmLck:");
+ if (vmlck == NULL)
+ error_msg("/proc/%s/status does not contain 'VmLck:' line", pid_str);
+ else if (1 != sscanf(vmlck + 6, "%lu kB\n", &lck_bytes))
+ error_msg("Failed to parse 'VmLck:' line in /proc/%s/status", pid_str);
+
+ if (lck_bytes)
+ {
+ log_notice("Process %s of user %lu has locked memory",
+ pid_str, (long unsigned)uid);
+
+ dd_mark_as_notreportable(dd, "The process had locked memory "
+ "which usually indicates efforts to protect sensitive "
+ "data (passwords) from being written to disk.\n"
+ "In order to avoid sensitive information leakages, "
+ "ABRT will not allow you to report this problem to "
+ "bug tracking tools");
+ }
+
if (setting_SaveBinaryImage)
{
if (save_crashing_binary(pid, dd))
--
2.6.3

View file

@ -0,0 +1,103 @@
From 4e5dfba1a1a4d5c5e49b7f6320bd2cb8052f86d1 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 30 Sep 2015 11:50:18 +0200
Subject: [PATCH] a-a-i-d-to-abrt-cache: make own random temporary directory
The set-user-ID wrapper must use own new temporary directory in order to
avoid security issues with unpacking specially crafted debuginfo
packages that might be used to create files or symlinks anywhere on the
file system as the abrt user.
Withot the forking code the temporary directory would remain on the
filesystem in the case where all debuginfo data are already available.
This is caused by the fact that the underlying libreport functionality
accepts path to a desired temporary directory and creates it only if
necessary. Otherwise, the directory is not touched at all.
This commit addresses CVE-2015-5273
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/plugins/Makefile.am | 1 +
.../abrt-action-install-debuginfo-to-abrt-cache.c | 41 +++++++++++++++++++---
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/src/plugins/Makefile.am b/src/plugins/Makefile.am
index 1c1ff8a..c5ea1ec 100644
--- a/src/plugins/Makefile.am
+++ b/src/plugins/Makefile.am
@@ -328,6 +328,7 @@ abrt_action_install_debuginfo_to_abrt_cache_CPPFLAGS = \
-D_GNU_SOURCE \
-DBIN_DIR=\"$(bindir)\" \
-DSBIN_DIR=\"$(sbindir)\" \
+ -DLARGE_DATA_TMP_DIR=\"$(LARGE_DATA_TMP_DIR)\" \
$(LIBREPORT_CFLAGS) \
-Wall -Wwrite-strings \
-fPIE
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 81b1486..52d00de 100644
--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
+++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
@@ -108,8 +108,14 @@ int main(int argc, char **argv)
build_ids_self_fd = xasprintf("/proc/self/fd/%d", build_ids_fd);
}
- /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */
- const char *args[11];
+ char tmp_directory[] = LARGE_DATA_TMP_DIR"/abrt-tmp-debuginfo.XXXXXX";
+ if (mkdtemp(tmp_directory) == NULL)
+ perror_msg_and_die("Failed to create working directory");
+
+ log_info("Created working directory: %s", tmp_directory);
+
+ /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, -t, PATH, --, NULL */
+ const char *args[13];
{
const char *verbs[] = { "", "-v", "-vv", "-vvv" };
unsigned i = 0;
@@ -130,6 +136,8 @@ int main(int argc, char **argv)
args[i++] = "--repo";
args[i++] = repo;
}
+ args[i++] = "--tmpdir";
+ args[i++] = tmp_directory;
args[i++] = "--";
args[i] = NULL;
}
@@ -204,6 +212,31 @@ int main(int argc, char **argv)
umask(0022);
}
- execvp(EXECUTABLE, (char **)args);
- error_msg_and_die("Can't execute %s", EXECUTABLE);
+ pid_t pid = fork();
+ if (pid < 0)
+ perror_msg_and_die("fork");
+
+ if (pid == 0)
+ {
+ execvp(EXECUTABLE, (char **)args);
+ error_msg_and_die("Can't execute %s", EXECUTABLE);
+ }
+
+ int status;
+ if (safe_waitpid(pid, &status, 0) < 0)
+ perror_msg_and_die("waitpid");
+
+ if (rmdir(tmp_directory) >= 0)
+ log_info("Removed working directory: %s", tmp_directory);
+ else if (errno != ENOENT)
+ perror_msg("Failed to remove working directory");
+
+ /* Normal execution should exit here. */
+ if (WIFEXITED(status))
+ return WEXITSTATUS(status);
+
+ if (WIFSIGNALED(status))
+ error_msg_and_die("Child terminated with signal %d", WTERMSIG(status));
+
+ error_msg_and_die("Child exit failed");
}
--
2.6.3

View file

@ -0,0 +1,101 @@
From 1cb9013d7ec62b282fd404062e88371916a12fad Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 30 Sep 2015 12:17:47 +0200
Subject: [PATCH] conf: introduce DebugLevel
ABRT should ignore problems caused by ABRT tools if DebugLevel == 0.
DebugLevel is set to 0 by default.
Related to CVE-2015-5287
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
doc/abrt.conf.txt | 8 ++++++++
src/daemon/abrt.conf | 8 ++++++++
src/include/libabrt.h | 2 ++
src/lib/abrt_conf.c | 14 ++++++++++++++
4 files changed, 32 insertions(+)
diff --git a/doc/abrt.conf.txt b/doc/abrt.conf.txt
index 6626596..1276f55 100644
--- a/doc/abrt.conf.txt
+++ b/doc/abrt.conf.txt
@@ -36,6 +36,14 @@ DeleteUploaded = 'yes/no'::
or not.
The default value is 'no'.
+DebugLevel = '0-100':
+ Allows ABRT tools to detect problems in ABRT itself. By increasing the value
+ you can force ABRT to detect, process and report problems in ABRT. You have
+ to bare in mind that ABRT might fall into an infinite loop when handling
+ problems caused by itself.
+ The default is 0 (non debug mode).
+
+
SEE ALSO
--------
abrtd(8)
diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf
index 0050c6d..dbc2007 100644
--- a/src/daemon/abrt.conf
+++ b/src/daemon/abrt.conf
@@ -59,3 +59,11 @@ AutoreportingEnabled = no
# SPECIAL ROOT DIRECTORY IN USER MOUNT NAMESAPCE
#
# ExploreChroots = false
+
+# Allows ABRT tools to detect problems in ABRT itself. By increasing the value
+# you can force ABRT to detect, process and report problems in ABRT. You have
+# to bare in mind that ABRT might fall into an infinite loop when handling
+# problems caused by itself.
+# The default is 0 (non debug mode).
+#
+# DebugLevel = 0
diff --git a/src/include/libabrt.h b/src/include/libabrt.h
index da565f9..6f89959 100644
--- a/src/include/libabrt.h
+++ b/src/include/libabrt.h
@@ -77,6 +77,8 @@ extern char * g_settings_autoreporting_event;
extern bool g_settings_shortenedreporting;
#define g_settings_explorechroots abrt_g_settings_explorechroots
extern bool g_settings_explorechroots;
+#define g_settings_debug_level abrt_g_settings_debug_level
+extern unsigned int g_settings_debug_level;
#define load_abrt_conf abrt_load_abrt_conf
diff --git a/src/lib/abrt_conf.c b/src/lib/abrt_conf.c
index 46ff689..f623344 100644
--- a/src/lib/abrt_conf.c
+++ b/src/lib/abrt_conf.c
@@ -28,6 +28,7 @@ bool g_settings_autoreporting = 0;
char * g_settings_autoreporting_event = NULL;
bool g_settings_shortenedreporting = 0;
bool g_settings_explorechroots = 0;
+unsigned int g_settings_debug_level = 0;
void free_abrt_conf_data()
{
@@ -116,6 +117,19 @@ static void ParseCommon(map_string_t *settings, const char *conf_filename)
else
g_settings_explorechroots = false;
+ value = get_map_string_item_or_NULL(settings, "DebugLevel");
+ if (value)
+ {
+ char *end;
+ errno = 0;
+ unsigned long ul = strtoul(value, &end, 10);
+ if (errno || end == value || *end != '\0' || ul > INT_MAX)
+ error_msg("Error parsing %s setting: '%s'", "DebugLevel", value);
+ else
+ g_settings_debug_level = ul;
+ remove_map_string_item(settings, "DebugLevel");
+ }
+
GHashTableIter iter;
const char *name;
/*char *value; - already declared */
--
2.6.3

View file

@ -0,0 +1,40 @@
From 82264feebb3a816098e68f0dce1502521b6b7a92 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 30 Sep 2015 12:19:48 +0200
Subject: [PATCH] ccpp: ignore crashes of ABRT binaries if DebugLevel == 0
Prior this commit abrt-hook-ccpp was saved core file of any
crashed process executing program whose name starts with "abrt" in
DUMP_LOCATION.
ABRT does not check size constraints of these core files, so the files
could consume an uncontrolled amount of disk space.
Related to CVE-2015-5287
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/hooks/abrt-hook-ccpp.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
index 4b79900..4a31b81 100644
--- a/src/hooks/abrt-hook-ccpp.c
+++ b/src/hooks/abrt-hook-ccpp.c
@@ -703,6 +703,13 @@ int main(int argc, char** argv)
const char *last_slash = strrchr(executable, '/');
if (last_slash && strncmp(++last_slash, "abrt", 4) == 0)
{
+ if (g_settings_debug_level == 0)
+ {
+ log_warning("Ignoring crash of %s (SIG%s).",
+ executable, signame ? signame : signal_str);
+ goto cleanup_and_exit;
+ }
+
/* If abrtd/abrt-foo crashes, we don't want to create a _directory_,
* since that can make new copy of abrtd to process it,
* and maybe crash again...
--
2.6.3

View file

@ -0,0 +1,34 @@
From fce92be28e87f4bbcf637ec06cc3e28652628684 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 30 Sep 2015 12:24:32 +0200
Subject: [PATCH] ccpp: save abrt core files only to new files
Prior this commit abrt-hook-ccpp saved a core file generated by a
process running a program whose name starts with "abrt" in
DUMP_LOCATION/$(basename program)-coredump. If the file was a symlink,
the hook followed and wrote core file to the symlink's target.
Addresses CVE-2015-5287
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/hooks/abrt-hook-ccpp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
index 4a31b81..58d9c28 100644
--- a/src/hooks/abrt-hook-ccpp.c
+++ b/src/hooks/abrt-hook-ccpp.c
@@ -718,7 +718,8 @@ int main(int argc, char** argv)
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);
+ unlink(path);
+ int abrt_core_fd = xopen3(path, O_WRONLY | O_CREAT | O_EXCL, 0600);
off_t core_size = copyfd_eof(STDIN_FILENO, abrt_core_fd, COPYFD_SPARSE);
if (core_size < 0 || fsync(abrt_core_fd) != 0)
{
--
2.6.3

View file

@ -0,0 +1,97 @@
From d8a3bd0d464f5b75ac360f4ee4e3cc6927a09199 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 30 Sep 2015 14:13:35 +0200
Subject: [PATCH] lib: add convenient wrappers for ensuring writable dir
Replace lchown with fchown and chmod with fchmod.
Related CVE-2015-5287
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/include/libabrt.h | 4 ++++
src/lib/hooklib.c | 41 ++++++++++++++++++++++++++++++++++-------
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/src/include/libabrt.h b/src/include/libabrt.h
index 6f89959..b26dcc6 100644
--- a/src/include/libabrt.h
+++ b/src/include/libabrt.h
@@ -42,8 +42,12 @@ int low_free_space(unsigned setting_MaxCrashReportsSize, const char *dump_locati
#define trim_problem_dirs abrt_trim_problem_dirs
void trim_problem_dirs(const char *dirname, double cap_size, const char *exclude_path);
+#define ensure_writable_dir_id abrt_ensure_writable_dir_uid_git
+void ensure_writable_dir_uid_gid(const char *dir, mode_t mode, uid_t uid, gid_t gid);
#define ensure_writable_dir abrt_ensure_writable_dir
void ensure_writable_dir(const char *dir, mode_t mode, const char *user);
+#define ensure_writable_dir_group abrt_ensure_writable_dir_group
+void ensure_writable_dir_group(const char *dir, mode_t mode, const char *user, const char *group);
#define run_unstrip_n abrt_run_unstrip_n
char *run_unstrip_n(const char *dump_dir_name, unsigned timeout_sec);
#define get_backtrace abrt_get_backtrace
diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c
index 2b76eea..0daa144 100644
--- a/src/lib/hooklib.c
+++ b/src/lib/hooklib.c
@@ -476,23 +476,50 @@ int signal_is_fatal(int signal_no, const char **name)
return signame != NULL;
}
-void ensure_writable_dir(const char *dir, mode_t mode, const char *user)
+void ensure_writable_dir_uid_gid(const char *dir, mode_t mode, uid_t uid, gid_t gid)
{
struct stat sb;
+ int dir_fd;
if (mkdir(dir, mode) != 0 && errno != EEXIST)
perror_msg_and_die("Can't create '%s'", dir);
- if (stat(dir, &sb) != 0 || !S_ISDIR(sb.st_mode))
- error_msg_and_die("'%s' is not a directory", dir);
+ dir_fd = open(dir, O_DIRECTORY | O_NOFOLLOW);
+ if (dir_fd < 0)
+ perror_msg_and_die("Can't open directory '%s'", dir);
+
+ if (fstat(dir_fd, &sb) != 0)
+ perror_msg_and_die("Can't stat directory '%s'", dir);
+
+ if ((sb.st_uid != uid || sb.st_gid != gid) && fchown(dir_fd, uid, gid) != 0)
+ perror_msg_and_die("Can't set owner %u:%u on '%s'", (unsigned int)uid, (unsigned int)gid, dir);
+
+ if ((sb.st_mode & 07777) != mode && fchmod(dir_fd, mode) != 0)
+ perror_msg_and_die("Can't set mode %o on '%s'", mode, dir);
+
+ close(dir_fd);
+}
+
+void ensure_writable_dir(const char *dir, mode_t mode, const char *user)
+{
+ struct passwd *pw = getpwnam(user);
+ if (!pw)
+ perror_msg_and_die("Can't find user '%s'", user);
+
+ ensure_writable_dir_uid_gid(dir, mode, pw->pw_uid, pw->pw_gid);
+}
+
+void ensure_writable_dir_group(const char *dir, mode_t mode, const char *user, const char *group)
+{
struct passwd *pw = getpwnam(user);
if (!pw)
perror_msg_and_die("Can't find user '%s'", user);
- if ((sb.st_uid != pw->pw_uid || sb.st_gid != pw->pw_gid) && lchown(dir, pw->pw_uid, pw->pw_gid) != 0)
- perror_msg_and_die("Can't set owner %u:%u on '%s'", (unsigned int)pw->pw_uid, (unsigned int)pw->pw_gid, dir);
- if ((sb.st_mode & 07777) != mode && chmod(dir, mode) != 0)
- perror_msg_and_die("Can't set mode %o on '%s'", mode, dir);
+ struct group *gr = getgrnam(group);
+ if (!gr)
+ perror_msg_and_die("Can't find group '%s'", group);
+
+ ensure_writable_dir_uid_gid(dir, mode, pw->pw_uid, gr->gr_gid);
}
bool dir_is_in_dump_location(const char *dir_name)
--
2.6.3

View file

@ -0,0 +1,29 @@
From 5653ddfeb61279df38e80ab18652afa68c964eb6 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 30 Sep 2015 14:14:31 +0200
Subject: [PATCH] abrtd: switch owner of the dump location to 'root'
Additional hardening suggested by Florian Weimer <fweimer@redhat.com>
Related to CVE-2015-5287
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
src/daemon/abrtd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/daemon/abrtd.c b/src/daemon/abrtd.c
index 0352eed..90a7163 100644
--- a/src/daemon/abrtd.c
+++ b/src/daemon/abrtd.c
@@ -195,7 +195,7 @@ static void sanitize_dump_dir_rights(void)
* us with thousands of bogus or malicious dumps */
/* 07000 bits are setuid, setgit, and sticky, and they must be unset */
/* 00777 bits are usual "rwxrwxrwx" access rights */
- ensure_writable_dir(g_settings_dump_location, DEFAULT_DUMP_LOCATION_MODE, "abrt");
+ ensure_writable_dir_group(g_settings_dump_location, DEFAULT_DUMP_LOCATION_MODE, "root", "abrt");
/* temp dir */
ensure_writable_dir(VAR_RUN"/abrt", 0755, "root");
}
--
2.6.3

View file

@ -49,7 +49,7 @@
Summary: Automatic bug detection and reporting tool
Name: abrt
Version: 2.6.1
Release: 6%{?dist}
Release: 7%{?dist}
License: GPLv2+
Group: Applications/System
URL: https://abrt.readthedocs.org/
@ -88,6 +88,20 @@ Patch0026: 0026-abrt-dump-xorg-support-Xorg-log-backtraces-prefixed-.patch
Patch0027: 0027-a-a-a-ccpp-local-don-t-delete-build_ids.patch
Patch0028: 0028-abrt-retrace-client-use-atoll-for-_size-conversion.patch
Patch0029: 0029-doc-fix-default-DumpLocation-in-abrt.conf-man-page.patch
Patch0030: 0030-dbus-ensure-expected-bytes-width-of-DBus-numbers.patch
#Patch0031: 0031-spec-add-missing-man-page-for-abrt-dump-journal-core.patch
Patch0032: 0032-doc-add-missing-man-page-for-abrt-dump-journal-core.patch
Patch0033: 0033-a-d-journal-core-set-root-owner-for-created-dump-dir.patch
Patch0034: 0034-doc-a-a-analyze-xorg-fix-path-to-conf-file.patch
Patch0035: 0035-a-a-s-p-data-fix-segfault-if-GPGKeysDir-isn-t-config.patch
Patch0036: 0036-ccpp-make-crashes-of-processes-with-locked-memory-no.patch
Patch0037: 0037-a-a-i-d-to-abrt-cache-make-own-random-temporary-dire.patch
Patch0038: 0038-conf-introduce-DebugLevel.patch
Patch0039: 0039-ccpp-ignore-crashes-of-ABRT-binaries-if-DebugLevel-0.patch
Patch0040: 0040-ccpp-save-abrt-core-files-only-to-new-files.patch
Patch0041: 0041-lib-add-convenient-wrappers-for-ensuring-writable-di.patch
Patch0042: 0042-abrtd-switch-owner-of-the-dump-location-to-root.patch
#Patch0043: 0043-spec-switch-owner-of-the-dump-location-to-root.patch
# '%%autosetup -S git' -> git
BuildRequires: git
@ -764,7 +778,7 @@ killall abrt-dbus >/dev/null 2>&1 || :
%{_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(0751, abrt, abrt) %{_localstatedir}/spool/%{name}
%dir %attr(0751, root, 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}
@ -894,6 +908,7 @@ killall abrt-dbus >/dev/null 2>&1 || :
%{_mandir}/man*/abrt-action-analyze-core.*
%{_mandir}/man*/abrt-action-analyze-vulnerability.*
%{_mandir}/man*/abrt-action-perform-ccpp-analysis.*
%{_mandir}/man*/abrt-dump-journal-core.*
%files addon-upload-watch
%defattr(-,root,root,-)
@ -1076,6 +1091,17 @@ killall abrt-dbus >/dev/null 2>&1 || :
%config(noreplace) %{_sysconfdir}/profile.d/abrt-console-notification.sh
%changelog
* Wed Nov 25 2015 Jakub Filak <jfilak@redhat.com> - 2.6.1-7
- CVE-2015-5287: switch owner of /var/spool/abrt to 'root'
- CVE-2015-5287: ccpp: save abrt core files only if DebugLevel > 0
- CVE-2015-5287: ccpp: save abrt core files only to new files
- CVE-2015-5287: abrt configuration: introduce DebugLevel
- CVE-2015-5273: a-a-i-d-to-abrt-cache: make own random temporary directory
- ccpp: make crashes of processes with locked memory not-reportable
- a-d-journal-core: set root owner for created dump directory
- spec: add missing man page for abrt-dump-journal-core
- Resolves: #1262252, #1284557
* Fri Oct 16 2015 Matej Habrnal <mhabrnal@redhat.com> 2.6.1-6
- doc: fix default DumpLocation in abrt.conf man page
- abrt-retrace-client: use atoll for _size conversion