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