commit dc94fe3d31adf72259adc31f343537151a6c0bdd
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>
    (cherry picked from commit 2b7948ef907669e844b52c4fa2268d6e3162a70c)

Index: dbus-1.2.10/dbus/dbus-sysdeps-unix.h
===================================================================
--- dbus-1.2.10.orig/dbus/dbus-sysdeps-unix.h
+++ dbus-1.2.10/dbus/dbus-sysdeps-unix.h
@@ -87,6 +87,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 */
@@ -100,6 +101,7 @@ struct DBusUserInfo
  */
 struct DBusGroupInfo
 {
+  size_t      refcount;       /**< Reference count */
   dbus_gid_t  gid;            /**< GID */
   char       *groupname;      /**< Group name */
 };
Index: dbus-1.2.10/dbus/dbus-userdb.c
===================================================================
--- dbus-1.2.10.orig/dbus/dbus-userdb.c
+++ dbus-1.2.10/dbus/dbus-userdb.c
@@ -1,10 +1,10 @@
 /* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */
 /* dbus-userdb.c User database abstraction
- * 
+ *
  * Copyright (C) 2003, 2004  Red Hat, Inc.
  *
  * Licensed under the Academic Free License version 2.1
- * 
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -14,7 +14,7 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- * 
+ *
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
@@ -34,34 +34,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);
 }
@@ -91,12 +114,12 @@ _dbus_group_info_free (DBusGroupInfo
 }
 
 /**
- * Checks if a given string is actually a number 
- * and converts it if it is 
+ * Checks if a given string is actually a number
+ * and converts it if it is
  *
  * @param str the string to check
  * @param num the memory location of the unsigned long to fill in
- * @returns TRUE if str is a number and num is filled in 
+ * @returns TRUE if str is a number and num is filled in
  */
 dbus_bool_t
 _dbus_is_a_number (const DBusString *str,
@@ -119,7 +142,7 @@ _dbus_is_a_number (const DBusString *str
  *
  * @param db the database
  * @param uid the user ID or #DBUS_UID_UNSET
- * @param username username or #NULL 
+ * @param username username or #NULL
  * @param error error to fill in
  * @returns the entry in the database (borrowed, do not free)
  */
@@ -143,7 +166,7 @@ _dbus_user_database_lookup (DBusUserData
         uid = n;
     }
 
-#ifdef DBUS_ENABLE_USERDB_CACHE  
+#ifdef DBUS_ENABLE_USERDB_CACHE
   if (uid != DBUS_UID_UNSET)
     info = _dbus_hash_table_lookup_ulong (db->users, uid);
   else
@@ -156,7 +179,7 @@ _dbus_user_database_lookup (DBusUserData
       return info;
     }
   else
-#else 
+#else
   if (1)
 #endif
     {
@@ -166,20 +189,21 @@ _dbus_user_database_lookup (DBusUserData
       else
 	_dbus_verbose ("No cache for user \"%s\"\n",
 		       _dbus_string_get_const_data (username));
-      
+
       info = dbus_new0 (DBusUserInfo, 1);
       if (info == NULL)
         {
           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;
             }
         }
@@ -188,7 +212,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;
             }
         }
@@ -198,22 +222,31 @@ _dbus_user_database_lookup (DBusUserData
       username = NULL;
 
       /* insert into hash */
-      if (!_dbus_hash_table_insert_ulong (db->users, info->uid, info))
+      if (_dbus_hash_table_insert_ulong (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,
+      if (_dbus_hash_table_insert_string (db->users_by_name,
                                            info->username,
                                            info))
         {
+          _dbus_user_info_ref (info);
+        }
+      else
+        {
           _dbus_hash_table_remove_ulong (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;
@@ -224,7 +257,7 @@ static dbus_bool_t database_locked = FAL
 static DBusUserDatabase *system_db = NULL;
 static DBusString process_username;
 static DBusString process_homedir;
-      
+
 static void
 shutdown_system_db (void *data)
 {
@@ -238,12 +271,12 @@ static dbus_bool_t
 init_system_db (void)
 {
   _dbus_assert (database_locked);
-    
+
   if (system_db == NULL)
     {
       DBusError error = DBUS_ERROR_INIT;
       const DBusUserInfo *info;
-      
+
       system_db = _dbus_user_database_new ();
       if (system_db == NULL)
         return FALSE;
@@ -255,7 +288,7 @@ init_system_db (void)
         {
           _dbus_user_database_unref (system_db);
           system_db = NULL;
-          
+
           if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
             {
               dbus_error_free (&error);
@@ -335,7 +368,7 @@ _dbus_user_database_get_system (void)
   _dbus_assert (database_locked);
 
   init_system_db ();
-  
+
   return system_db;
 }
 
@@ -346,7 +379,7 @@ void
 _dbus_user_database_flush_system (void)
 {
   _dbus_user_database_lock_system ();
-   
+
   _dbus_user_database_flush (system_db);
 
   _dbus_user_database_unlock_system ();
@@ -369,7 +402,7 @@ _dbus_username_from_current_process (con
       return FALSE;
     }
   *username = &process_username;
-  _dbus_user_database_unlock_system ();  
+  _dbus_user_database_unlock_system ();
 
   return TRUE;
 }
@@ -430,7 +463,7 @@ _dbus_homedir_from_username (const DBusS
       _dbus_user_database_unlock_system ();
       return FALSE;
     }
-  
+
   _dbus_user_database_unlock_system ();
   return TRUE;
 }
@@ -469,7 +502,7 @@ _dbus_homedir_from_uid (dbus_uid_t
       _dbus_user_database_unlock_system ();
       return FALSE;
     }
-  
+
   _dbus_user_database_unlock_system ();
   return TRUE;
 }
@@ -480,11 +513,11 @@ _dbus_homedir_from_uid (dbus_uid_t
  * Used among other purposes to parses a desired identity provided
  * from a client in the auth protocol. On UNIX this means parsing a
  * UID, on Windows probably parsing an SID string.
- * 
+ *
  * @todo this is broken because it treats OOM and parse error
  * the same way. Needs a #DBusError.
- * 
- * @param credentials credentials to fill in 
+ *
+ * @param credentials credentials to fill in
  * @param username the username
  * @returns #TRUE if the username existed and we got some credentials
  */
@@ -516,7 +549,7 @@ _dbus_credentials_add_from_user (DBusCre
       _dbus_user_database_unlock_system ();
       return FALSE;
     }
-  
+
   _dbus_user_database_unlock_system ();
   return TRUE;
 }
@@ -530,7 +563,7 @@ DBusUserDatabase*
 _dbus_user_database_new (void)
 {
   DBusUserDatabase *db;
-  
+
   db = dbus_new0 (DBusUserDatabase, 1);
   if (db == NULL)
     return NULL;
@@ -538,39 +571,39 @@ _dbus_user_database_new (void)
   db->refcount = 1;
 
   db->users = _dbus_hash_table_new (DBUS_HASH_ULONG,
-                                    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_ULONG,
-                                     NULL, (DBusFreeFunction) _dbus_group_info_free_allocated);
-  
+                                     NULL, (DBusFreeFunction) _dbus_user_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_user_info_unref);
   if (db->groups_by_name == NULL)
     goto failed;
-  
+
   return db;
-  
+
  failed:
   _dbus_user_database_unref (db);
   return NULL;
 }
 
 /**
- * Flush all information out of the user database. 
+ * Flush all information out of the user database.
  */
 void
-_dbus_user_database_flush (DBusUserDatabase *db) 
+_dbus_user_database_flush (DBusUserDatabase *db)
 {
   _dbus_hash_table_remove_all(db->users_by_name);
   _dbus_hash_table_remove_all(db->groups_by_name);
@@ -618,14 +651,14 @@ _dbus_user_database_unref (DBusUserDatab
 
       if (db->groups_by_name)
         _dbus_hash_table_unref (db->groups_by_name);
-      
+
       dbus_free (db);
     }
 }
 
 /**
  * Gets the user information for the given UID,
- * returned user info should not be freed. 
+ * returned user info should not be freed.
  *
  * @param db user database
  * @param uid the user ID
Index: dbus-1.2.10/dbus/dbus-userdb.h
===================================================================
--- dbus-1.2.10.orig/dbus/dbus-userdb.h
+++ dbus-1.2.10/dbus/dbus-userdb.h
@@ -81,8 +81,8 @@ const DBusGroupInfo* _dbus_user_database
                                                  dbus_gid_t        gid,
                                                  const DBusString *groupname,
                                                  DBusError        *error);
-void           _dbus_user_info_free_allocated   (DBusUserInfo     *info);
-void           _dbus_group_info_free_allocated  (DBusGroupInfo    *info);
+void           _dbus_user_info_unref            (DBusUserInfo     *info);
+void           _dbus_group_info_unref           (DBusGroupInfo    *info);
 #endif /* DBUS_USERDB_INCLUDES_PRIVATE */
 
 DBusUserDatabase* _dbus_user_database_get_system    (void);
