File fix-upstream-CVE-2020-35512.patch of Package dbus-1-x11

Overview Repositories Revisions Requests Users Attributes Meta

File fix-upstream-CVE-2020-35512.patch of Package dbus-1-x11 (Revision 0985657dab80d85437d4085ba677756e)

Currently displaying revision 0985657dab80d85437d4085ba677756e , Show latest

commit 2b7948ef907669e844b52c4fa2268d6e3162a70c
Author: Simon McVittie <smcv@collabora.com>
Date:   Tue Jun 30 19:29:06 2020 +0100

userdb: Reference-count DBusUserInfo, DBusGroupInfo
    
    Previously, the hash table indexed by uid (or gid) took ownership of the
    single reference to the heap-allocated struct, and the hash table
    indexed by username (or group name) had a borrowed pointer to the same
    struct that exists in the other hash table.
    
    However, this can break down if you have two or more distinct usernames
    that share a numeric identifier. This is generally a bad idea, because
    the user-space model in such situations does not match the kernel-space
    reality, and in particular there is no effective kernel-level security
    boundary between such users, but it is sometimes done anyway.
    
    In this case, when the second username is looked up in the userdb, it
    overwrites (replaces) the entry in the hash table that is indexed by
    uid, freeing the DBusUserInfo. This results in both the key and the
    value in the hash table that is indexed by username becoming dangling
    pointers (use-after-free), leading to undefined behaviour, which is
    certainly not what we want to see when doing access control.
    
    An equivalent situation can occur with groups, in the rare case where
    a numeric group ID has two names (although I have not heard of this
    being done in practice).
    
    Solve this by reference-counting the data structure. There are up to
    three references in practice: one held temporarily while the lookup
    function is populating and storing it, one held by the hash table that
    is indexed by uid, and one held by the hash table that is indexed by
    name.
    
    Closes: dbus#305
    Signed-off-by: Simon McVittie <smcv@collabora.com>

Index: dbus-1.12.2/dbus/dbus-sysdeps-unix.h
===================================================================
--- dbus-1.12.2.orig/dbus/dbus-sysdeps-unix.h
+++ dbus-1.12.2/dbus/dbus-sysdeps-unix.h
@@ -105,6 +105,7 @@ typedef struct DBusGroupInfo DBusGroupIn
  */
 struct DBusUserInfo
 {
+  size_t      refcount;       /**< Reference count */
   dbus_uid_t  uid;            /**< UID */
   dbus_gid_t  primary_gid;    /**< GID */
   dbus_gid_t *group_ids;      /**< Groups IDs, *including* above primary group */
@@ -118,6 +119,7 @@ struct DBusUserInfo
  */
 struct DBusGroupInfo
 {
+  size_t      refcount;       /**< Reference count */
   dbus_gid_t  gid;            /**< GID */
   char       *groupname;      /**< Group name */
 };
Index: dbus-1.12.2/dbus/dbus-userdb-util.c
===================================================================
--- dbus-1.12.2.orig/dbus/dbus-userdb-util.c
+++ dbus-1.12.2/dbus/dbus-userdb-util.c
@@ -38,6 +38,15 @@
  * @{
  */
 
+static DBusGroupInfo *
+_dbus_group_info_ref (DBusGroupInfo *info)
+{
+  _dbus_assert (info->refcount > 0);
+  _dbus_assert (info->refcount < SIZE_MAX);
+  info->refcount++;
+  return info;
+}
+
 /**
  * Checks to see if the UID sent in is the console user
  *
@@ -287,13 +296,14 @@ _dbus_user_database_lookup_group (DBusUs
           dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
           return NULL;
         }
+      info->refcount = 1;
 
       if (gid != DBUS_GID_UNSET)
         {
           if (!_dbus_group_info_fill_gid (info, gid, error))
             {
               _DBUS_ASSERT_ERROR_IS_SET (error);
-              _dbus_group_info_free_allocated (info);
+              _dbus_group_info_unref (info);
               return NULL;
             }
         }
@@ -302,7 +312,7 @@ _dbus_user_database_lookup_group (DBusUs
           if (!_dbus_group_info_fill (info, groupname, error))
             {
               _DBUS_ASSERT_ERROR_IS_SET (error);
-              _dbus_group_info_free_allocated (info);
+              _dbus_group_info_unref (info);
               return NULL;
             }
         }
@@ -311,23 +321,35 @@ _dbus_user_database_lookup_group (DBusUs
       gid = DBUS_GID_UNSET;
       groupname = NULL;
 
-      if (!_dbus_hash_table_insert_uintptr (db->groups, info->gid, info))
+      if (_dbus_hash_table_insert_uintptr (db->groups, info->gid, info))
+        {
+          _dbus_group_info_ref (info);
+        }
+      else
         {
           dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
-          _dbus_group_info_free_allocated (info);
+          _dbus_group_info_unref (info);
           return NULL;
         }
 
 
-      if (!_dbus_hash_table_insert_string (db->groups_by_name,
-                                           info->groupname,
-                                           info))
+      if (_dbus_hash_table_insert_string (db->groups_by_name,
+                                          info->groupname,
+                                          info))
+        {
+          _dbus_group_info_ref (info);
+        }
+      else
         {
           _dbus_hash_table_remove_uintptr (db->groups, info->gid);
+          _dbus_group_info_unref (info);
           dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
           return NULL;
         }
-      
+
+      /* Release the original reference */
+      _dbus_group_info_unref (info);
+
       /* Return a borrowed reference to the DBusGroupInfo owned by the
        * two hash tables */
       return info;
Index: dbus-1.12.2/dbus/dbus-userdb.c
===================================================================
--- dbus-1.12.2.orig/dbus/dbus-userdb.c
+++ dbus-1.12.2/dbus/dbus-userdb.c
@@ -35,34 +35,57 @@
  * @{
  */
 
+static DBusUserInfo *
+_dbus_user_info_ref (DBusUserInfo *info)
+{
+  _dbus_assert (info->refcount > 0);
+  _dbus_assert (info->refcount < SIZE_MAX);
+  info->refcount++;
+  return info;
+}
+
 /**
- * Frees the given #DBusUserInfo's members with _dbus_user_info_free()
+ * Decrements the reference count. If it reaches 0,
+ * frees the given #DBusUserInfo's members with _dbus_user_info_free()
  * and also calls dbus_free() on the block itself
  *
  * @param info the info
  */
 void
-_dbus_user_info_free_allocated (DBusUserInfo *info)
+_dbus_user_info_unref (DBusUserInfo *info)
 {
   if (info == NULL) /* hash table will pass NULL */
     return;
 
+  _dbus_assert (info->refcount > 0);
+  _dbus_assert (info->refcount < SIZE_MAX);
+
+  if (--info->refcount > 0)
+    return;
+
   _dbus_user_info_free (info);
   dbus_free (info);
 }
 
 /**
- * Frees the given #DBusGroupInfo's members with _dbus_group_info_free()
+ * Decrements the reference count. If it reaches 0,
+ * frees the given #DBusGroupInfo's members with _dbus_group_info_free()
  * and also calls dbus_free() on the block itself
  *
  * @param info the info
  */
 void
-_dbus_group_info_free_allocated (DBusGroupInfo *info)
+_dbus_group_info_unref (DBusGroupInfo *info)
 {
   if (info == NULL) /* hash table will pass NULL */
     return;
 
+  _dbus_assert (info->refcount > 0);
+  _dbus_assert (info->refcount < SIZE_MAX);
+
+  if (--info->refcount > 0)
+    return;
+
   _dbus_group_info_free (info);
   dbus_free (info);
 }
@@ -170,13 +193,14 @@ _dbus_user_database_lookup (DBusUserData
           dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
           return NULL;
         }
+      info->refcount = 1;
 
       if (uid != DBUS_UID_UNSET)
         {
           if (!_dbus_user_info_fill_uid (info, uid, error))
             {
               _DBUS_ASSERT_ERROR_IS_SET (error);
-              _dbus_user_info_free_allocated (info);
+              _dbus_user_info_unref (info);
               return NULL;
             }
         }
@@ -185,7 +209,7 @@ _dbus_user_database_lookup (DBusUserData
           if (!_dbus_user_info_fill (info, username, error))
             {
               _DBUS_ASSERT_ERROR_IS_SET (error);
-              _dbus_user_info_free_allocated (info);
+              _dbus_user_info_unref (info);
               return NULL;
             }
         }
@@ -195,22 +219,33 @@ _dbus_user_database_lookup (DBusUserData
       username = NULL;
 
       /* insert into hash */
-      if (!_dbus_hash_table_insert_uintptr (db->users, info->uid, info))
+      if (_dbus_hash_table_insert_uintptr (db->users, info->uid, info))
+        {
+          _dbus_user_info_ref (info);
+        }
+      else
         {
           dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
-          _dbus_user_info_free_allocated (info);
+          _dbus_user_info_unref (info);
           return NULL;
         }
 
-      if (!_dbus_hash_table_insert_string (db->users_by_name,
-                                           info->username,
-                                           info))
+      if (_dbus_hash_table_insert_string (db->users_by_name,
+                                          info->username,
+                                          info))
+        {
+          _dbus_user_info_ref (info);
+        }
+      else
         {
           _dbus_hash_table_remove_uintptr (db->users, info->uid);
           dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+          _dbus_user_info_unref (info);
           return NULL;
         }
-      
+
+      _dbus_user_info_unref (info);
+
       /* Return a borrowed pointer to the DBusUserInfo owned by the
        * hash tables */
       return info;
@@ -570,24 +605,24 @@ _dbus_user_database_new (void)
   db->refcount = 1;
 
   db->users = _dbus_hash_table_new (DBUS_HASH_UINTPTR,
-                                    NULL, (DBusFreeFunction) _dbus_user_info_free_allocated);
+                                    NULL, (DBusFreeFunction) _dbus_user_info_unref);
   
   if (db->users == NULL)
     goto failed;
 
   db->groups = _dbus_hash_table_new (DBUS_HASH_UINTPTR,
-                                     NULL, (DBusFreeFunction) _dbus_group_info_free_allocated);
+                                     NULL, (DBusFreeFunction) _dbus_group_info_unref);
   
   if (db->groups == NULL)
     goto failed;
 
   db->users_by_name = _dbus_hash_table_new (DBUS_HASH_STRING,
-                                            NULL, NULL);
+                                            NULL, (DBusFreeFunction) _dbus_user_info_unref);
   if (db->users_by_name == NULL)
     goto failed;
   
   db->groups_by_name = _dbus_hash_table_new (DBUS_HASH_STRING,
-                                             NULL, NULL);
+                                             NULL, (DBusFreeFunction) _dbus_group_info_unref);
   if (db->groups_by_name == NULL)
     goto failed;
   
Index: dbus-1.12.2/dbus/dbus-userdb.h
===================================================================
--- dbus-1.12.2.orig/dbus/dbus-userdb.h
+++ dbus-1.12.2/dbus/dbus-userdb.h
@@ -85,10 +85,10 @@ const DBusGroupInfo* _dbus_user_database
                                                        dbus_gid_t        gid,
                                                        const DBusString *groupname,
                                                        DBusError        *error);
+
+void           _dbus_user_info_unref            (DBusUserInfo     *info);
 DBUS_PRIVATE_EXPORT
-void           _dbus_user_info_free_allocated   (DBusUserInfo     *info);
-DBUS_PRIVATE_EXPORT
-void           _dbus_group_info_free_allocated  (DBusGroupInfo    *info);
+void           _dbus_group_info_unref           (DBusGroupInfo    *info);
 #endif /* DBUS_USERDB_INCLUDES_PRIVATE */
 
 DBUS_PRIVATE_EXPORT

Places

File fix-upstream-CVE-2020-35512.patch of Package dbus-1-x11 (Revision 0985657dab80d85437d4085ba677756e)

Places