doczkal / rpms / abrt

Forked from rpms/abrt 4 years ago
Clone

Blame SOURCES/0108-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch

baab13
From 7814554e0827ece778ca88fd90832bd4d05520b1 Mon Sep 17 00:00:00 2001
baab13
From: Jakub Filak <jfilak@redhat.com>
baab13
Date: Fri, 24 Apr 2015 13:48:32 +0200
baab13
Subject: [ABRT PATCH] dbus: avoid race-conditions in tests for dum dir
baab13
 availability
baab13
baab13
Florian Weimer <fweimer@redhat.com>
baab13
baab13
    dump_dir_accessible_by_uid() is fundamentally insecure because it
baab13
    opens up a classic time-of-check-time-of-use race between this
baab13
    function and and dd_opendir().
baab13
baab13
Related: #1214745
baab13
baab13
Signed-off-by: Jakub Filak <jfilak@redhat.com>
baab13
---
baab13
 src/dbus/abrt-dbus.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++-------
baab13
 src/lib/problem_api.c | 15 ++++++++++--
baab13
 2 files changed, 71 insertions(+), 10 deletions(-)
baab13
baab13
diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
baab13
index 7400dff..9e1844a 100644
baab13
--- a/src/dbus/abrt-dbus.c
baab13
+++ b/src/dbus/abrt-dbus.c
baab13
@@ -245,7 +245,15 @@ static struct dump_dir *open_directory_for_modification_of_element(
baab13
         }
baab13
     }
baab13
 
baab13
-    if (!dump_dir_accessible_by_uid(problem_id, caller_uid))
baab13
+    int dir_fd = dd_openfd(problem_id);
baab13
+    if (dir_fd < 0)
baab13
+    {
baab13
+        perror_msg("can't open problem directory '%s'", problem_id);
baab13
+        return_InvalidProblemDir_error(invocation, problem_id);
baab13
+        return NULL;
baab13
+    }
baab13
+
baab13
+    if (!fdump_dir_accessible_by_uid(dir_fd, caller_uid))
baab13
     {
baab13
         if (errno == ENOTDIR)
baab13
         {
baab13
@@ -260,10 +268,11 @@ static struct dump_dir *open_directory_for_modification_of_element(
baab13
                                 _("Not Authorized"));
baab13
         }
baab13
 
baab13
+        close(dir_fd);
baab13
         return NULL;
baab13
     }
baab13
 
baab13
-    struct dump_dir *dd = dd_opendir(problem_id, /* flags : */ 0);
baab13
+    struct dump_dir *dd = dd_fdopendir(dir_fd, problem_id, /* flags : */ 0);
baab13
     if (!dd)
baab13
     {   /* This should not happen because of the access check above */
baab13
         log_notice("Can't access the problem '%s' for modification", problem_id);
baab13
@@ -429,7 +438,15 @@ static void handle_method_call(GDBusConnection *connection,
baab13
             return;
baab13
         }
baab13
 
baab13
-        int ddstat = dump_dir_stat_for_uid(problem_dir, caller_uid);
baab13
+        int dir_fd = dd_openfd(problem_dir);
baab13
+        if (dir_fd < 0)
baab13
+        {
baab13
+            perror_msg("can't open problem directory '%s'", problem_dir);
baab13
+            return_InvalidProblemDir_error(invocation, problem_dir);
baab13
+            return;
baab13
+        }
baab13
+
baab13
+        int ddstat = fdump_dir_stat_for_uid(dir_fd, caller_uid);
baab13
         if (ddstat < 0)
baab13
         {
baab13
             if (errno == ENOTDIR)
baab13
@@ -443,6 +460,7 @@ static void handle_method_call(GDBusConnection *connection,
baab13
 
baab13
             return_InvalidProblemDir_error(invocation, problem_dir);
baab13
 
baab13
+            close(dir_fd);
baab13
             return;
baab13
         }
baab13
 
baab13
@@ -450,6 +468,7 @@ static void handle_method_call(GDBusConnection *connection,
baab13
         {   //caller seems to be in group with access to this dir, so no action needed
baab13
             log_notice("caller has access to the requested directory %s", problem_dir);
baab13
             g_dbus_method_invocation_return_value(invocation, NULL);
baab13
+            close(dir_fd);
baab13
             return;
baab13
         }
baab13
 
baab13
@@ -460,10 +479,11 @@ static void handle_method_call(GDBusConnection *connection,
baab13
             g_dbus_method_invocation_return_dbus_error(invocation,
baab13
                                               "org.freedesktop.problems.AuthFailure",
baab13
                                               _("Not Authorized"));
baab13
+            close(dir_fd);
baab13
             return;
baab13
         }
baab13
 
baab13
-        struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES);
baab13
+        struct dump_dir *dd = dd_fdopendir(dir_fd, problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES);
baab13
         if (!dd)
baab13
         {
baab13
             return_InvalidProblemDir_error(invocation, problem_dir);
baab13
@@ -497,12 +517,21 @@ static void handle_method_call(GDBusConnection *connection,
baab13
             return;
baab13
         }
baab13
 
baab13
-        if (!dump_dir_accessible_by_uid(problem_dir, caller_uid))
baab13
+        int dir_fd = dd_openfd(problem_dir);
baab13
+        if (dir_fd < 0)
baab13
+        {
baab13
+            perror_msg("can't open problem directory '%s'", problem_dir);
baab13
+            return_InvalidProblemDir_error(invocation, problem_dir);
baab13
+            return;
baab13
+        }
baab13
+
baab13
+        if (!fdump_dir_accessible_by_uid(dir_fd, caller_uid))
baab13
         {
baab13
             if (errno == ENOTDIR)
baab13
             {
baab13
                 log_notice("Requested directory does not exist '%s'", problem_dir);
baab13
                 return_InvalidProblemDir_error(invocation, problem_dir);
baab13
+                close(dir_fd);
baab13
                 return;
baab13
             }
baab13
 
baab13
@@ -512,11 +541,12 @@ static void handle_method_call(GDBusConnection *connection,
baab13
                 g_dbus_method_invocation_return_dbus_error(invocation,
baab13
                                                   "org.freedesktop.problems.AuthFailure",
baab13
                                                   _("Not Authorized"));
baab13
+                close(dir_fd);
baab13
                 return;
baab13
             }
baab13
         }
baab13
 
baab13
-        struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES);
baab13
+        struct dump_dir *dd = dd_fdopendir(dir_fd, problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES);
baab13
         if (!dd)
baab13
         {
baab13
             return_InvalidProblemDir_error(invocation, problem_dir);
baab13
@@ -677,20 +707,40 @@ static void handle_method_call(GDBusConnection *connection,
baab13
         for (GList *l = problem_dirs; l; l = l->next)
baab13
         {
baab13
             const char *dir_name = (const char*)l->data;
baab13
-            if (!dump_dir_accessible_by_uid(dir_name, caller_uid))
baab13
+
baab13
+            int dir_fd = dd_openfd(dir_name);
baab13
+            if (dir_fd < 0)
baab13
+            {
baab13
+                perror_msg("can't open problem directory '%s'", dir_name);
baab13
+                return_InvalidProblemDir_error(invocation, dir_name);
baab13
+                return;
baab13
+            }
baab13
+
baab13
+            if (!fdump_dir_accessible_by_uid(dir_fd, caller_uid))
baab13
             {
baab13
                 if (errno == ENOTDIR)
baab13
                 {
baab13
                     log_notice("Requested directory does not exist '%s'", dir_name);
baab13
+                    close(dir_fd);
baab13
                     continue;
baab13
                 }
baab13
 
baab13
                 if (polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes)
baab13
                 { // if user didn't provide correct credentials, just move to the next dir
baab13
+                    close(dir_fd);
baab13
                     continue;
baab13
                 }
baab13
             }
baab13
-            delete_dump_dir(dir_name);
baab13
+
baab13
+            struct dump_dir *dd = dd_fdopendir(dir_fd, dir_name, /*flags:*/ 0);
baab13
+            if (dd)
baab13
+            {
baab13
+                if (dd_delete(dd) != 0)
baab13
+                {
baab13
+                    error_msg("Failed to delete problem directory '%s'", dir_name);
baab13
+                    dd_close(dd);
baab13
+                }
baab13
+            }
baab13
         }
baab13
 
baab13
         g_dbus_method_invocation_return_value(invocation, NULL);
baab13
diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c
baab13
index c2b4b1c..b343882 100644
baab13
--- a/src/lib/problem_api.c
baab13
+++ b/src/lib/problem_api.c
baab13
@@ -46,7 +46,15 @@ int for_each_problem_in_dir(const char *path,
baab13
             continue; /* skip "." and ".." */
baab13
 
baab13
         char *full_name = concat_path_file(path, dent->d_name);
baab13
-        if (caller_uid == -1 || dump_dir_accessible_by_uid(full_name, caller_uid))
baab13
+
baab13
+        int dir_fd = dd_openfd(full_name);
baab13
+        if (dir_fd < 0)
baab13
+        {
baab13
+            VERB2 perror_msg("can't open problem directory '%s'", full_name);
baab13
+            continue;
baab13
+        }
baab13
+
baab13
+        if (caller_uid == -1 || fdump_dir_accessible_by_uid(dir_fd, caller_uid))
baab13
         {
baab13
             /* Silently ignore *any* errors, not only EACCES.
baab13
              * We saw "lock file is locked by process PID" error
baab13
@@ -54,7 +62,7 @@ int for_each_problem_in_dir(const char *path,
baab13
              */
baab13
             int sv_logmode = logmode;
baab13
             logmode = 0;
baab13
-            struct dump_dir *dd = dd_opendir(full_name, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES | DD_DONT_WAIT_FOR_LOCK);
baab13
+            struct dump_dir *dd = dd_fdopendir(dir_fd, full_name, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES | DD_DONT_WAIT_FOR_LOCK);
baab13
             logmode = sv_logmode;
baab13
             if (dd)
baab13
             {
baab13
@@ -62,6 +70,9 @@ int for_each_problem_in_dir(const char *path,
baab13
                 dd_close(dd);
baab13
             }
baab13
         }
baab13
+        else
baab13
+            close(dir_fd);
baab13
+
baab13
         free(full_name);
baab13
         if (brk)
baab13
             break;
baab13
-- 
baab13
1.8.3.1
baab13