]> git.ipfire.org Git - thirdparty/systemd.git/commit
core: escape ExecStart command-line received over d-bus 24408/head
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 23 Aug 2022 05:34:49 +0000 (07:34 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Wed, 24 Aug 2022 07:54:48 +0000 (09:54 +0200)
commit24536bebe0fe4b674fda3ddf960287754e3e3cfe
treeaeba3a4039714f2f517a70ae4c30ed0fd135c983
parent6a6707ce8589fb5074a33ccbe6fdbd476e8b7021
core: escape ExecStart command-line received over d-bus

When some transient unit setting is received over D-Bus, we write it it to a
transient unit file. We escape backslashes and specifiers. For most settings
this is enough, because most settings only do parsing and interpolation of
specifiers. When systemd-run is called (or something equivalent that gives us a
command strv), we write ExecStart=, but when reading it, we not only do parsing
and interpolation of specifiers, but also split on semicolons and do variable
substitution. This results in an ugly situation where the commandline is
interpolated twice, once on the caller side, and once in the manager.

I think we need to treat this as a bug: current behaviour seems to be an
accident of implementation and hard to explain in a reasonable way. If we
*were* doing specifier expansion, then it'd be somewhat reasonable so say that
"the commandline is handled the same as ExecStart=". But since we explicitly
prevent specifier expansion, we best we could say is "the commandline has some
subset of features of ExecStart=". I think this is not useful, and unexpected
by users. Since most people use use a shell to call systemd-run, one level of
variable expansion is already done on the caller side, and having to take into
account another level of expansion (with slightly different rules), creates a
big mental overhead when the commandline needs to include a dollar character or
such. Not doing any expansion is much cleaner and easier to explain or use.
Thus I think it's better to change behaviour here, even though in principle
some people could be relying on current behaviour. I think it's more likely
that nobody noticed, because people generally don't use systemd-run for
complicated commandlines.

Thus this commit adds an additional mode of escaping that prevents variable
explansion and other elements of ExecStart= syntax. I looked over all the
places where unit_escape_setting() is called, and I think that only two need to
be changed to use the new flag.

Fixes #23631.
src/core/dbus-execute.c
src/core/unit.c
src/core/unit.h