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