diff options
| -rw-r--r-- | NEWS | 60 | ||||
| -rw-r--r-- | bus/connection.c | 7 | ||||
| -rw-r--r-- | configure.ac | 4 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps-unix.c | 53 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps-unix.h | 2 | ||||
| -rw-r--r-- | dbus/dbus-userdb-util.c | 44 | ||||
| -rw-r--r-- | dbus/dbus-userdb.c | 73 | ||||
| -rw-r--r-- | dbus/dbus-userdb.h | 16 | ||||
| -rw-r--r-- | test/fdpass.c | 14 |
9 files changed, 219 insertions, 54 deletions
@@ -1,3 +1,63 @@ +dbus 1.10.x end-of-life +== + +The dbus 1.10.x branch was originally released in 2015 and reached +end-of-life status in July 2020. + +If new security issues are discovered in dbus, they will not be fixed +in the 1.10.x branch. + +If you are a dbus downstream maintainer in a long-lived OS distribution +and you want to use the upstream dbus-1.10 git branch as a place +to share backported security fixes with other distributions, please +contact the dbus maintainers via the dbus-security mailing list on +lists.freedesktop.org. + +dbus 1.10.32 (2020-07-02) +== + +The “technically a venom” release. + +Maybe security fixes: + +• On Unix, avoid a use-after-free if two usernames have the same + numeric uid. In older versions this could lead to a crash (denial of + service) or other undefined behaviour, possibly including incorrect + authorization decisions if <policy group=...> is used. + Like Unix filesystems, D-Bus' model of identity cannot distinguish + between users of different names with the same numeric uid, so this + configuration is not advisable on systems where D-Bus will be used. + Thanks to Daniel Onaca. + (dbus#305, dbus!166, CVE-2020-35512; Simon McVittie) + +Other fixes: + +• On Solaris and its derivatives, if a cmsg header is truncated, ensure + that we do not overrun the buffer used for fd-passing, even if the + kernel tells us to. + (dbus#304, dbus!165; Andy Fiddaman) + +dbus 1.10.30 (2020-06-02) +== + +The “centaur bus” release. + +Denial of service fixes: + +• CVE-2020-12049: If a message contains more file descriptors than can + be sent, close those that did get through before reporting error. + Previously, a local attacker could cause the system dbus-daemon (or + another system service with its own DBusServer) to run out of file + descriptors, by repeatedly connecting to the server and sending fds that + would get leaked. + Thanks to Kevin Backhouse of GitHub Security Lab. + (dbus#294, GHSL-2020-057; Simon McVittie) + +Other fixes: + +• Fix a crash when the dbus-daemon is terminated while one or more + monitors are active (dbus#291, dbus!140; Simon McVittie) + dbus 1.10.28 (2019-06-11) == diff --git a/bus/connection.c b/bus/connection.c index 31ed6be7..05daa6a4 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -540,9 +540,6 @@ bus_connections_unref (BusConnections *connections) _dbus_assert (connections->n_incomplete == 0); - /* drop all monitors */ - _dbus_list_clear (&connections->monitors); - /* drop all real connections */ while (connections->completed != NULL) { @@ -558,6 +555,10 @@ bus_connections_unref (BusConnections *connections) _dbus_assert (connections->n_completed == 0); + /* disconnecting all the connections should have emptied the list of + * monitors (each link is removed in bus_connection_disconnected) */ + _dbus_assert (connections->monitors == NULL); + bus_expire_list_free (connections->pending_replies); _dbus_loop_remove_timeout (bus_context_get_loop (connections->context), diff --git a/configure.ac b/configure.ac index 04ebae5f..30d7f115 100644 --- a/configure.ac +++ b/configure.ac @@ -3,7 +3,7 @@ AC_PREREQ([2.63]) m4_define([dbus_major_version], [1]) m4_define([dbus_minor_version], [10]) -m4_define([dbus_micro_version], [28]) +m4_define([dbus_micro_version], [32]) m4_define([dbus_version], [dbus_major_version.dbus_minor_version.dbus_micro_version]) AC_INIT([dbus],[dbus_version],[https://bugs.freedesktop.org/enter_bug.cgi?product=dbus],[dbus]) @@ -38,7 +38,7 @@ LT_CURRENT=17 ## increment any time the source changes; set to ## 0 if you increment CURRENT -LT_REVISION=16 +LT_REVISION=18 ## increment if any interfaces have been added; set to 0 ## if any interfaces have been changed or removed. removal has diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index b7309712..4989e4a9 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -432,25 +432,13 @@ _dbus_read_socket_with_unix_fds (DBusSocket fd, struct cmsghdr *cm; dbus_bool_t found = FALSE; - if (m.msg_flags & MSG_CTRUNC) - { - /* Hmm, apparently the control data was truncated. The bad - thing is that we might have completely lost a couple of fds - without chance to recover them. Hence let's treat this as a - serious error. */ - - errno = ENOSPC; - _dbus_string_set_length (buffer, start); - return -1; - } - for (cm = CMSG_FIRSTHDR(&m); cm; cm = CMSG_NXTHDR(&m, cm)) if (cm->cmsg_level == SOL_SOCKET && cm->cmsg_type == SCM_RIGHTS) { size_t i; int *payload = (int *) CMSG_DATA (cm); size_t payload_len_bytes = (cm->cmsg_len - CMSG_LEN (0)); - size_t payload_len_fds = payload_len_bytes / sizeof (int); + size_t payload_len_fds; size_t fds_to_use; /* Every non-negative int fits in a size_t without truncation, @@ -458,6 +446,25 @@ _dbus_read_socket_with_unix_fds (DBusSocket fd, * casting (size_t) *n_fds is OK */ _DBUS_STATIC_ASSERT (sizeof (size_t) >= sizeof (int)); + if ((m.msg_flags & MSG_CTRUNC) && CMSG_NXTHDR(&m, cm) == NULL && + (char *) payload + payload_len_bytes > + (char *) m.msg_control + m.msg_controllen) + { + /* This is the last cmsg in a truncated message and using + * cmsg_len would apparently overrun the allocated buffer. + * Some operating systems (illumos and Solaris are known) do + * not adjust cmsg_len in the last cmsg when truncation occurs. + * Adjust the payload length here. The calculation for + * payload_len_fds below will discard any trailing bytes that + * belong to an incomplete file descriptor - the kernel will + * have already closed that (at least for illumos and Solaris) + */ + payload_len_bytes = m.msg_controllen - + ((char *) payload - (char *) m.msg_control); + } + + payload_len_fds = payload_len_bytes / sizeof (int); + if (_DBUS_LIKELY (payload_len_fds <= (size_t) *n_fds)) { /* The fds in the payload will fit in our buffer */ @@ -498,6 +505,26 @@ _dbus_read_socket_with_unix_fds (DBusSocket fd, if (!found) *n_fds = 0; + if (m.msg_flags & MSG_CTRUNC) + { + int i; + + /* Hmm, apparently the control data was truncated. The bad + thing is that we might have completely lost a couple of fds + without chance to recover them. Hence let's treat this as a + serious error. */ + + /* We still need to close whatever fds we *did* receive, + * otherwise they'll never get closed. (CVE-2020-12049) */ + for (i = 0; i < *n_fds; i++) + close (fds[i]); + + *n_fds = 0; + errno = ENOSPC; + _dbus_string_set_length (buffer, start); + return -1; + } + /* put length back (doesn't actually realloc) */ _dbus_string_set_length (buffer, start + bytes_read); diff --git a/dbus/dbus-sysdeps-unix.h b/dbus/dbus-sysdeps-unix.h index 279cae27..91b4c606 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 888a23e9..5fba282d 100644 --- a/dbus/dbus-userdb-util.c +++ b/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 * @@ -240,9 +249,9 @@ _dbus_get_user_id_and_primary_group (const DBusString *username, * @param gid the group ID or #DBUS_GID_UNSET * @param groupname group name or #NULL * @param error error to fill in - * @returns the entry in the database + * @returns the entry in the database (borrowed, do not free) */ -DBusGroupInfo* +const DBusGroupInfo * _dbus_user_database_lookup_group (DBusUserDatabase *db, dbus_gid_t gid, const DBusString *groupname, @@ -287,13 +296,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; } } @@ -302,7 +312,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; } } @@ -311,23 +321,37 @@ _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 52f927a3..19fc32e4 100644 --- a/dbus/dbus-userdb.c +++ b/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); } @@ -122,9 +145,9 @@ _dbus_is_a_number (const DBusString *str, * @param uid the user ID or #DBUS_UID_UNSET * @param username username or #NULL * @param error error to fill in - * @returns the entry in the database + * @returns the entry in the database (borrowed, do not free) */ -DBusUserInfo* +const DBusUserInfo * _dbus_user_database_lookup (DBusUserDatabase *db, dbus_uid_t uid, const DBusString *username, @@ -170,13 +193,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; } } @@ -185,7 +209,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; } } @@ -195,22 +219,35 @@ _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; } } @@ -558,24 +595,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 53fc90b5..b38e3d18 100644 --- a/dbus/dbus-userdb.h +++ b/dbus/dbus-userdb.h @@ -76,19 +76,19 @@ dbus_bool_t _dbus_user_database_get_groupname (DBusUserDatabase *db, DBusError *error); DBUS_PRIVATE_EXPORT -DBusUserInfo* _dbus_user_database_lookup (DBusUserDatabase *db, +const DBusUserInfo *_dbus_user_database_lookup (DBusUserDatabase *db, dbus_uid_t uid, const DBusString *username, DBusError *error); DBUS_PRIVATE_EXPORT -DBusGroupInfo* _dbus_user_database_lookup_group (DBusUserDatabase *db, - dbus_gid_t gid, - const DBusString *groupname, - DBusError *error); -DBUS_PRIVATE_EXPORT -void _dbus_user_info_free_allocated (DBusUserInfo *info); +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_group_info_free_allocated (DBusGroupInfo *info); +void _dbus_group_info_unref (DBusGroupInfo *info); #endif /* DBUS_USERDB_INCLUDES_PRIVATE */ DBUS_PRIVATE_EXPORT diff --git a/test/fdpass.c b/test/fdpass.c index 665b4a12..d8d9c670 100644 --- a/test/fdpass.c +++ b/test/fdpass.c @@ -50,6 +50,14 @@ #include "test-utils-glib.h" +#ifdef DBUS_ENABLE_EMBEDDED_TESTS +#include <dbus/dbus-message-internal.h> +#else +typedef struct _DBusInitialFDs DBusInitialFDs; +#define _dbus_check_fdleaks_enter() NULL +#define _dbus_check_fdleaks_leave(fds) do {} while (0) +#endif + /* Arbitrary; included here to avoid relying on the default */ #define MAX_MESSAGE_UNIX_FDS 20 /* This test won't work on Linux unless this is true. */ @@ -91,6 +99,7 @@ typedef struct { GQueue messages; int fd_before; + DBusInitialFDs *initial_fds; } Fixture; static void oom (const gchar *doing) G_GNUC_NORETURN; @@ -172,6 +181,8 @@ test_connect (Fixture *f, { char *address; + f->initial_fds = _dbus_check_fdleaks_enter (); + g_assert (f->left_server_conn == NULL); g_assert (f->right_server_conn == NULL); @@ -835,6 +846,9 @@ teardown (Fixture *f, if (f->fd_before >= 0 && close (f->fd_before) < 0) g_error ("%s", g_strerror (errno)); #endif + + if (f->initial_fds != NULL) + _dbus_check_fdleaks_leave (f->initial_fds); } int |