|
 |
a60cd7 |
From 6e811d78e2719988ae291181f5b133af32ce62d8 Mon Sep 17 00:00:00 2001
|
|
|
a60cd7 |
From: Jakub Filak <jfilak@redhat.com>
|
|
|
a60cd7 |
Date: Thu, 23 Apr 2015 14:46:27 +0200
|
|
|
a60cd7 |
Subject: [ABRT PATCH] dbus: process only valid sub-directories of the dump
|
|
|
a60cd7 |
location
|
|
|
a60cd7 |
|
|
|
a60cd7 |
Must have correct rights and must be a direct sub-directory of the dump
|
|
|
a60cd7 |
location.
|
|
|
a60cd7 |
|
|
|
a60cd7 |
This issue was discovered by Florian Weimer of Red Hat Product Security.
|
|
|
a60cd7 |
|
|
|
a60cd7 |
Related: #1214451
|
|
|
a60cd7 |
|
|
|
a60cd7 |
Signed-off-by: Jakub Filak <jfilak@redhat.com>
|
|
|
a60cd7 |
---
|
|
|
a60cd7 |
src/dbus/abrt-dbus.c | 36 ++++++++++++++++++++++++++----------
|
|
|
a60cd7 |
1 file changed, 26 insertions(+), 10 deletions(-)
|
|
|
a60cd7 |
|
|
|
a60cd7 |
diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
|
|
|
a60cd7 |
index 308a9af..7400dff 100644
|
|
|
a60cd7 |
--- a/src/dbus/abrt-dbus.c
|
|
|
a60cd7 |
+++ b/src/dbus/abrt-dbus.c
|
|
|
a60cd7 |
@@ -132,18 +132,34 @@ static uid_t get_caller_uid(GDBusConnection *connection, GDBusMethodInvocation *
|
|
|
a60cd7 |
return caller_uid;
|
|
|
a60cd7 |
}
|
|
|
a60cd7 |
|
|
|
a60cd7 |
-static bool allowed_problem_dir(const char *dir_name)
|
|
|
a60cd7 |
+bool allowed_problem_dir(const char *dir_name)
|
|
|
a60cd7 |
{
|
|
|
a60cd7 |
-//HACK HACK HACK! Disabled for now until we fix clients (abrt-gui) to not pass /home/user/.cache/abrt/spool
|
|
|
a60cd7 |
+ if (!dir_is_in_dump_location(dir_name))
|
|
|
a60cd7 |
+ {
|
|
|
a60cd7 |
+ error_msg("Bad problem directory name '%s', should start with: '%s'", dir_name, g_settings_dump_location);
|
|
|
a60cd7 |
+ return false;
|
|
|
a60cd7 |
+ }
|
|
|
a60cd7 |
+
|
|
|
a60cd7 |
+ /* We cannot test correct permissions yet because we still need to chown
|
|
|
a60cd7 |
+ * dump directories before reporting and Chowing changes the file owner to
|
|
|
a60cd7 |
+ * the reporter, which causes this test to fail and prevents users from
|
|
|
a60cd7 |
+ * getting problem data after reporting it.
|
|
|
a60cd7 |
+ *
|
|
|
a60cd7 |
+ * Fortunately, libreport has been hardened against hard link and symbolic
|
|
|
a60cd7 |
+ * link attacks and refuses to work with such files, so this test isn't
|
|
|
a60cd7 |
+ * really necessary, however, we will use it once we get rid of the
|
|
|
a60cd7 |
+ * chowning files.
|
|
|
a60cd7 |
+ *
|
|
|
a60cd7 |
+ * abrt-server refuses to run post-create on directories that have
|
|
|
a60cd7 |
+ * incorrect owner (not "root:(abrt|root)"), incorrect permissions (other
|
|
|
a60cd7 |
+ * bits are not 0) and are complete (post-create finished). So, there is no
|
|
|
a60cd7 |
+ * way to run security sensitive event scripts (post-create) on crafted
|
|
|
a60cd7 |
+ * problem directories.
|
|
|
a60cd7 |
+ */
|
|
|
a60cd7 |
#if 0
|
|
|
a60cd7 |
- unsigned len = strlen(g_settings_dump_location);
|
|
|
a60cd7 |
-
|
|
|
a60cd7 |
- /* If doesn't start with "g_settings_dump_location[/]"... */
|
|
|
a60cd7 |
- if (strncmp(dir_name, g_settings_dump_location, len) != 0
|
|
|
a60cd7 |
- || (dir_name[len] != '/' && dir_name[len] != '\0')
|
|
|
a60cd7 |
- /* or contains "/." anywhere (-> might contain ".." component) */
|
|
|
a60cd7 |
- || strstr(dir_name + len, "/.")
|
|
|
a60cd7 |
- ) {
|
|
|
a60cd7 |
+ if (!dir_has_correct_permissions(dir_name))
|
|
|
a60cd7 |
+ {
|
|
|
a60cd7 |
+ error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dir_name);
|
|
|
a60cd7 |
return false;
|
|
|
a60cd7 |
}
|
|
|
a60cd7 |
#endif
|
|
|
a60cd7 |
--
|
|
|
a60cd7 |
1.8.3.1
|
|
|
a60cd7 |
|