Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE
dbus-1-x11.16857
fix-upstream-CVE-2020-35512.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File fix-upstream-CVE-2020-35512.patch of Package dbus-1-x11.16857
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> diff --git a/dbus/dbus-sysdeps-unix.h b/dbus/dbus-sysdeps-unix.h index 3bbf34d1..e86de6d9 100644 --- a/dbus/dbus-sysdeps-unix.h +++ b/dbus/dbus-sysdeps-unix.h @@ -105,6 +105,7 @@ typedef struct DBusGroupInfo DBusGroupInfo; */ 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 */ }; diff --git a/dbus/dbus-userdb-util.c b/dbus/dbus-userdb-util.c index 18621a17..1ca21eb7 100644 --- a/dbus/dbus-userdb-util.c +++ b/dbus/dbus-userdb-util.c @@ -45,6 +45,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 * @@ -296,13 +305,14 @@ _dbus_user_database_lookup_group (DBusUserDatabase *db, 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; } } @@ -311,7 +321,7 @@ _dbus_user_database_lookup_group (DBusUserDatabase *db, 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; } } @@ -320,23 +330,35 @@ _dbus_user_database_lookup_group (DBusUserDatabase *db, 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; diff --git a/dbus/dbus-userdb.c b/dbus/dbus-userdb.c index 78169e5b..ddf41a3c 100644 --- a/dbus/dbus-userdb.c +++ b/dbus/dbus-userdb.c @@ -42,34 +42,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); } @@ -177,13 +200,14 @@ _dbus_user_database_lookup (DBusUserDatabase *db, 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; } } @@ -192,7 +216,7 @@ _dbus_user_database_lookup (DBusUserDatabase *db, 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; } } @@ -202,22 +226,33 @@ _dbus_user_database_lookup (DBusUserDatabase *db, 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; @@ -569,24 +604,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; diff --git a/dbus/dbus-userdb.h b/dbus/dbus-userdb.h index 479b2311..fcb515c8 100644 --- a/dbus/dbus-userdb.h +++ b/dbus/dbus-userdb.h @@ -76,10 +76,10 @@ const DBusGroupInfo* _dbus_user_database_lookup_group (DBusUserDatabase *db, 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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
