be0c12
From 8e322f5bc24547963978be071a8a2547abad875a Mon Sep 17 00:00:00 2001
be0c12
From: Henri Chain <henri.chain@enioka.com>
be0c12
Date: Tue, 5 Oct 2021 13:10:31 +0200
be0c12
Subject: [PATCH] core: fix SIGABRT on empty exec command argv
be0c12
be0c12
This verifies that the argv part of any exec_command parameters that
be0c12
are sent through dbus is not empty at deserialization time.
be0c12
be0c12
There is an additional check in service.c service_verify() that again
be0c12
checks if all exec_commands are correctly populated, after the service
be0c12
has been loaded, whether through dbus or otherwise.
be0c12
be0c12
Fixes #20933.
be0c12
be0c12
(cherry picked from commit 29500cf8c47e6eb0518d171d62aa8213020c9152)
be0c12
be0c12
Resolves: #2020239
be0c12
---
be0c12
 src/core/dbus-execute.c             |  4 ++++
be0c12
 src/core/service.c                  | 12 +++++++++++
be0c12
 test/TEST-23-TYPE-EXEC/testsuite.sh | 31 +++++++++++++++++++++++++++++
be0c12
 3 files changed, 47 insertions(+)
be0c12
be0c12
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
be0c12
index 8348663000..2e64f0baf4 100644
be0c12
--- a/src/core/dbus-execute.c
be0c12
+++ b/src/core/dbus-execute.c
be0c12
@@ -969,6 +969,10 @@ int bus_set_transient_exec_command(
be0c12
                 if (r < 0)
be0c12
                         return r;
be0c12
 
be0c12
+                if (strv_isempty(argv))
be0c12
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
be0c12
+                                                 "\"%s\" argv cannot be empty", name);
be0c12
+
be0c12
                 r = sd_bus_message_read(message, "b", &b);
be0c12
                 if (r < 0)
be0c12
                         return r;
be0c12
diff --git a/src/core/service.c b/src/core/service.c
be0c12
index 5e3e75b5ae..12adf89dd4 100644
be0c12
--- a/src/core/service.c
be0c12
+++ b/src/core/service.c
be0c12
@@ -536,6 +536,18 @@ static int service_verify(Service *s) {
be0c12
         if (UNIT(s)->load_state != UNIT_LOADED)
be0c12
                 return 0;
be0c12
 
be0c12
+        for (ServiceExecCommand c = 0; c < _SERVICE_EXEC_COMMAND_MAX; c++) {
be0c12
+                ExecCommand *command;
be0c12
+
be0c12
+                LIST_FOREACH(command, command, s->exec_command[c])
be0c12
+                        if (strv_isempty(command->argv)) {
be0c12
+                                log_unit_error(UNIT(s),
be0c12
+                                               "Service has an empty argv in %s=. Refusing.",
be0c12
+                                               service_exec_command_to_string(c));
be0c12
+                                return -ENOEXEC;
be0c12
+                        }
be0c12
+        }
be0c12
+
be0c12
         if (!s->exec_command[SERVICE_EXEC_START] && !s->exec_command[SERVICE_EXEC_STOP]) {
be0c12
                 log_unit_error(UNIT(s), "Service lacks both ExecStart= and ExecStop= setting. Refusing.");
be0c12
                 return -ENOEXEC;
be0c12
diff --git a/test/TEST-23-TYPE-EXEC/testsuite.sh b/test/TEST-23-TYPE-EXEC/testsuite.sh
be0c12
index 80734bbbdc..e0c34cfd04 100755
be0c12
--- a/test/TEST-23-TYPE-EXEC/testsuite.sh
be0c12
+++ b/test/TEST-23-TYPE-EXEC/testsuite.sh
be0c12
@@ -21,6 +21,37 @@ systemd-run --unit=four -p Type=exec /bin/sleep infinity
be0c12
 ! systemd-run --unit=five -p Type=exec -p User=idontexist /bin/sleep infinity
be0c12
 ! systemd-run --unit=six -p Type=exec /tmp/brokenbinary
be0c12
 
be0c12
+# For issue #20933
be0c12
+
be0c12
+# Should work normally
be0c12
+busctl call \
be0c12
+  org.freedesktop.systemd1 /org/freedesktop/systemd1 \
be0c12
+  org.freedesktop.systemd1.Manager StartTransientUnit \
be0c12
+  "ssa(sv)a(sa(sv))" test-20933-ok.service replace 1 \
be0c12
+    ExecStart "a(sasb)" 1 \
be0c12
+      /usr/bin/sleep 2 /usr/bin/sleep 1 true \
be0c12
+  0
be0c12
+
be0c12
+# DBus call should fail but not crash systemd
be0c12
+busctl call \
be0c12
+  org.freedesktop.systemd1 /org/freedesktop/systemd1 \
be0c12
+  org.freedesktop.systemd1.Manager StartTransientUnit \
be0c12
+  "ssa(sv)a(sa(sv))" test-20933-bad.service replace 1 \
be0c12
+    ExecStart "a(sasb)" 1 \
be0c12
+      /usr/bin/sleep 0 true \
be0c12
+  0 && { echo 'unexpected success'; exit 1; }
be0c12
+
be0c12
+# Same but with the empty argv in the middle
be0c12
+busctl call \
be0c12
+  org.freedesktop.systemd1 /org/freedesktop/systemd1 \
be0c12
+  org.freedesktop.systemd1.Manager StartTransientUnit \
be0c12
+  "ssa(sv)a(sa(sv))" test-20933-bad-middle.service replace 1 \
be0c12
+    ExecStart "a(sasb)" 3 \
be0c12
+      /usr/bin/sleep 2 /usr/bin/sleep 1 true \
be0c12
+      /usr/bin/sleep 0                  true \
be0c12
+      /usr/bin/sleep 2 /usr/bin/sleep 1 true \
be0c12
+  0 && { echo 'unexpected success'; exit 1; }
be0c12
+
be0c12
 systemd-analyze set-log-level info
be0c12
 
be0c12
 echo OK > /testok