core: fix SIGABRT on empty exec command argv

This verifies that the argv part of any exec_command parameters that
are sent through dbus is not empty at deserialization time.

There is an additional check in service.c service_verify() that again
checks if all exec_commands are correctly populated, after the service
has been loaded, whether through dbus or otherwise.

Fixes #20933.

(cherry picked from commit 29500cf8c47e6eb0518d171d62aa8213020c9152)
(cherry picked from commit 7a58bf7aac8b2c812ee0531b0cc426e0067edd35)

diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 5239c41d673556444da6656cf86f94cb5f8dd330..c0a7ad61afdedfbf07bbf19bdf0c4d65df6d59cf 100644 (file)
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -1420,6 +1420,10 @@ int bus_set_transient_exec_command(
                 if (r < 0)
                         return r;
 
+                if (strv_isempty(argv))
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
+                                                 "\"%s\" argv cannot be empty", name);
+
                 r = is_ex_prop ? sd_bus_message_read_strv(message, &ex_opts) : sd_bus_message_read(message, "b", &b);
                 if (r < 0)
                         return r;

diff --git a/src/core/service.c b/src/core/service.c
index 07443fec43438d4a751fbeb2f6ca5e37e0eec54e..8c6f9837a7190754b03c424379806f02d64a4c93 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -548,6 +548,16 @@ static int service_verify(Service *s) {
         assert(s);
         assert(UNIT(s)->load_state == UNIT_LOADED);
 
+        for (ServiceExecCommand c = 0; c < _SERVICE_EXEC_COMMAND_MAX; c++) {
+                ExecCommand *command;
+
+                LIST_FOREACH(command, command, s->exec_command[c])
+                        if (strv_isempty(command->argv))
+                                return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC),
+                                                            "Service has an empty argv in %s=. Refusing.",
+                                                            service_exec_command_to_string(c));
+        }
+
         if (!s->exec_command[SERVICE_EXEC_START] && !s->exec_command[SERVICE_EXEC_STOP] &&
             UNIT(s)->success_action == EMERGENCY_ACTION_NONE)
                 /* FailureAction= only makes sense if one of the start or stop commands is specified.

diff --git a/test/units/testsuite-23.sh b/test/units/testsuite-23.sh
index 5e2966f848e0cd60013146a4400023455794cc1c..7bcbbf9925e98f47d552bfa737832516cb1bf00d 100755 (executable)
--- a/test/units/testsuite-23.sh
+++ b/test/units/testsuite-23.sh
@@ -27,6 +27,37 @@ test $(systemctl show --value -p RestartKillSignal seven.service) -eq 2
 systemctl restart seven.service
 systemctl stop seven.service
 
+# For issue #20933
+
+# Should work normally
+busctl call \
+  org.freedesktop.systemd1 /org/freedesktop/systemd1 \
+  org.freedesktop.systemd1.Manager StartTransientUnit \
+  "ssa(sv)a(sa(sv))" test-20933-ok.service replace 1 \
+    ExecStart "a(sasb)" 1 \
+      /usr/bin/sleep 2 /usr/bin/sleep 1 true \
+  0
+
+# DBus call should fail but not crash systemd
+busctl call \
+  org.freedesktop.systemd1 /org/freedesktop/systemd1 \
+  org.freedesktop.systemd1.Manager StartTransientUnit \
+  "ssa(sv)a(sa(sv))" test-20933-bad.service replace 1 \
+    ExecStart "a(sasb)" 1 \
+      /usr/bin/sleep 0 true \
+  0 && { echo 'unexpected success'; exit 1; }
+
+# Same but with the empty argv in the middle
+busctl call \
+  org.freedesktop.systemd1 /org/freedesktop/systemd1 \
+  org.freedesktop.systemd1.Manager StartTransientUnit \
+  "ssa(sv)a(sa(sv))" test-20933-bad-middle.service replace 1 \
+    ExecStart "a(sasb)" 3 \
+      /usr/bin/sleep 2 /usr/bin/sleep 1 true \
+      /usr/bin/sleep 0                  true \
+      /usr/bin/sleep 2 /usr/bin/sleep 1 true \
+  0 && { echo 'unexpected success'; exit 1; }
+
 systemd-analyze log-level info
 
 echo OK > /testok