diff options
author | Henri Chain <henri.chain@enioka.com> | 2021-10-05 13:10:31 +0200 |
---|---|---|
committer | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2021-10-12 17:19:03 +0200 |
commit | 7a58bf7aac8b2c812ee0531b0cc426e0067edd35 (patch) | |
tree | d99ee3960bc79c1dd56eb7e66f9a8fe32ab30780 | |
parent | 5ee578fd13809e08fbda1a9bca2256ffd24e9857 (diff) | |
download | systemd-7a58bf7aac8b2c812ee0531b0cc426e0067edd35.tar.gz |
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)
-rw-r--r-- | src/core/dbus-execute.c | 4 | ||||
-rw-r--r-- | src/core/service.c | 10 | ||||
-rwxr-xr-x | test/units/testsuite-23.sh | 31 |
3 files changed, 45 insertions, 0 deletions
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 50daef6702..902e074bd2 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1421,6 +1421,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 b7cfc04c84..e061d488c7 100644 --- 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 4ef7c878a8..5488447a87 100755 --- 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 |