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