Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge branch 'jb53541' into 'master'
[systemd] Fix CVE-2020-1712. Fixes JB#53541

See merge request mer-core/systemd!45
  • Loading branch information
Matti Kosola committed Mar 16, 2021
2 parents 782a22a + 0fe3b1c commit 1604d9d
Show file tree
Hide file tree
Showing 2 changed files with 198 additions and 0 deletions.
197 changes: 197 additions & 0 deletions rpm/systemd-245-polkit-async-CVE-2020-1712.diff
@@ -0,0 +1,197 @@
Fix for CVE-2020-1712.

A heap use-after-free vulnerability was found in systemd before version
v245-rc1, where asynchronous Polkit queries are performed while handling dbus
messages. A local unprivileged attacker can abuse this flaw to crash systemd
services or potentially execute code and elevate their privileges, by sending
specially crafted dbus messages.

This patch is based on the following upstream commits:
https://github.com/systemd/systemd/commit/1068447e6954dc6ce52f099ed174c442cb89ed54
https://github.com/systemd/systemd/commit/637486261528e8aa3da9f26a4487dc254f4b7abb
https://github.com/systemd/systemd/commit/bc130b6858327b382b07b3985cf48e2aa9016b2d

diff -purN systemd/src/libsystemd/sd-bus/bus-message.h systemd-izh/src/libsystemd/sd-bus/bus-message.h
--- systemd/src/libsystemd/sd-bus/bus-message.h 2021-03-12 19:24:48.837860321 +0000
+++ systemd-izh/src/libsystemd/sd-bus/bus-message.h 2021-03-12 19:56:36.396779572 +0000
@@ -232,3 +232,4 @@ int bus_message_append_sender(sd_bus_mes

void bus_message_set_sender_driver(sd_bus *bus, sd_bus_message *m);
void bus_message_set_sender_local(sd_bus *bus, sd_bus_message *m);
+int sd_bus_enqueue_for_read(sd_bus *bus, sd_bus_message *m);
diff -purN systemd/src/libsystemd/sd-bus/sd-bus.c systemd-izh/src/libsystemd/sd-bus/sd-bus.c
--- systemd/src/libsystemd/sd-bus/sd-bus.c 2021-03-12 19:24:48.841860276 +0000
+++ systemd-izh/src/libsystemd/sd-bus/sd-bus.c 2021-03-12 19:56:17.176986593 +0000
@@ -4111,3 +4111,27 @@ _public_ int sd_bus_get_n_queued_write(s
*ret = bus->wqueue_size;
return 0;
}
+
+int sd_bus_enqueue_for_read(sd_bus *bus, sd_bus_message *m) {
+ int r;
+
+ assert_return(bus, -EINVAL);
+ assert_return(bus = bus_resolve(bus), -ENOPKG);
+ assert_return(m, -EINVAL);
+ assert_return(m->sealed, -EINVAL);
+ assert_return(!bus_pid_changed(bus), -ECHILD);
+
+ if (!BUS_IS_OPEN(bus->state))
+ return -ENOTCONN;
+
+ /* Re-enqueue a message for reading. This is primarily useful for PolicyKit-style authentication,
+ * where we accept a message, then determine we need to interactively authenticate the user, and then
+ * we want to process the message again. */
+
+ r = bus_rqueue_make_room(bus);
+ if (r < 0)
+ return r;
+
+ bus->rqueue[bus->rqueue_size++] = sd_bus_message_ref(m);
+ return 0;
+}
diff -purN systemd/src/shared/bus-util.c systemd-izh/src/shared/bus-util.c
--- systemd/src/shared/bus-util.c 2021-03-12 19:24:48.881859831 +0000
+++ systemd-izh/src/shared/bus-util.c 2021-03-12 19:55:17.197632645 +0000
@@ -337,14 +337,13 @@ int bus_test_polkit(

typedef struct AsyncPolkitQuery {
sd_bus_message *request, *reply;
- sd_bus_message_handler_t callback;
- void *userdata;
sd_bus_slot *slot;
+
Hashmap *registry;
+ sd_event_source *defer_event_source;
} AsyncPolkitQuery;

static void async_polkit_query_free(AsyncPolkitQuery *q) {
-
if (!q)
return;

@@ -356,9 +355,24 @@ static void async_polkit_query_free(Asyn
sd_bus_message_unref(q->request);
sd_bus_message_unref(q->reply);

+ if (q->defer_event_source)
+ (void) sd_event_source_set_enabled(q->defer_event_source, SD_EVENT_OFF);
+ sd_event_source_unref(q->defer_event_source);
free(q);
}

+static int async_polkit_defer(sd_event_source *s, void *userdata) {
+ AsyncPolkitQuery *q = userdata;
+
+ assert(s);
+
+ /* This is called as idle event source after we processed the async polkit reply, hopefully after the
+ * method call we re-enqueued has been properly processed. */
+
+ async_polkit_query_free(q);
+ return 0;
+}
+
static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) {
_cleanup_(sd_bus_error_free) sd_bus_error error_buffer = SD_BUS_ERROR_NULL;
AsyncPolkitQuery *q = userdata;
@@ -367,21 +381,46 @@ static int async_polkit_callback(sd_bus_
assert(reply);
assert(q);

+ assert(q->slot);
q->slot = sd_bus_slot_unref(q->slot);
+
+ assert(!q->reply);
q->reply = sd_bus_message_ref(reply);

+ /* Now, let's dispatch the original message a second time be re-enqueing. This will then traverse the
+ * whole message processing again, and thus re-validating and re-retrieving the "userdata" field
+ * again.
+ *
+ * We install an idle event loop event to clean-up the PolicyKit request data when we are idle again,
+ * i.e. after the second time the message is processed is complete. */
+
+ assert(!q->defer_event_source);
+ r = sd_event_add_defer(sd_bus_get_event(sd_bus_message_get_bus(reply)), &q->defer_event_source, async_polkit_defer, q);
+ if (r < 0)
+ goto fail;
+
+ r = sd_event_source_set_priority(q->defer_event_source, SD_EVENT_PRIORITY_IDLE);
+ if (r < 0)
+ goto fail;
+
+ r = sd_event_source_set_enabled(q->defer_event_source, SD_EVENT_ONESHOT);
+ if (r < 0)
+ goto fail;
+
r = sd_bus_message_rewind(q->request, true);
- if (r < 0) {
- r = sd_bus_reply_method_errno(q->request, r, NULL);
- goto finish;
- }
+ if (r < 0)
+ goto fail;

- r = q->callback(q->request, q->userdata, &error_buffer);
- r = bus_maybe_reply_error(q->request, r, &error_buffer);
+ r = sd_bus_enqueue_for_read(sd_bus_message_get_bus(q->request), q->request);
+ if (r < 0)
+ goto fail;

-finish:
- async_polkit_query_free(q);
+ return 1;

+fail:
+ log_debug_errno(r, "Processing asynchronous PolicyKit reply failed, ignoring: %m");
+ (void) sd_bus_reply_method_errno(q->request, r, NULL);
+ async_polkit_query_free(q);
return r;
}

@@ -400,11 +439,10 @@ int bus_verify_polkit_async(
#if ENABLE_POLKIT
_cleanup_(sd_bus_message_unrefp) sd_bus_message *pk = NULL;
AsyncPolkitQuery *q;
- const char *sender, **k, **v;
- sd_bus_message_handler_t callback;
- void *userdata;
+ const char **k, **v;
int c;
#endif
+ const char *sender;
int r;

assert(call);
@@ -462,20 +500,11 @@ int bus_verify_polkit_async(
else if (r > 0)
return 1;

-#if ENABLE_POLKIT
- if (sd_bus_get_current_message(call->bus) != call)
- return -EINVAL;
-
- callback = sd_bus_get_current_handler(call->bus);
- if (!callback)
- return -EINVAL;
-
- userdata = sd_bus_get_current_userdata(call->bus);
-
sender = sd_bus_message_get_sender(call);
if (!sender)
return -EBADMSG;

+#if ENABLE_POLKIT
c = sd_bus_message_get_allow_interactive_authorization(call);
if (c < 0)
return c;
@@ -527,8 +556,6 @@ int bus_verify_polkit_async(
return -ENOMEM;

q->request = sd_bus_message_ref(call);
- q->callback = callback;
- q->userdata = userdata;

r = hashmap_put(*registry, call, q);
if (r < 0) {
1 change: 1 addition & 0 deletions rpm/systemd.spec
Expand Up @@ -58,6 +58,7 @@ Patch57: systemd-revert-PID-file-hardening-for-booster-silica-qt5.diff
Patch58: systemd-240-core-remove-support-for-API-bus-started-outside-our-.patch
Patch59: systemd-240-units-add-new-system-update-pre.target.patch
Patch60: systemd-Fix-udev-firmware-events-dependencies.patch
Patch61: systemd-245-polkit-async-CVE-2020-1712.diff
# This patch serves two purposes: it adds needed "#include <sys/sysmacros.h>"
# and initializes variables with automatic cleanup functions to silence
# compiler warnings.
Expand Down

0 comments on commit 1604d9d

Please sign in to comment.