Compare commits

...

10 Commits

Author SHA1 Message Date
openeuler-ci-bot
2023052193
!88 同步上游社区补丁
From: @hongjinghao 
Reviewed-by: @dillon_chen 
Signed-off-by: @dillon_chen
2024-06-18 09:13:14 +00:00
hongjinghao
fb37e8c245 sync patches from dbus community 2024-06-18 14:14:30 +08:00
openeuler-ci-bot
f0f19b210e
!87 fix CVE-2023-34969
From: @hongjinghao 
Reviewed-by: @dillon_chen 
Signed-off-by: @dillon_chen
2024-06-18 02:51:00 +00:00
hongjinghao
15d47613ca fix CVE-2023-34969 2024-06-18 10:15:07 +08:00
openeuler-ci-bot
591a09b3db
!83 当XDG_DATA_DIRS设置大于128个目录时,避免dbus-daemon crash
From: @hongjinghao 
Reviewed-by: @licunlong 
Signed-off-by: @licunlong
2024-02-21 09:25:49 +00:00
hongjinghao
ccf9658000 Do not crash with > 128 dirs 2024-02-21 16:32:58 +08:00
openeuler-ci-bot
dddf1b004f
!55 fix CVE-2022-42010,CVE-2022-42011,CVE-2022-42012
From: @hongjinghao 
Reviewed-by: @licunlong 
Signed-off-by: @licunlong
2022-10-17 08:50:35 +00:00
hongjinghao
10bea11beb fix CVE-2022-42010,CVE-2022-42011,CVE-2022-42012 2022-10-17 16:25:44 +08:00
openeuler-ci-bot
119eb59641
!51 [sync] PR-50: Stop using selinux set_mapping function
From: @openeuler-sync-bot 
Reviewed-by: @licunlong 
Signed-off-by: @licunlong
2022-09-20 13:02:12 +00:00
hongjinghao
c18fc45b7e Stop using selinux set_mapping function
(cherry picked from commit c8b5ed1b2764bf6f273f42b4d05fca204853f123)
2022-09-20 15:43:47 +08:00
14 changed files with 1482 additions and 1 deletions

View File

@ -0,0 +1,114 @@
From 9d07424e9011e3bbe535e83043d335f3093d2916 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 13 Sep 2022 15:10:22 +0100
Subject: [PATCH] dbus-marshal-validate: Check brackets in signature nest
correctly
In debug builds with assertions enabled, a signature with incorrectly
nested `()` and `{}`, for example `a{i(u}` or `(a{ii)}`, could result
in an assertion failure.
In production builds without assertions enabled, a signature with
incorrectly nested `()` and `{}` could potentially result in a crash
or incorrect message parsing, although we do not have a concrete example
of either of these failure modes.
Thanks: Evgeny Vereshchagin
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/418
Resolves: CVE-2022-42010
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
dbus/dbus-marshal-validate.c | 38 +++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c
index 4d492f3f..ae68414d 100644
--- a/dbus/dbus-marshal-validate.c
+++ b/dbus/dbus-marshal-validate.c
@@ -62,6 +62,8 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
int element_count;
DBusList *element_count_stack;
+ char opened_brackets[DBUS_MAXIMUM_TYPE_RECURSION_DEPTH * 2 + 1] = { '\0' };
+ char last_bracket;
result = DBUS_VALID;
element_count_stack = NULL;
@@ -93,6 +95,10 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
while (p != end)
{
+ _dbus_assert (struct_depth + dict_entry_depth >= 0);
+ _dbus_assert (struct_depth + dict_entry_depth < _DBUS_N_ELEMENTS (opened_brackets));
+ _dbus_assert (opened_brackets[struct_depth + dict_entry_depth] == '\0');
+
switch (*p)
{
case DBUS_TYPE_BYTE:
@@ -136,6 +142,10 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
goto out;
}
+ _dbus_assert (struct_depth + dict_entry_depth >= 1);
+ _dbus_assert (struct_depth + dict_entry_depth < _DBUS_N_ELEMENTS (opened_brackets));
+ _dbus_assert (opened_brackets[struct_depth + dict_entry_depth - 1] == '\0');
+ opened_brackets[struct_depth + dict_entry_depth - 1] = DBUS_STRUCT_BEGIN_CHAR;
break;
case DBUS_STRUCT_END_CHAR:
@@ -151,9 +161,20 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
goto out;
}
+ _dbus_assert (struct_depth + dict_entry_depth >= 1);
+ _dbus_assert (struct_depth + dict_entry_depth < _DBUS_N_ELEMENTS (opened_brackets));
+ last_bracket = opened_brackets[struct_depth + dict_entry_depth - 1];
+
+ if (last_bracket != DBUS_STRUCT_BEGIN_CHAR)
+ {
+ result = DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED;
+ goto out;
+ }
+
_dbus_list_pop_last (&element_count_stack);
struct_depth -= 1;
+ opened_brackets[struct_depth + dict_entry_depth] = '\0';
break;
case DBUS_DICT_ENTRY_BEGIN_CHAR:
@@ -178,6 +199,10 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
goto out;
}
+ _dbus_assert (struct_depth + dict_entry_depth >= 1);
+ _dbus_assert (struct_depth + dict_entry_depth < _DBUS_N_ELEMENTS (opened_brackets));
+ _dbus_assert (opened_brackets[struct_depth + dict_entry_depth - 1] == '\0');
+ opened_brackets[struct_depth + dict_entry_depth - 1] = DBUS_DICT_ENTRY_BEGIN_CHAR;
break;
case DBUS_DICT_ENTRY_END_CHAR:
@@ -186,8 +211,19 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
result = DBUS_INVALID_DICT_ENTRY_ENDED_BUT_NOT_STARTED;
goto out;
}
-
+
+ _dbus_assert (struct_depth + dict_entry_depth >= 1);
+ _dbus_assert (struct_depth + dict_entry_depth < _DBUS_N_ELEMENTS (opened_brackets));
+ last_bracket = opened_brackets[struct_depth + dict_entry_depth - 1];
+
+ if (last_bracket != DBUS_DICT_ENTRY_BEGIN_CHAR)
+ {
+ result = DBUS_INVALID_DICT_ENTRY_ENDED_BUT_NOT_STARTED;
+ goto out;
+ }
+
dict_entry_depth -= 1;
+ opened_brackets[struct_depth + dict_entry_depth] = '\0';
element_count =
_DBUS_POINTER_TO_INT (_dbus_list_pop_last (&element_count_stack));
--
2.33.0

View File

@ -0,0 +1,55 @@
From 079bbf16186e87fb0157adf8951f19864bc2ed69 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 12 Sep 2022 13:14:18 +0100
Subject: [PATCH] dbus-marshal-validate: Validate length of arrays of
fixed-length items
This fast-path previously did not check that the array was made up
of an integer number of items. This could lead to assertion failures
and out-of-bounds accesses during subsequent message processing (which
assumes that the message has already been validated), particularly after
the addition of _dbus_header_remove_unknown_fields(), which makes it
more likely that dbus-daemon will apply non-trivial edits to messages.
Thanks: Evgeny Vereshchagin
Fixes: e61f13cf "Bug 18064 - more efficient validation for fixed-size type arrays"
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/413
Resolves: CVE-2022-42011
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
dbus/dbus-marshal-validate.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c
index ae68414d..7d0d6cf7 100644
--- a/dbus/dbus-marshal-validate.c
+++ b/dbus/dbus-marshal-validate.c
@@ -503,13 +503,24 @@ validate_body_helper (DBusTypeReader *reader,
*/
if (dbus_type_is_fixed (array_elem_type))
{
+ /* Note that fixed-size types all have sizes equal to
+ * their alignments, so this is really the item size. */
+ alignment = _dbus_type_get_alignment (array_elem_type);
+ _dbus_assert (alignment == 1 || alignment == 2 ||
+ alignment == 4 || alignment == 8);
+
+ /* Because the alignment is a power of 2, this is
+ * equivalent to: (claimed_len % alignment) != 0,
+ * but avoids slower integer division */
+ if ((claimed_len & (alignment - 1)) != 0)
+ return DBUS_INVALID_ARRAY_LENGTH_INCORRECT;
+
/* bools need to be handled differently, because they can
* have an invalid value
*/
if (array_elem_type == DBUS_TYPE_BOOLEAN)
{
dbus_uint32_t v;
- alignment = _dbus_type_get_alignment (array_elem_type);
while (p < array_end)
{
--
2.33.0

View File

@ -0,0 +1,71 @@
From 236f16e444e88a984cf12b09225e0f8efa6c5b44 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 30 Sep 2022 13:46:31 +0100
Subject: [PATCH] dbus-marshal-byteswap: Byte-swap Unix fd indexes if needed
When a D-Bus message includes attached file descriptors, the body of the
message contains unsigned 32-bit indexes pointing into an out-of-band
array of file descriptors. Some D-Bus APIs like GLib's GDBus refer to
these indexes as "handles" for the associated fds (not to be confused
with a Windows HANDLE, which is a kernel object).
The assertion message removed by this commit is arguably correct up to
a point: fd-passing is only reasonable on a local machine, and no known
operating system allows processes of differing endianness even on a
multi-endian ARM or PowerPC CPU, so it makes little sense for the sender
to specify a byte-order that differs from the byte-order of the recipient.
However, this doesn't account for the fact that a malicious sender
doesn't have to restrict itself to only doing things that make sense.
On a system with untrusted local users, a message sender could crash
the system dbus-daemon (a denial of service) by sending a message in
the opposite endianness that contains handles to file descriptors.
Before this commit, if assertions are enabled, attempting to byteswap
a fd index would cleanly crash the message recipient with an assertion
failure. If assertions are disabled, attempting to byteswap a fd index
would silently do nothing without advancing the pointer p, causing the
message's type and the pointer into its contents to go out of sync, which
can result in a subsequent crash (the crash demonstrated by fuzzing was
a use-after-free, but other failure modes might be possible).
In principle we could resolve this by rejecting wrong-endianness messages
from a local sender, but it's actually simpler and less code to treat
wrong-endianness messages as valid and byteswap them.
Thanks: Evgeny Vereshchagin
Fixes: ba7daa60 "unix-fd: add basic marshalling code for unix fds"
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/417
Resolves: CVE-2022-42012
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
dbus/dbus-marshal-byteswap.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/dbus/dbus-marshal-byteswap.c b/dbus/dbus-marshal-byteswap.c
index e9de6f02..9dd1246f 100644
--- a/dbus/dbus-marshal-byteswap.c
+++ b/dbus/dbus-marshal-byteswap.c
@@ -62,6 +62,7 @@ byteswap_body_helper (DBusTypeReader *reader,
case DBUS_TYPE_BOOLEAN:
case DBUS_TYPE_INT32:
case DBUS_TYPE_UINT32:
+ case DBUS_TYPE_UNIX_FD:
{
p = _DBUS_ALIGN_ADDRESS (p, 4);
*((dbus_uint32_t*)p) = DBUS_UINT32_SWAP_LE_BE (*((dbus_uint32_t*)p));
@@ -192,11 +193,6 @@ byteswap_body_helper (DBusTypeReader *reader,
}
break;
- case DBUS_TYPE_UNIX_FD:
- /* fds can only be passed on a local machine, so byte order must always match */
- _dbus_assert_not_reached("attempted to byteswap unix fds which makes no sense");
- break;
-
default:
_dbus_assert_not_reached ("invalid typecode in supposedly-validated signature");
break;
--
2.33.0

View File

@ -0,0 +1,96 @@
From 37a4dc5835731a1f7a81f1b67c45b8dfb556dd1c Mon Sep 17 00:00:00 2001
From: hongjinghao <q1204531485@163.com>
Date: Mon, 5 Jun 2023 18:17:06 +0100
Subject: [PATCH] bus: Assign a serial number for messages from the driver
Normally, it's enough to rely on a message being given a serial number
by the DBusConnection just before it is actually sent. However, in the
rare case where the policy blocks the driver from sending a message
(due to a deny rule or the outgoing message quota being full), we need
to get a valid serial number sooner, so that we can copy it into the
DBUS_HEADER_FIELD_REPLY_SERIAL field (which is mandatory) in the error
message sent to monitors. Otherwise, the dbus-daemon will crash with
an assertion failure if at least one Monitoring client is attached,
because zero is not a valid serial number to copy.
This fixes a denial-of-service vulnerability: if a privileged user is
monitoring the well-known system bus using a Monitoring client like
dbus-monitor or `busctl monitor`, then an unprivileged user can cause
denial-of-service by triggering this crash. A mitigation for this
vulnerability is to avoid attaching Monitoring clients to the system
bus when they are not needed. If there are no Monitoring clients, then
the vulnerable code is not reached.
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: dbus/dbus#457
(cherry picked from commit b159849e031000d1dbc1ab876b5fc78a3ce9b534)
---
bus/connection.c | 15 +++++++++++++++
dbus/dbus-connection-internal.h | 2 ++
dbus/dbus-connection.c | 11 ++++++++++-
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/bus/connection.c b/bus/connection.c
index b3583433..215f0230 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -2350,6 +2350,21 @@ bus_transaction_send_from_driver (BusTransaction *transaction,
if (!dbus_message_set_sender (message, DBUS_SERVICE_DBUS))
return FALSE;
+ /* Make sure the message has a non-zero serial number, otherwise
+ * bus_transaction_capture_error_reply() will not be able to mock up
+ * a corresponding reply for it. Normally this would be delayed until
+ * the first time we actually send the message out from a
+ * connection, when the transaction is committed, but that's too late
+ * in this case.
+ */
+ if (dbus_message_get_serial (message) == 0)
+ {
+ dbus_uint32_t next_serial;
+
+ next_serial = _dbus_connection_get_next_client_serial (connection);
+ dbus_message_set_serial (message, next_serial);
+ }
+
if (bus_connection_is_active (connection))
{
if (!dbus_message_set_destination (message,
diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h
index 48357321..ba79b192 100644
--- a/dbus/dbus-connection-internal.h
+++ b/dbus/dbus-connection-internal.h
@@ -54,6 +54,8 @@ DBUS_PRIVATE_EXPORT
DBusConnection * _dbus_connection_ref_unlocked (DBusConnection *connection);
DBUS_PRIVATE_EXPORT
void _dbus_connection_unref_unlocked (DBusConnection *connection);
+DBUS_PRIVATE_EXPORT
+dbus_uint32_t _dbus_connection_get_next_client_serial (DBusConnection *connection);
void _dbus_connection_queue_received_message_link (DBusConnection *connection,
DBusList *link);
dbus_bool_t _dbus_connection_has_messages_to_send_unlocked (DBusConnection *connection);
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index c525b6dc..09cef278 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -1456,7 +1456,16 @@ _dbus_connection_unref_unlocked (DBusConnection *connection)
_dbus_connection_last_unref (connection);
}
-static dbus_uint32_t
+/**
+ * Allocate and return the next non-zero serial number for outgoing messages.
+ *
+ * This method is only valid to call from single-threaded code, such as
+ * the dbus-daemon, or with the connection lock held.
+ *
+ * @param connection the connection
+ * @returns A suitable serial number for the next message to be sent on the connection.
+ */
+dbus_uint32_t
_dbus_connection_get_next_client_serial (DBusConnection *connection)
{
dbus_uint32_t serial;
--
2.27.0

View File

@ -0,0 +1,66 @@
From c3b1e4daa5b0ed5729f0f12bc6a3ba50a391f7f6 Mon Sep 17 00:00:00 2001
From: hongjinghao <hongjinghao@huawei.com>
Date: Thu, 4 Jan 2024 15:15:53 +0800
Subject: [PATCH] Do not crash when reloading configuration with > 128 dirs
When `dbus-daemon` sets more than 128 directories for `XDG_DATA_DIRS`,
none of the elements in `new_dirs` will be `NULL`, which resulted in
these loops reading out-of-bounds (undefined behaviour). In practice
this led to a crash.
To avoid this, make sure to stop iteration at the end of the array.
[smcv: Expanded commit message]
Resolves: dbus/dbus#481
---
bus/dir-watch-inotify.c | 4 ++--
bus/dir-watch-kqueue.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/bus/dir-watch-inotify.c b/bus/dir-watch-inotify.c
index 77b2d5a92..4f269777f 100644
--- a/bus/dir-watch-inotify.c
+++ b/bus/dir-watch-inotify.c
@@ -131,7 +131,7 @@ _set_watched_dirs_internal (BusContext *context,
/* Look for directories in both the old and new sets, if
* we find one, move its data into the new set.
*/
- for (i = 0; new_dirs[i]; i++)
+ for (i = 0; i < MAX_DIRS_TO_WATCH && new_dirs[i]; i++)
{
for (j = 0; j < num_wds; j++)
{
@@ -160,7 +160,7 @@ _set_watched_dirs_internal (BusContext *context,
}
}
- for (i = 0; new_dirs[i]; i++)
+ for (i = 0; i < MAX_DIRS_TO_WATCH && new_dirs[i]; i++)
{
if (new_wds[i] == -1)
{
diff --git a/bus/dir-watch-kqueue.c b/bus/dir-watch-kqueue.c
index b419606e3..07b505c99 100644
--- a/bus/dir-watch-kqueue.c
+++ b/bus/dir-watch-kqueue.c
@@ -235,7 +235,7 @@ bus_set_watched_dirs (BusContext *context, DBusList **directories)
/* Look for directories in both the old and new sets, if
* we find one, move its data into the new set.
*/
- for (i = 0; new_dirs[i]; i++)
+ for (i = 0; i < MAX_DIRS_TO_WATCH && new_dirs[i]; i++)
{
for (j = 0; j < num_fds; j++)
{
@@ -264,7 +264,7 @@ bus_set_watched_dirs (BusContext *context, DBusList **directories)
}
}
- for (i = 0; new_dirs[i]; i++)
+ for (i = 0; i < MAX_DIRS_TO_WATCH && new_dirs[i]; i++)
{
if (new_fds[i] == -1)
{
--
GitLab

View File

@ -0,0 +1,152 @@
From 6072f8b24153d844a3033108a17bcd0c1a967816 Mon Sep 17 00:00:00 2001
From: Laurent Bigonville <bigon@bigon.be>
Date: Sat, 3 Mar 2018 11:15:23 +0100
Subject: [PATCH] Stop using selinux_set_mapping() function
Currently, if the "dbus" security class or the associated AV doesn't
exist, dbus-daemon fails to initialize and exits immediately. Also the
security classes or access vector cannot be reordered in the policy.
This can be a problem for people developing their own policy or trying
to access a machine where, for some reasons, there is not policy defined
at all.
The code here copy the behaviour of the selinux_check_access() function.
We cannot use this function here as it doesn't allow us to define the
AVC entry reference.
See the discussion at https://marc.info/?l=selinux&m=152163374332372&w=2
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/198
Conflict:function bus_selinux_full_init (void) is modified to adapt to the context.
Reference:https://github.com/freedesktop/dbus/commit/6072f8b24153d844a3033108a17bcd0c1a967816
---
bus/selinux.c | 75 ++++++++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 33 deletions(-)
diff --git a/bus/selinux.c b/bus/selinux.c
index a005b84f..7e63348c 100644
--- a/bus/selinux.c
+++ b/bus/selinux.c
@@ -232,24 +232,6 @@ bus_selinux_pre_init (void)
#endif
}
-/*
- * Private Flask definitions; the order of these constants must
- * exactly match that of the structure array below!
- */
-/* security dbus class constants */
-#define SECCLASS_DBUS 1
-
-/* dbus's per access vector constants */
-#define DBUS__ACQUIRE_SVC 1
-#define DBUS__SEND_MSG 2
-
-#ifdef HAVE_SELINUX
-static struct security_class_mapping dbus_map[] = {
- { "dbus", { "acquire_svc", "send_msg", NULL } },
- { NULL }
-};
-#endif /* HAVE_SELINUX */
-
/**
* Establish dynamic object class and permission mapping and
* initialize the user space access vector cache (AVC) for D-Bus and set up
@@ -350,13 +350,6 @@ bus_selinux_full_init (void)
_dbus_verbose ("SELinux is enabled in this kernel.\n");
- if (selinux_set_mapping (dbus_map) < 0)
- {
- _dbus_warn ("Failed to set up security class mapping (selinux_set_mapping():%s).",
- strerror (errno));
- return FALSE;
- }
-
avc_entry_ref_init (&aeref);
if (avc_init ("avc", &mem_cb, &log_cb, &thread_cb, &lock_cb) < 0)
{
@@ -392,19 +367,53 @@ error:
static dbus_bool_t
bus_selinux_check (BusSELinuxID *sender_sid,
BusSELinuxID *override_sid,
- security_class_t target_class,
- access_vector_t requested,
- DBusString *auxdata)
+ const char *target_class,
+ const char *requested,
+ DBusString *auxdata)
{
+ int saved_errno;
+ security_class_t security_class;
+ access_vector_t requested_access;
+
if (!selinux_enabled)
return TRUE;
+ security_class = string_to_security_class (target_class);
+ if (security_class == 0)
+ {
+ saved_errno = errno;
+ log_callback (SELINUX_ERROR, "Unknown class %s", target_class);
+ if (security_deny_unknown () == 0)
+ {
+ return TRUE;
+ }
+
+ _dbus_verbose ("Unknown class %s\n", target_class);
+ errno = saved_errno;
+ return FALSE;
+ }
+
+ requested_access = string_to_av_perm (security_class, requested);
+ if (requested_access == 0)
+ {
+ saved_errno = errno;
+ log_callback (SELINUX_ERROR, "Unknown permission %s for class %s", requested, target_class);
+ if (security_deny_unknown () == 0)
+ {
+ return TRUE;
+ }
+
+ _dbus_verbose ("Unknown permission %s for class %s\n", requested, target_class);
+ errno = saved_errno;
+ return FALSE;
+ }
+
/* Make the security check. AVC checks enforcing mode here as well. */
if (avc_has_perm (SELINUX_SID_FROM_BUS (sender_sid),
override_sid ?
SELINUX_SID_FROM_BUS (override_sid) :
bus_sid,
- target_class, requested, &aeref, auxdata) < 0)
+ security_class, requested_access, &aeref, auxdata) < 0)
{
switch (errno)
{
@@ -471,8 +480,8 @@ bus_selinux_allows_acquire_service (DBusConnection *connection,
ret = bus_selinux_check (connection_sid,
service_sid,
- SECCLASS_DBUS,
- DBUS__ACQUIRE_SVC,
+ "dbus",
+ "acquire_svc",
&auxdata);
_dbus_string_free (&auxdata);
@@ -600,8 +609,8 @@ bus_selinux_allows_send (DBusConnection *sender,
ret = bus_selinux_check (sender_sid,
recipient_sid,
- SECCLASS_DBUS,
- DBUS__SEND_MSG,
+ "dbus",
+ "send_msg",
&auxdata);
_dbus_string_free (&auxdata);
--
2.23.0

View File

@ -0,0 +1,71 @@
From 678e73bfd034f2d32dde5ffd1c6b63c456989541 Mon Sep 17 00:00:00 2001
From: Peter Benie <pjb1008>
Date: Fri, 23 Jun 2023 11:51:00 +0100
Subject: [PATCH] bus: Don't crash if bus_context_create_client_policy() fails
If policy creation fails, we can't usefully leave a NULL policy in the
BusConnectionData. If we did, the next attempt to reload policy would
crash with a NULL dereference when we tried to unref it, or with
an assertion failure.
One situation in which we can legitimately fail to create a client policy
is an out-of-memory condition. Another is if we are unable to look up a
connection's supplementary groups with SO_PEERGROUPS, and also unable to
look up the connection's uid's groups in the system user database, for
example because it belongs to a user account that has been deleted (which
is sysadmin error, but can happen, particularly in automated test systems)
or because a service required by a Name Service Switch plugin has failed.
Keeping the last known policy is consistent with what happens to all
the connections that are after this one in iteration order: after we
early-return, all of those connections retain their previous policies
(which doesn't seem ideal either, but that's how this has always worked).
[smcv: Add commit message]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
(cherry picked from commit 63522f2887878e6b9e40c9bb6742484679631ea9)
---
bus/connection.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/bus/connection.c b/bus/connection.c
index 215f0230..7a482e2b 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1636,22 +1636,26 @@ bus_connections_reload_policy (BusConnections *connections,
link;
link = _dbus_list_get_next_link (&(connections->completed), link))
{
+ BusClientPolicy *policy;
+
connection = link->data;
d = BUS_CONNECTION_DATA (connection);
_dbus_assert (d != NULL);
_dbus_assert (d->policy != NULL);
- bus_client_policy_unref (d->policy);
- d->policy = bus_context_create_client_policy (connections->context,
- connection,
- error);
- if (d->policy == NULL)
+ policy = bus_context_create_client_policy (connections->context,
+ connection,
+ error);
+ if (policy == NULL)
{
_dbus_verbose ("Failed to create security policy for connection %p\n",
connection);
_DBUS_ASSERT_ERROR_IS_SET (error);
return FALSE;
}
+
+ bus_client_policy_unref (d->policy);
+ d->policy = policy;
}
return TRUE;
--
2.27.0

View File

@ -0,0 +1,112 @@
From b14c778eb5156f649610804bc4ce3a762aa2b58b Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 29 Jun 2023 19:52:39 +0100
Subject: [PATCH] bus: When failing to reload client policy, continue iteration
If we have a large number of connections to the bus, and we fail to
reload the policy for one of them (perhaps because its uid no longer
exists in the system user database), previously we would crash, which
is obviously unintended. After the previous commit, we would stop
iteration through the list of client connections, which doesn't seem
great either: one bad connection shouldn't prevent us from reloading
the rest of our state.
Instead, let's distinguish between new connections (where we want
failure to establish a security policy to be fatal), and pre-existing
connections (where the current security policy is presumably good
enough to keep using if we have nothing better). If we're unable to
reload the policy for a pre-existing connection, log a warning and
carry on iterating.
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
bus/bus.c | 35 +++++++++++++++++++++++++++++++++--
bus/bus.h | 1 +
bus/connection.c | 2 ++
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/bus/bus.c b/bus/bus.c
index 2ad8e789..f5100739 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -1285,11 +1285,42 @@ bus_context_get_policy (BusContext *context)
BusClientPolicy*
bus_context_create_client_policy (BusContext *context,
DBusConnection *connection,
+ BusClientPolicy *previous,
DBusError *error)
{
+ BusClientPolicy *client;
+ DBusError local_error = DBUS_ERROR_INIT;
+ const char *conn;
+ const char *loginfo;
+
_DBUS_ASSERT_ERROR_IS_CLEAR (error);
- return bus_policy_create_client_policy (context->policy, connection,
- error);
+
+ client = bus_policy_create_client_policy (context->policy, connection,
+ &local_error);
+
+ /* On success, use new policy */
+ if (client != NULL)
+ return client;
+
+ /* On failure while setting up a new connection, fail */
+ if (previous == NULL)
+ {
+ dbus_move_error (&local_error, error);
+ return NULL;
+ }
+
+ /* On failure while reloading, keep the previous policy */
+ conn = bus_connection_get_name (connection);
+ loginfo = bus_connection_get_loginfo (connection);
+
+ if (conn == NULL)
+ conn = "(inactive)";
+
+ bus_context_log (context, DBUS_SYSTEM_LOG_WARNING,
+ "Unable to reload policy for connection \"%s\" (%s), "
+ "keeping current policy: %s",
+ conn, loginfo, local_error.message);
+ return bus_client_policy_ref (previous);
}
int
diff --git a/bus/bus.h b/bus/bus.h
index 2e0de825..6ebea2c8 100644
--- a/bus/bus.h
+++ b/bus/bus.h
@@ -109,6 +109,7 @@ BusPolicy* bus_context_get_policy (BusContext
BusClientPolicy* bus_context_create_client_policy (BusContext *context,
DBusConnection *connection,
+ BusClientPolicy *previous,
DBusError *error);
int bus_context_get_activation_timeout (BusContext *context);
int bus_context_get_auth_timeout (BusContext *context);
diff --git a/bus/connection.c b/bus/connection.c
index 8a8ce5f4..b7439296 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1560,6 +1560,7 @@ bus_connection_complete (DBusConnection *connection,
d->policy = bus_context_create_client_policy (d->connections->context,
connection,
+ NULL,
error);
/* we may have a NULL policy on OOM or error getting list of
@@ -1645,6 +1646,7 @@ bus_connections_reload_policy (BusConnections *connections,
policy = bus_context_create_client_policy (connections->context,
connection,
+ d->policy,
error);
if (policy == NULL)
{
--
2.27.0

View File

@ -0,0 +1,64 @@
From b551b3e9737958216a1a9d359150a4110a9d0549 Mon Sep 17 00:00:00 2001
From: Jan Tojnar <jtojnar@gmail.com>
Date: Wed, 20 Apr 2022 11:07:25 +0200
Subject: [PATCH] bus/dir-watch: Do not crash with > 128 dirs
Without this running, dbus-daemon with long XDG_DATA_DIRS
will crash on out-of-bounds write:
$ XDG_DATA_DIRS=$(seq -f "/foo/%g" -s ':' 129) dbus-daemon --session
*** stack smashing detected ***: terminated
---
bus/dir-watch-inotify.c | 7 ++++++-
bus/dir-watch-kqueue.c | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/bus/dir-watch-inotify.c b/bus/dir-watch-inotify.c
index b52a24c0f..9beadb0ec 100644
--- a/bus/dir-watch-inotify.c
+++ b/bus/dir-watch-inotify.c
@@ -108,12 +108,17 @@ _set_watched_dirs_internal (DBusList **directories)
i = 0;
link = _dbus_list_get_first_link (directories);
- while (link != NULL)
+ while (link != NULL && i < MAX_DIRS_TO_WATCH)
{
new_dirs[i++] = (char *)link->data;
link = _dbus_list_get_next_link (directories, link);
}
+ if (link != NULL)
+ {
+ _dbus_warn ("Too many directories to watch them all, only watching first %d.", MAX_DIRS_TO_WATCH);
+ }
+
/* Look for directories in both the old and new sets, if
* we find one, move its data into the new set.
*/
diff --git a/bus/dir-watch-kqueue.c b/bus/dir-watch-kqueue.c
index 183db241c..15519fcb5 100644
--- a/bus/dir-watch-kqueue.c
+++ b/bus/dir-watch-kqueue.c
@@ -218,12 +218,17 @@ bus_set_watched_dirs (BusContext *context, DBusList **directories)
i = 0;
link = _dbus_list_get_first_link (directories);
- while (link != NULL)
+ while (link != NULL && i < MAX_DIRS_TO_WATCH)
{
new_dirs[i++] = (char *)link->data;
link = _dbus_list_get_next_link (directories, link);
}
+ if (link != NULL)
+ {
+ _dbus_warn ("Too many directories to watch them all, only watching first %d.", MAX_DIRS_TO_WATCH);
+ }
+
/* Look for directories in both the old and new sets, if
* we find one, move its data into the new set.
*/
--
GitLab

View File

@ -0,0 +1,42 @@
From 3a1b1e9a4010e581e2e940e61d37c4f617eb5eff Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 5 Jun 2023 17:56:33 +0100
Subject: [PATCH] monitor test: Log the messages that we monitored
This is helpful while debugging test failures.
Helps: dbus/dbus#457
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 8ee5d3e04420975107c27073b50f8758871a998b)
---
test/monitor.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/test/monitor.c b/test/monitor.c
index df5a7180..182110f8 100644
--- a/test/monitor.c
+++ b/test/monitor.c
@@ -196,6 +196,10 @@ _log_message (DBusMessage *m,
not_null (dbus_message_get_signature (m)));
g_test_message ("\terror name: %s",
not_null (dbus_message_get_error_name (m)));
+ g_test_message ("\tserial number: %u",
+ dbus_message_get_serial (m));
+ g_test_message ("\tin reply to: %u",
+ dbus_message_get_reply_serial (m));
if (strcmp ("s", dbus_message_get_signature (m)) == 0)
{
@@ -339,6 +343,9 @@ monitor_filter (DBusConnection *connection,
{
Fixture *f = user_data;
+ g_test_message ("Monitor received message:");
+ log_message (message);
+
g_assert_cmpstr (dbus_message_get_interface (message), !=,
"com.example.Tedious");
--
2.27.0

View File

@ -0,0 +1,197 @@
From 2c699f6ba9c162878c69d0728298c1ab7308db72 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 5 Jun 2023 18:51:22 +0100
Subject: [PATCH] monitor test: Reproduce dbus/dbus#457
The exact failure mode reported in dbus/dbus#457 is quite difficult
to achieve in a reliable way in a unit test, because we'd have to send
enough messages to a client to fill up its queue, then stop that client
from draining its queue, while still triggering a message that gets a
reply from the bus driver. However, we can trigger the same crash in a
slightly different way by not allowing the client to receive a
particular message. I chose NameAcquired.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 986611ad0f7f67a3693e5672cd66bc608c00b228)
---
.../valid-config-files/forbidding.conf.in | 3 +
test/monitor.c | 77 ++++++++++++++++---
2 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/test/data/valid-config-files/forbidding.conf.in b/test/data/valid-config-files/forbidding.conf.in
index d145613c..58b3cc6a 100644
--- a/test/data/valid-config-files/forbidding.conf.in
+++ b/test/data/valid-config-files/forbidding.conf.in
@@ -24,5 +24,8 @@
<allow send_interface="com.example.CannotUnicast2" send_broadcast="true"/>
<deny receive_interface="com.example.CannotReceive"/>
+
+ <!-- Used to reproduce dbus#457 -->
+ <deny receive_interface="org.freedesktop.DBus" receive_member="NameAcquired"/>
</policy>
</busconfig>
diff --git a/test/monitor.c b/test/monitor.c
index 182110f8..42e0734d 100644
--- a/test/monitor.c
+++ b/test/monitor.c
@@ -155,6 +155,21 @@ static Config side_effects_config = {
TRUE
};
+static dbus_bool_t
+config_forbids_name_acquired_signal (const Config *config)
+{
+ if (config == NULL)
+ return FALSE;
+
+ if (config->config_file == NULL)
+ return FALSE;
+
+ if (strcmp (config->config_file, forbidding_config.config_file) == 0)
+ return TRUE;
+
+ return FALSE;
+}
+
static inline const char *
not_null2 (const char *x,
const char *fallback)
@@ -253,9 +268,6 @@ do { \
#define assert_name_acquired(m) \
do { \
- DBusError _e = DBUS_ERROR_INIT; \
- const char *_s; \
- \
g_assert_cmpstr (dbus_message_type_to_string (dbus_message_get_type (m)), \
==, dbus_message_type_to_string (DBUS_MESSAGE_TYPE_SIGNAL)); \
g_assert_cmpstr (dbus_message_get_sender (m), ==, DBUS_SERVICE_DBUS); \
@@ -265,7 +277,14 @@ do { \
g_assert_cmpstr (dbus_message_get_signature (m), ==, "s"); \
g_assert_cmpint (dbus_message_get_serial (m), !=, 0); \
g_assert_cmpint (dbus_message_get_reply_serial (m), ==, 0); \
+} while (0)
+
+#define assert_unique_name_acquired(m) \
+do { \
+ DBusError _e = DBUS_ERROR_INIT; \
+ const char *_s; \
\
+ assert_name_acquired (m); \
dbus_message_get_args (m, &_e, \
DBUS_TYPE_STRING, &_s, \
DBUS_TYPE_INVALID); \
@@ -333,6 +352,21 @@ do { \
g_assert_cmpint (dbus_message_get_reply_serial (m), !=, 0); \
} while (0)
+/* forbidding.conf does not allow receiving NameAcquired, so if we are in
+ * that configuration, then dbus-daemon synthesizes an error reply to itself
+ * and sends that to monitors */
+#define expect_name_acquired_error(queue, in_reply_to) \
+do { \
+ DBusMessage *message; \
+ \
+ message = g_queue_pop_head (queue); \
+ assert_error_reply (message, DBUS_SERVICE_DBUS, DBUS_SERVICE_DBUS, \
+ DBUS_ERROR_ACCESS_DENIED); \
+ g_assert_cmpint (dbus_message_get_reply_serial (message), ==, \
+ dbus_message_get_serial (in_reply_to)); \
+ dbus_message_unref (message); \
+} while (0)
+
/* This is called after processing pending replies to our own method
* calls, but before anything else.
*/
@@ -797,6 +831,11 @@ test_become_monitor (Fixture *f,
test_assert_no_error (&f->e);
g_assert_cmpint (ret, ==, DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER);
+ /* If the policy forbids receiving NameAcquired, then we'll never
+ * receive it, so behave as though we had */
+ if (config_forbids_name_acquired_signal (f->config))
+ got_unique = got_a = got_b = got_c = TRUE;
+
while (!got_unique || !got_a || !got_b || !got_c)
{
if (g_queue_is_empty (&f->monitored))
@@ -1448,6 +1487,7 @@ test_dbus_daemon (Fixture *f,
{
DBusMessage *m;
int res;
+ size_t n_expected;
if (f->address == NULL)
return;
@@ -1463,7 +1503,12 @@ test_dbus_daemon (Fixture *f,
test_assert_no_error (&f->e);
g_assert_cmpint (res, ==, DBUS_RELEASE_NAME_REPLY_RELEASED);
- while (g_queue_get_length (&f->monitored) < 8)
+ n_expected = 8;
+
+ if (config_forbids_name_acquired_signal (context))
+ n_expected += 1;
+
+ while (g_queue_get_length (&f->monitored) < n_expected)
test_main_context_iterate (f->ctx, TRUE);
m = g_queue_pop_head (&f->monitored);
@@ -1476,10 +1521,12 @@ test_dbus_daemon (Fixture *f,
"NameOwnerChanged", "sss", NULL);
dbus_message_unref (m);
- /* FIXME: should we get this? */
m = g_queue_pop_head (&f->monitored);
- assert_signal (m, DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS,
- "NameAcquired", "s", f->sender_name);
+ assert_name_acquired (m);
+
+ if (config_forbids_name_acquired_signal (f->config))
+ expect_name_acquired_error (&f->monitored, m);
+
dbus_message_unref (m);
m = g_queue_pop_head (&f->monitored);
@@ -1701,8 +1748,14 @@ static void
expect_new_connection (Fixture *f)
{
DBusMessage *m;
+ size_t n_expected;
- while (g_queue_get_length (&f->monitored) < 4)
+ n_expected = 4;
+
+ if (config_forbids_name_acquired_signal (f->config))
+ n_expected += 1;
+
+ while (g_queue_get_length (&f->monitored) < n_expected)
test_main_context_iterate (f->ctx, TRUE);
m = g_queue_pop_head (&f->monitored);
@@ -1719,7 +1772,11 @@ expect_new_connection (Fixture *f)
dbus_message_unref (m);
m = g_queue_pop_head (&f->monitored);
- assert_name_acquired (m);
+ assert_unique_name_acquired (m);
+
+ if (config_forbids_name_acquired_signal (f->config))
+ expect_name_acquired_error (&f->monitored, m);
+
dbus_message_unref (m);
}
@@ -2044,6 +2101,8 @@ main (int argc,
setup, test_method_call, teardown);
g_test_add ("/monitor/forbidden-method", Fixture, &forbidding_config,
setup, test_forbidden_method_call, teardown);
+ g_test_add ("/monitor/forbidden-reply", Fixture, &forbidding_config,
+ setup, test_dbus_daemon, teardown);
g_test_add ("/monitor/dbus-daemon", Fixture, NULL,
setup, test_dbus_daemon, teardown);
g_test_add ("/monitor/selective", Fixture, &selective_config,
--
2.27.0

View File

@ -0,0 +1,187 @@
From 5337c629e945dec1b53884aa16daceb0500dea9b Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 29 Jun 2023 16:54:46 +0100
Subject: [PATCH] test: Add a targeted test for _dbus_unix_groups_from_uid()
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
test/Makefile.am | 4 ++
test/internals/userdb.c | 143 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 147 insertions(+)
create mode 100644 test/internals/userdb.c
diff --git a/test/Makefile.am b/test/Makefile.am
index 3bbf7f73..ffba702a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -121,6 +121,9 @@ test_server_oom_LDADD = libdbus-testutils.la $(GLIB_LIBS)
test_syslog_SOURCES = internals/syslog.c
test_syslog_LDADD = libdbus-testutils.la $(GLIB_LIBS)
+test_userdb_SOURCES = internals/userdb.c
+test_userdb_LDADD = libdbus-testutils.la $(GLIB_LIBS)
+
test_variant_SOURCES = internals/variant.c
test_variant_LDADD = libdbus-testutils.la $(GLIB_LIBS)
@@ -181,6 +184,7 @@ installable_tests += \
test-syntax \
test-syslog \
test-uid-permissions \
+ test-userdb \
test-variant \
$(NULL)
diff --git a/test/internals/userdb.c b/test/internals/userdb.c
new file mode 100644
index 00000000..905791b3
--- /dev/null
+++ b/test/internals/userdb.c
@@ -0,0 +1,143 @@
+/*
+ * Copyright © 2023 Collabora Ltd.
+ * SPDX-License-Identifier: MIT
+ */
+
+#include <config.h>
+
+#include <glib.h>
+
+#include <dbus/dbus.h>
+#include "dbus/dbus-sysdeps.h"
+#include "test-utils-glib.h"
+
+#ifdef DBUS_UNIX
+#include <errno.h>
+#include <pwd.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "dbus/dbus-sysdeps-unix.h"
+#include "dbus/dbus-userdb.h"
+#endif
+
+typedef struct
+{
+ int dummy;
+} Fixture;
+
+static void
+setup (Fixture *f G_GNUC_UNUSED,
+ gconstpointer context G_GNUC_UNUSED)
+{
+}
+
+static void
+test_groups_from_uid (Fixture *f,
+ gconstpointer context G_GNUC_UNUSED)
+{
+ DBusError error = DBUS_ERROR_INIT;
+ dbus_gid_t *gids = NULL;
+ int n_gids = -1;
+ dbus_bool_t ret;
+#ifdef DBUS_UNIX
+ int i;
+#endif
+
+ /* We assume that uid 0 (root) is available on all Unix systems,
+ * so this should succeed */
+ ret = _dbus_unix_groups_from_uid (0, &gids, &n_gids, &error);
+
+#ifdef DBUS_UNIX
+ test_assert_no_error (&error);
+ g_assert_true (ret);
+ g_assert_cmpint (n_gids, >=, 0);
+
+ g_test_message ("Groups of uid 0:");
+
+ for (i = 0; i < n_gids; i++)
+ {
+ g_test_message ("[%d]: %ld", i, (long) gids[i]);
+ g_assert_cmpint (gids[i], >=, 0);
+ }
+#else
+ g_assert_cmpstr (error.name, ==, DBUS_ERROR_NOT_SUPPORTED);
+ g_assert_false (ret);
+ g_test_message ("Getting Unix groups on Windows failed as expected: %s: %s",
+ error.name, error.message);
+ g_assert_null (gids);
+ g_assert_cmpint (n_gids, <=, 0);
+#endif
+
+ dbus_free (gids);
+ dbus_error_free (&error);
+
+#ifdef DBUS_UNIX
+ /* Assume that the current uid is something sensible */
+ ret = _dbus_unix_groups_from_uid (geteuid (), &gids, &n_gids, &error);
+ test_assert_no_error (&error);
+ g_assert_true (ret);
+ g_assert_cmpint (n_gids, >=, 0);
+
+ g_test_message ("Groups of uid %ld:", (long) geteuid ());
+
+ for (i = 0; i < n_gids; i++)
+ {
+ g_test_message ("[%d]: %ld", i, (long) gids[i]);
+ g_assert_cmpint (gids[i], >=, 0);
+ }
+
+ g_test_message ("Total: %i groups", n_gids);
+
+ dbus_free (gids);
+ dbus_error_free (&error);
+
+ errno = 0;
+
+ /* arbitrarily chosen, probably isn't a valid uid */
+ if (getpwuid (31337) == NULL)
+ {
+ g_test_message ("uid 31337 doesn't exist: %s",
+ errno == 0 ? "(no errno)" : g_strerror (errno));
+ ret = _dbus_unix_groups_from_uid (31337, &gids, &n_gids, &error);
+ g_assert_nonnull (error.name);
+ g_assert_nonnull (error.message);
+ g_assert_false (ret);
+ g_test_message ("Getting groups from non-uid failed as expected: %s: %s",
+ error.name, error.message);
+ /* The Unix implementation always clears gids/n_gids,
+ * even on failure, and even if they were uninitialized */
+ g_assert_null (gids);
+ g_assert_cmpint (n_gids, ==, 0);
+
+ dbus_free (gids);
+ dbus_error_free (&error);
+ }
+ else
+ {
+ g_test_skip ("against our expectations, uid 31337 exists on this system");
+ }
+#endif
+}
+
+static void
+teardown (Fixture *f G_GNUC_UNUSED,
+ gconstpointer context G_GNUC_UNUSED)
+{
+}
+
+int
+main (int argc,
+ char **argv)
+{
+ int ret;
+
+ test_init (&argc, &argv);
+
+ g_test_add ("/userdb/groups_from_uid",
+ Fixture, NULL, setup, test_groups_from_uid, teardown);
+
+ ret = g_test_run ();
+ dbus_shutdown ();
+ return ret;
+}
--
2.27.0

View File

@ -0,0 +1,224 @@
From fc757d494089b7e1e4e37b7eaaa798cd7e9ad391 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 29 Jun 2023 16:06:39 +0100
Subject: [PATCH] userdb: Add proper error reporting when getting groups from a
uid
Previously, if dbus_connection_get_unix_user() succeeded but
_dbus_unix_groups_from_uid() failed, then bus_connection_get_unix_groups()
would incorrectly fail without setting the error indicator, resulting
in "(null)" being logged, which is rather unhelpful.
This also lets us distinguish between ENOMEM and other errors, such as
the uid not existing in the system's user database.
Fixes: 145fb99b (untitled refactoring commit, 2006-12-12)
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
bus/connection.c | 2 +-
bus/policy.c | 2 +-
dbus/dbus-sysdeps-util-unix.c | 6 ++++--
dbus/dbus-sysdeps-util-win.c | 15 ++++++++++++---
dbus/dbus-sysdeps.h | 3 ++-
dbus/dbus-userdb-util.c | 19 ++++++++++++-------
dbus/dbus-userdb.h | 3 ++-
7 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/bus/connection.c b/bus/connection.c
index 7a482e2b..8a8ce5f4 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1009,7 +1009,7 @@ bus_connection_get_unix_groups (DBusConnection *connection,
if (dbus_connection_get_unix_user (connection, &uid))
{
- if (!_dbus_unix_groups_from_uid (uid, groups, n_groups))
+ if (!_dbus_unix_groups_from_uid (uid, groups, n_groups, error))
{
_dbus_verbose ("Did not get any groups for UID %lu\n",
uid);
diff --git a/bus/policy.c b/bus/policy.c
index a37be804..6c22a103 100644
--- a/bus/policy.c
+++ b/bus/policy.c
@@ -450,7 +450,7 @@ bus_policy_allow_unix_user (BusPolicy *policy,
int n_group_ids;
/* On OOM or error we always reject the user */
- if (!_dbus_unix_groups_from_uid (uid, &group_ids, &n_group_ids))
+ if (!_dbus_unix_groups_from_uid (uid, &group_ids, &n_group_ids, NULL))
{
_dbus_verbose ("Did not get any groups for UID %lu\n",
uid);
diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c
index bed6fd3e..961b3eff 100644
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -987,14 +987,16 @@ _dbus_parse_unix_group_from_config (const DBusString *groupname,
* @param uid the UID
* @param group_ids return location for array of group IDs
* @param n_group_ids return location for length of returned array
+ * @param error error location
* @returns #TRUE if the UID existed and we got some credentials
*/
dbus_bool_t
_dbus_unix_groups_from_uid (dbus_uid_t uid,
dbus_gid_t **group_ids,
- int *n_group_ids)
+ int *n_group_ids,
+ DBusError *error)
{
- return _dbus_groups_from_uid (uid, group_ids, n_group_ids);
+ return _dbus_groups_from_uid (uid, group_ids, n_group_ids, error);
}
/**
diff --git a/dbus/dbus-sysdeps-util-win.c b/dbus/dbus-sysdeps-util-win.c
index 8c8fbed8..42b1acd2 100644
--- a/dbus/dbus-sysdeps-util-win.c
+++ b/dbus/dbus-sysdeps-util-win.c
@@ -647,6 +647,13 @@ dbus_bool_t _dbus_windows_user_is_process_owner (const char *windows_sid)
unix emulation functions - should be removed sometime in the future
=====================================================================*/
+static void
+set_unix_uid_unsupported (DBusError *error)
+{
+ dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
+ "UNIX user IDs not supported on Windows");
+}
+
/**
* Checks to see if the UNIX user ID is at the console.
* Should always fail on Windows (set the error to
@@ -660,8 +667,7 @@ dbus_bool_t
_dbus_unix_user_is_at_console (dbus_uid_t uid,
DBusError *error)
{
- dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
- "UNIX user IDs not supported on Windows\n");
+ set_unix_uid_unsupported (error);
return FALSE;
}
@@ -705,13 +711,16 @@ _dbus_parse_unix_user_from_config (const DBusString *username,
* @param uid the UID
* @param group_ids return location for array of group IDs
* @param n_group_ids return location for length of returned array
+ * @param error error location
* @returns #TRUE if the UID existed and we got some credentials
*/
dbus_bool_t
_dbus_unix_groups_from_uid (dbus_uid_t uid,
dbus_gid_t **group_ids,
- int *n_group_ids)
+ int *n_group_ids,
+ DBusError *error)
{
+ set_unix_uid_unsupported (error);
return FALSE;
}
diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index 24fbec6a..93d85d28 100644
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -278,7 +278,8 @@ dbus_bool_t _dbus_parse_unix_group_from_config (const DBusString *groupname,
dbus_gid_t *gid_p);
dbus_bool_t _dbus_unix_groups_from_uid (dbus_uid_t uid,
dbus_gid_t **group_ids,
- int *n_group_ids);
+ int *n_group_ids,
+ DBusError *error);
dbus_bool_t _dbus_unix_user_is_at_console (dbus_uid_t uid,
DBusError *error);
dbus_bool_t _dbus_unix_user_is_process_owner (dbus_uid_t uid);
diff --git a/dbus/dbus-userdb-util.c b/dbus/dbus-userdb-util.c
index 170d233e..44c898ee 100644
--- a/dbus/dbus-userdb-util.c
+++ b/dbus/dbus-userdb-util.c
@@ -406,31 +406,35 @@ _dbus_user_database_get_gid (DBusUserDatabase *db,
* @param uid the UID
* @param group_ids return location for array of group IDs
* @param n_group_ids return location for length of returned array
+ * @param error error to fill in on failure
* @returns #TRUE if the UID existed and we got some credentials
*/
dbus_bool_t
_dbus_groups_from_uid (dbus_uid_t uid,
dbus_gid_t **group_ids,
- int *n_group_ids)
+ int *n_group_ids,
+ DBusError *error)
{
DBusUserDatabase *db;
const DBusUserInfo *info;
*group_ids = NULL;
*n_group_ids = 0;
- /* FIXME: this can't distinguish ENOMEM from other errors */
if (!_dbus_user_database_lock_system ())
- return FALSE;
+ {
+ _DBUS_SET_OOM (error);
+ return FALSE;
+ }
db = _dbus_user_database_get_system ();
if (db == NULL)
{
+ _DBUS_SET_OOM (error);
_dbus_user_database_unlock_system ();
return FALSE;
}
- if (!_dbus_user_database_get_uid (db, uid,
- &info, NULL))
+ if (!_dbus_user_database_get_uid (db, uid, &info, error))
{
_dbus_user_database_unlock_system ();
return FALSE;
@@ -443,6 +447,7 @@ _dbus_groups_from_uid (dbus_uid_t uid,
*group_ids = dbus_new (dbus_gid_t, info->n_group_ids);
if (*group_ids == NULL)
{
+ _DBUS_SET_OOM (error);
_dbus_user_database_unlock_system ();
return FALSE;
}
@@ -473,7 +478,7 @@ _dbus_userdb_test (const char *test_data_dir)
dbus_uid_t uid;
unsigned long *group_ids;
int n_group_ids, i;
- DBusError error;
+ DBusError error = DBUS_ERROR_INIT;
if (!_dbus_username_from_current_process (&username))
_dbus_assert_not_reached ("didn't get username");
@@ -484,7 +489,7 @@ _dbus_userdb_test (const char *test_data_dir)
if (!_dbus_get_user_id (username, &uid))
_dbus_assert_not_reached ("didn't get uid");
- if (!_dbus_groups_from_uid (uid, &group_ids, &n_group_ids))
+ if (!_dbus_groups_from_uid (uid, &group_ids, &n_group_ids, &error))
_dbus_assert_not_reached ("didn't get groups");
printf (" Current user: %s homedir: %s gids:",
diff --git a/dbus/dbus-userdb.h b/dbus/dbus-userdb.h
index b38e3d18..b161b395 100644
--- a/dbus/dbus-userdb.h
+++ b/dbus/dbus-userdb.h
@@ -111,7 +111,8 @@ dbus_bool_t _dbus_credentials_from_uid (dbus_uid_t user_id,
DBusCredentials *credentials);
dbus_bool_t _dbus_groups_from_uid (dbus_uid_t uid,
dbus_gid_t **group_ids,
- int *n_group_ids);
+ int *n_group_ids,
+ DBusError *error);
DBUS_PRIVATE_EXPORT
dbus_bool_t _dbus_is_console_user (dbus_uid_t uid,
DBusError *error);
--
2.27.0

View File

@ -1,7 +1,7 @@
Name: dbus
Epoch: 1
Version: 1.12.20
Release: 6
Release: 11
Summary: System Message Bus
License: AFLv3.0 or GPLv2+
URL: http://www.freedesktop.org/Software/dbus/
@ -16,6 +16,19 @@ Patch6001: backport-bus-Also-tell-systemd-when-we-re-reloading.patch
Patch6002: backport-bus-Also-tell-systemd-before-we-shut-down.patch
Patch6003: backport-bus-Don-t-pass-systemd-environment-variables-to-acti.patch
Patch6004: backport-bus-Clear-INVOCATION_ID-when-carrying-out-traditiona.patch
Patch6005: backport-Stop-using-selinux_set_mapping-function.patch
Patch6006: backport-CVE-2022-42010.patch
Patch6007: backport-CVE-2022-42011.patch
Patch6008: backport-CVE-2022-42012.patch
Patch6009: backport-bus-dir-watch-Do-not-crash-with-128-dirs.patch
Patch6010: backport-Do-not-crash-when-reloading-configuration.patch
Patch6011: backport-CVE-2023-34969.patch
Patch6012: backport-monitor-test-Log-the-messages-that-we-monitored.patch
Patch6013: backport-monitor-test-Reproduce-dbus-dbus-457.patch
Patch6014: backport-test-Add-a-targeted-test-for-_dbus_unix_groups_from_.patch
Patch6015: backport-userdb-Add-proper-error-reporting-when-getting-group.patch
Patch6016: backport-bus-Don-t-crash-if-bus_context_create_client_policy-.patch
Patch6017: backport-bus-When-failing-to-reload-client-policy-continue-it.patch
BuildRequires: systemd-devel expat-devel libselinux-devel audit-libs-devel doxygen xmlto cmake
BuildRequires: autoconf-archive libtool libX11-devel libcap-ng-devel libxslt
@ -96,6 +109,7 @@ Man pages and other related documents for D-Bus.
%autosetup -n %{name}-%{version} -p1
%build
aclocal
%configure \
--disable-static \
--enable-inotify \
@ -228,6 +242,22 @@ fi
%exclude %{_pkgdocdir}/README
%changelog
* Tue Jun 18 2024 hongjinghao <hongjinghao@huawei.com> - 1:1.12.20-11
- Sync patches from dbus community
* Tue Jun 18 2024 hongjinghao <hongjinghao@huawei.com> - 1:1.12.20-10
- fix CVE-2023-34969
* Wed Feb 21 2024 hongjinghao <hongjinghao@huawei.com> - 1:1.12.20-9
- add backport-bus-dir-watch-Do-not-crash-with-128-dirs.patch
backport-Do-not-crash-when-reloading-configuration.patch
* Mon Oct 17 2022 hongjinghao <hongjinghao@huawei.com> - 1:1.12.20-8
- fix CVE-2022-42010,CVE-2022-42011,CVE-2022-42012
* Tue Sep 20 2022 hongjinghao <hongjinghao@huawei.com> - 1:1.12.20-7
- Stop using selinux set_mapping function.
* Sat Jan 29 2022 licunlong <licunlong1@huawei.com> - 1:1.12.20-6
- Tell systemd when dbus is ready/shutting down/reloading config.