From c5cd9d25473e2fea29f7fa609c81ae821e50f118 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Thu, 23 Apr 2015 14:46:27 +0200
Subject: [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 <jfilak@redhat.com>
---
 src/dbus/abrt-dbus.c  | 46 ++++++++++++++++++++++++++++++++++------------
 src/lib/problem_api.c |  7 +++++++
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
index 4eeff41..585fcd7 100644
--- a/src/dbus/abrt-dbus.c
+++ b/src/dbus/abrt-dbus.c
@@ -141,21 +141,20 @@ 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 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_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;
     }
-#endif
+
+    if (!dir_has_correct_permissions(dir_name, DD_PERM_DAEMONS))
+    {
+        error_msg("Problem directory '%s' has invalid owner, groop or mode", dir_name);
+        return false;
+    }
+
     return true;
 }
 
@@ -561,6 +560,12 @@ static void handle_method_call(GDBusConnection *connection,
 
         g_variant_get(parameters, "(&s)", &problem_id);
 
+        if (!allowed_problem_dir(problem_id))
+        {
+            return_InvalidProblemDir_error(invocation, problem_id);
+            return;
+        }
+
         int ddstat = dump_dir_stat_for_uid(problem_id, caller_uid);
         if ((ddstat & DD_STAT_ACCESSIBLE_BY_UID) == 0 &&
                 polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes)
@@ -624,6 +629,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 (element == NULL || element[0] == '\0' || strlen(element) > 64)
         {
             log_notice("'%s' is not a valid element name of '%s'", element, problem_id);
@@ -683,6 +694,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;
+        }
+
         struct dump_dir *dd = open_directory_for_modification_of_element(
                                     invocation, caller_uid, problem_id, element);
         if (!dd)
@@ -715,6 +732,11 @@ 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;
+        }
 
         struct dump_dir *dd = dd_opendir(problem_id, DD_OPEN_READONLY);
         if (!dd)
diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c
index 07707db..86222cf 100644
--- a/src/lib/problem_api.c
+++ b/src/lib/problem_api.c
@@ -76,6 +76,13 @@ int for_each_problem_in_dir(const char *path,
 
 static int add_dirname_to_GList(struct dump_dir *dd, void *arg)
 {
+    if (!dir_has_correct_permissions(dd->dd_dirname, DD_PERM_DAEMONS))
+    {
+        log("Ignoring '%s': invalid owner, group or mode", dd->dd_dirname);
+        /*Do not break*/
+        return 0;
+    }
+
     GList **list = arg;
     *list = g_list_prepend(*list, xstrdup(dd->dd_dirname));
     return 0;
-- 
2.1.0