From 0bfb03ea68eb745172feccfc0f01b2ad13ff33bb Mon Sep 17 00:00:00 2001

From: Jakub Filak <jfilak@redhat.com>

Date: Fri, 24 Apr 2015 13:48:32 +0200

Subject: [PATCH] dbus: avoid race-conditions in tests for dum dir availability

Florian Weimer <fweimer@redhat.com>

    dump_dir_accessible_by_uid() is fundamentally insecure because it

    opens up a classic time-of-check-time-of-use race between this

    function and and dd_opendir().

Related: #1214745

Signed-off-by: Jakub Filak <jfilak@redhat.com>

---

 src/dbus/abrt-dbus.c  | 225 +++++++++++++++++++++-----------------------------

 src/lib/problem_api.c |  21 +++--

 2 files changed, 109 insertions(+), 137 deletions(-)

diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c

index 585fcd7..0f7ac2d 100644

--- a/src/dbus/abrt-dbus.c

+++ b/src/dbus/abrt-dbus.c

@@ -201,6 +201,69 @@ static void return_InvalidProblemDir_error(GDBusMethodInvocation *invocation, co

     free(msg);

 }

+enum {

+    OPEN_FAIL_NO_REPLY = 1 << 0,

+    OPEN_AUTH_ASK      = 1 << 1,

+    OPEN_AUTH_FAIL     = 1 << 2,

+};

+

+static struct dump_dir *open_dump_directory(GDBusMethodInvocation *invocation,

+    const gchar *caller, uid_t caller_uid, const char *problem_dir, int dd_flags, int flags)

+{

+    if (!allowed_problem_dir(problem_dir))

+    {

+        log("UID=%d attempted to access not allowed problem directory '%s'",

+                caller_uid, problem_dir);

+        if (!(flags & OPEN_FAIL_NO_REPLY))

+            return_InvalidProblemDir_error(invocation, problem_dir);

+        return NULL;

+    }

+

+    struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_FD_ONLY);

+    if (dd == NULL)

+    {

+        perror_msg("can't open problem directory '%s'", problem_dir);

+        if (!(flags & OPEN_FAIL_NO_REPLY))

+            return_InvalidProblemDir_error(invocation, problem_dir);

+        return NULL;

+    }

+

+    if (!dd_accessible_by_uid(dd, caller_uid))

+    {

+        if (errno == ENOTDIR)

+        {

+            log_notice("Requested directory does not exist '%s'", problem_dir);

+            if (!(flags & OPEN_FAIL_NO_REPLY))

+                return_InvalidProblemDir_error(invocation, problem_dir);

+            dd_close(dd);

+            return NULL;

+        }

+

+        if (   !(flags & OPEN_AUTH_ASK)

+            || polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes)

+        {

+            log_notice("not authorized");

+            if (!(flags & OPEN_FAIL_NO_REPLY))

+                g_dbus_method_invocation_return_dbus_error(invocation,

+                                              "org.freedesktop.problems.AuthFailure",

+                                              _("Not Authorized"));

+            dd_close(dd);

+            return NULL;

+        }

+    }

+

+    dd = dd_fdopendir(dd, dd_flags);

+    if (dd == NULL)

+    {

+        log_notice("Can't open the problem '%s' with flags %x0", problem_dir, dd_flags);

+        if (!(flags & OPEN_FAIL_NO_REPLY))

+            g_dbus_method_invocation_return_dbus_error(invocation,

+                                "org.freedesktop.problems.Failure",

+                                _("Can't open the problem"));

+    }

+    return dd;

+}

+

 /*

  * Checks element's rights and does not open directory if element is protected.

  * Checks problem's rights and does not open directory if user hasn't got

@@ -237,35 +300,8 @@ static struct dump_dir *open_directory_for_modification_of_element(

         }

     }

-    if (!dump_dir_accessible_by_uid(problem_id, caller_uid))

-    {

-        if (errno == ENOTDIR)

-        {

-            log_notice("'%s' is not a valid problem directory", problem_id);

-            return_InvalidProblemDir_error(invocation, problem_id);

-        }

-        else

-        {

-            log_notice("UID(%d) is not Authorized to access '%s'", caller_uid, problem_id);

-            g_dbus_method_invocation_return_dbus_error(invocation,

-                                "org.freedesktop.problems.AuthFailure",

-                                _("Not Authorized"));

-        }

-

-        return NULL;

-    }

-

-    struct dump_dir *dd = dd_opendir(problem_id, /* flags : */ 0);

-    if (!dd)

-    {   /* This should not happen because of the access check above */

-        log_notice("Can't access the problem '%s' for modification", problem_id);

-        g_dbus_method_invocation_return_dbus_error(invocation,

-                                "org.freedesktop.problems.Failure",

-                                _("Can't access the problem for modification"));

-        return NULL;

-    }

-

-    return dd;

+    return open_dump_directory(invocation, /*caller*/NULL, caller_uid, problem_id, /*Read/Write*/0,

+                               OPEN_AUTH_FAIL);

 }

@@ -421,7 +457,15 @@ static void handle_method_call(GDBusConnection *connection,

             return;

         }

-        int ddstat = dump_dir_stat_for_uid(problem_dir, caller_uid);

+        struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_FD_ONLY);

+        if (dd == NULL)

+        {

+            perror_msg("can't open problem directory '%s'", problem_dir);

+            return_InvalidProblemDir_error(invocation, problem_dir);

+            return;

+        }

+

+        int ddstat = dd_stat_for_uid(dd, caller_uid);

         if (ddstat < 0)

         {

             if (errno == ENOTDIR)

@@ -435,6 +479,7 @@ static void handle_method_call(GDBusConnection *connection,

             return_InvalidProblemDir_error(invocation, problem_dir);

+            dd_close(dd);

             return;

         }

@@ -442,6 +487,7 @@ static void handle_method_call(GDBusConnection *connection,

         {   //caller seems to be in group with access to this dir, so no action needed

             log_notice("caller has access to the requested directory %s", problem_dir);

             g_dbus_method_invocation_return_value(invocation, NULL);

+            dd_close(dd);

             return;

         }

@@ -452,10 +498,11 @@ static void handle_method_call(GDBusConnection *connection,

             g_dbus_method_invocation_return_dbus_error(invocation,

                                               "org.freedesktop.problems.AuthFailure",

                                               _("Not Authorized"));

+            dd_close(dd);

             return;

         }

-        struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES);

+        dd = dd_fdopendir(dd, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES);

         if (!dd)

         {

             return_InvalidProblemDir_error(invocation, problem_dir);

@@ -483,37 +530,10 @@ static void handle_method_call(GDBusConnection *connection,

         g_variant_get_child(parameters, 0, "&s", &problem_dir);

         log_notice("problem_dir:'%s'", problem_dir);

-        if (!allowed_problem_dir(problem_dir))

-        {

-            return_InvalidProblemDir_error(invocation, problem_dir);

-            return;

-        }

-

-        if (!dump_dir_accessible_by_uid(problem_dir, caller_uid))

-        {

-            if (errno == ENOTDIR)

-            {

-                log_notice("Requested directory does not exist '%s'", problem_dir);

-                return_InvalidProblemDir_error(invocation, problem_dir);

-                return;

-            }

-

-            if (polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes)

-            {

-                log_notice("not authorized");

-                g_dbus_method_invocation_return_dbus_error(invocation,

-                                                  "org.freedesktop.problems.AuthFailure",

-                                                  _("Not Authorized"));

-                return;

-            }

-        }

-

-        struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES);

+        struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid,

+                problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES , OPEN_AUTH_ASK);

         if (!dd)

-        {

-            return_InvalidProblemDir_error(invocation, problem_dir);

             return;

-        }

 	/* Get 2nd param - vector of element names */

         GVariant *array = g_variant_get_child_value(parameters, 1);

@@ -560,32 +580,10 @@ 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)

-        {

-            log_notice("Unauthorized access : '%s'", problem_id);

-            g_dbus_method_invocation_return_dbus_error(invocation,

-                                              "org.freedesktop.problems.AuthFailure",

-                                              _("Not Authorized"));

-            return;

-        }

-

-        struct dump_dir *dd = dd_opendir(problem_id, DD_OPEN_READONLY);

-        if (dd == NULL)

-        {

-            log_notice("Can't access the problem '%s' for reading", problem_id);

-            g_dbus_method_invocation_return_dbus_error(invocation,

-                                    "org.freedesktop.problems.Failure",

-                                    _("Can't access the problem for reading"));

+        struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid,

+                    problem_id, DD_OPEN_READONLY, OPEN_AUTH_ASK);

+        if (!dd)

             return;

-        }

         problem_data_t *pd = create_problem_data_from_dump_dir(dd);

         dd_close(dd);

@@ -629,12 +627,6 @@ 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);

@@ -694,12 +686,6 @@ 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)

@@ -732,33 +718,10 @@ 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);

+        struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid,

+                problem_id, DD_OPEN_READONLY, OPEN_AUTH_ASK);

         if (!dd)

-        {

-            log_notice("Can't access the problem '%s'", problem_id);

-            g_dbus_method_invocation_return_dbus_error(invocation,

-                                    "org.freedesktop.problems.Failure",

-                                    _("Can't access the problem"));

-            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)

-        {

-            dd_close(dd);

-            log_notice("Unauthorized access : '%s'", problem_id);

-            g_dbus_method_invocation_return_dbus_error(invocation,

-                                              "org.freedesktop.problems.AuthFailure",

-                                              _("Not Authorized"));

             return;

-        }

         int ret = dd_exist(dd, element);

         dd_close(dd);

@@ -793,20 +756,18 @@ static void handle_method_call(GDBusConnection *connection,

         for (GList *l = problem_dirs; l; l = l->next)

         {

             const char *dir_name = (const char*)l->data;

-            if (!dump_dir_accessible_by_uid(dir_name, caller_uid))

+

+            struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid,

+                        dir_name, /*Read/Write*/0, OPEN_FAIL_NO_REPLY | OPEN_AUTH_ASK);

+

+            if (dd)

             {

-                if (errno == ENOTDIR)

+                if (dd_delete(dd) != 0)

                 {

-                    log_notice("Requested directory does not exist '%s'", dir_name);

-                    continue;

-                }

-

-                if (polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes)

-                { // if user didn't provide correct credentials, just move to the next dir

-                    continue;

+                    error_msg("Failed to delete problem directory '%s'", dir_name);

+                    dd_close(dd);

                 }

             }

-            delete_dump_dir(dir_name);

         }

         g_dbus_method_invocation_return_value(invocation, NULL);

diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c

index 86222cf..96a49fc 100644

--- a/src/lib/problem_api.c

+++ b/src/lib/problem_api.c

@@ -46,7 +46,17 @@ int for_each_problem_in_dir(const char *path,

             continue; /* skip "." and ".." */

         char *full_name = concat_path_file(path, dent->d_name);

-        if (caller_uid == -1 || dump_dir_accessible_by_uid(full_name, caller_uid))

+

+        struct dump_dir *dd = dd_opendir(full_name,   DD_OPEN_FD_ONLY

+                                                    | DD_FAIL_QUIETLY_ENOENT

+                                                    | DD_FAIL_QUIETLY_EACCES);

+        if (dd == NULL)

+        {

+            VERB2 perror_msg("can't open problem directory '%s'", full_name);

+            continue;

+        }

+

+        if (caller_uid == -1 || dd_accessible_by_uid(dd, caller_uid))

         {

             /* Silently ignore *any* errors, not only EACCES.

              * We saw "lock file is locked by process PID" error

@@ -55,14 +65,15 @@ int for_each_problem_in_dir(const char *path,

             int sv_logmode = logmode;

             /* Silently ignore errors only in the silent log level. */

             logmode = g_verbose == 0 ? 0: sv_logmode;

-            struct dump_dir *dd = dd_opendir(full_name, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES | DD_DONT_WAIT_FOR_LOCK);

+            dd = dd_fdopendir(dd, DD_OPEN_READONLY | DD_DONT_WAIT_FOR_LOCK);

             logmode = sv_logmode;

             if (dd)

-            {

                 brk = callback ? callback(dd, arg) : 0;

-                dd_close(dd);

-            }

         }

+

+        if (dd)

+            dd_close(dd);

+

         free(full_name);

         if (brk)

             break;

-- 

2.1.0