Discussion:
Definition of command line in the Exec key
Роман Чистоходов
2017-03-20 02:09:19 UTC
Permalink
The Exec key description (
https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s06.html )
is somewhat misleading.
It does not state what is "command line" exactly. Should it be considered
shell command line or something that can be called via execve.
Giving that desktop-file-validate does not like unquoted backticks I would
say the latter, but all this stuff about quotation rules makes me think
about the former.
This is probably due the legacy code that makes behavior of Exec similar to
the one if this command line was to be run in shell.

Spec should clearly say about such things. I was not confident at all while
was implementing this spec by myself and honestly still doubt that I get
things right.
Simon McVittie
2017-03-23 12:24:56 UTC
Permalink
Should it be considered shell command line
No, the content of Exec= is not a one-line shell script. If you need
to use shell constructs like if...fi, ``, ${} then you must invoke a
shell yourself:

Exec=sh -c "if foo; then bar; else baz; fi"
or something that can be called via execve.
You can't directly pass the value of the Exec key to execve, because
it's a single string and execve wants an array of strings, but it's close.

To parse an Exec value, you must unquote it according to the generic
.desktop file syntax, then break it into an array at unquoted whitespace,
then undo shell-style quoting within each entry of that array.
The desktop entry spec does describe this, although from the perspective
of a .desktop file author (there are many of those) rather than from the
perspective of an implementor (there are only a few of those).
GLib's g_shell_parse_argv() is one common C implementation of those
operations; there is another in dbus-daemon.

(For a general implementation you also need to expand *field codes* like %f,
some of which expand to more than one argument in the output argv. In GLib
this is done in a different layer - see GDesktopAppInfo for an implementation.)
Giving that desktop-file-validate does not like unquoted backticks I would say
the latter, but all this stuff about quotation rules makes me think about the
former.
This is probably due the legacy code that makes behavior of Exec similar to the
one if this command line was to be run in shell. 
It is essentially a subset of /bin/sh syntax. I believe the intention
is that every Exec value that is valid according to the Desktop Entry
Specification has the same meaning in /bin/sh and in a spec-compliant
.desktop file parser. However, many Exec values are not considered valid,
and would produce different results from g_shell_parse_argv() and a full
/bin/sh shell; it is probably not considered to be a bug in GLib's
g_shell_parse_argv(), KDE's equivalent, your equivalent, etc. if they
produce different results for an invalid .desktop file.

S
Роман Чистоходов
2017-03-23 12:49:13 UTC
Permalink
Of course I understand that Exec can't be directly passed to execve. What I
meant is it to do unescaping, unquoting and expanding field codes first and
then pass the resulting array to execve.
I just want spec to be a bit more clear on reasons behind all this
"reserved characters" stuff. It would be less confusing that way.
Whatever intentions the spec authors had when they added these rules, they
should be included in spec too. It's too hard to get it right if
implementer does not understand the reasons of why things're done the way
they're done. It would be also beneficial to have more examples like
"string as written in desktop file" -> "unescaped string" -> "array of
unquoted strings" -> "array of unquoted strings with expanded field codes"
so implementers be sure that they get all these rules correctly.
Post by Simon McVittie
Should it be considered shell command line
No, the content of Exec= is not a one-line shell script. If you need
to use shell constructs like if...fi, ``, ${} then you must invoke a
Exec=sh -c "if foo; then bar; else baz; fi"
or something that can be called via execve.
You can't directly pass the value of the Exec key to execve, because
it's a single string and execve wants an array of strings, but it's close.
To parse an Exec value, you must unquote it according to the generic
.desktop file syntax, then break it into an array at unquoted whitespace,
then undo shell-style quoting within each entry of that array.
The desktop entry spec does describe this, although from the perspective
of a .desktop file author (there are many of those) rather than from the
perspective of an implementor (there are only a few of those).
GLib's g_shell_parse_argv() is one common C implementation of those
operations; there is another in dbus-daemon.
(For a general implementation you also need to expand *field codes* like %f,
some of which expand to more than one argument in the output argv. In GLib
this is done in a different layer - see GDesktopAppInfo for an
implementation.)
Giving that desktop-file-validate does not like unquoted backticks I
would say
the latter, but all this stuff about quotation rules makes me think
about the
former.
This is probably due the legacy code that makes behavior of Exec similar
to the
one if this command line was to be run in shell.
It is essentially a subset of /bin/sh syntax. I believe the intention
is that every Exec value that is valid according to the Desktop Entry
Specification has the same meaning in /bin/sh and in a spec-compliant
.desktop file parser. However, many Exec values are not considered valid,
and would produce different results from g_shell_parse_argv() and a full
/bin/sh shell; it is probably not considered to be a bug in GLib's
g_shell_parse_argv(), KDE's equivalent, your equivalent, etc. if they
produce different results for an invalid .desktop file.
S
_______________________________________________
xdg mailing list
https://lists.freedesktop.org/mailman/listinfo/xdg
Oswald Buddenhagen
2017-03-28 08:51:05 UTC
Permalink
Post by Simon McVittie
Should it be considered shell command line
No, the content of Exec= is not a one-line shell script.
actually, it is in a way, as you're saying yourself further down.
and in kde, it is even in fact - the evaluator is smart enough to use an
actual shell if the command gets too complex. but relying on that is
obviously unportable.
Post by Simon McVittie
If you need to use shell constructs like if...fi, ``, ${} then you
Exec=sh -c "if foo; then bar; else baz; fi"
this is actually horrible advice, as it is pretty much guaranteed to
overtax the automatic quoting of expandos inside the nested command.
if you need a compound command, ship an _actual_ script and have the
.desktop file invoke it.
Post by Simon McVittie
It is essentially a subset of /bin/sh syntax. I believe the intention
is that every Exec value that is valid according to the Desktop Entry
Specification has the same meaning in /bin/sh and in a spec-compliant
.desktop file parser.
indeed, which makes it a "shell script", albeit a rather limited one.
Post by Simon McVittie
However, many Exec values are not considered valid, and would produce
different results from g_shell_parse_argv() and a full /bin/sh shell;
it is probably not considered to be a bug in GLib's
g_shell_parse_argv(), KDE's equivalent, your equivalent, etc. if they
produce different results for an invalid .desktop file.
i tend to agree with that, though i find it unfortunate that the shared
subset which is considered valid is comparably small.

fwiw, the kde implementation can be found in
https://code.woboq.org/qt5/kf5/kio/src/core/desktopexecparser.cpp.html

the underlying magic happens in
https://code.woboq.org/qt5/kf5/kcoreaddons/src/lib/util/kshell_unix.cpp.html
https://code.woboq.org/qt5/kf5/kcoreaddons/src/lib/text/kmacroexpander_unix.cpp.html

and there is an evolved version of the latter, which i shamelessly stole
from myself in
https://code.woboq.org/qt5/qt-creator/src/libs/utils/qtcprocess.cpp.html
(i still didn't manage to upstream it into qt proper ...)
Simon McVittie
2017-03-28 17:45:58 UTC
Permalink
Post by Oswald Buddenhagen
Post by Simon McVittie
No, the content of Exec= is not a one-line shell script.
actually, it is in a way, as you're saying yourself further down.
and in kde, it is even in fact - the evaluator is smart enough to use an
actual shell if the command gets too complex. but relying on that is
obviously unportable.
If you run it in an actual shell, do you filter/reject more complex
syntax in some way, so that there's some hope that desktop files written
by KDE users will be portable to other environments?
Post by Oswald Buddenhagen
Post by Simon McVittie
If you need to use shell constructs like if...fi, ``, ${} then you
Exec=sh -c "if foo; then bar; else baz; fi"
this is actually horrible advice, as it is pretty much guaranteed to
overtax the automatic quoting of expandos inside the nested command.
If by "expando" you mean "field code" in the desktop entry spec's jargon
(%f and so on), that's fine: field code expansion inside a quoted argument
is undefined behaviour according to the desktop entry spec anyway, so
implementations are not required to do something sensible when they
encounter it.

(GLib's implementation substitutes field codes (wrapped in shell-style
quoting) before doing shell-style parsing, so it is possible to get a
"garbage in, garbage out" situation.)
Post by Oswald Buddenhagen
if you need a compound command, ship an _actual_ script and have the
.desktop file invoke it.
This is good advice, though. Or preferably, write your logic in something
with fewer sharp edges than shell script :-)
Post by Oswald Buddenhagen
i find it unfortunate that the shared
subset which is considered valid is comparably small.
I think that's a feature more than a bug, actually: it means implementors
are allowed to avoid using an actual shell (the implementation in GLib
never does, and still isn't particularly large). "Don't ever run a shell"
is a good way to be confident that you've avoided shell injection
attacks.

S
Oswald Buddenhagen
2017-03-30 16:06:01 UTC
Permalink
Post by Simon McVittie
Post by Oswald Buddenhagen
and in kde, it is even in fact - the evaluator is smart enough to use an
actual shell if the command gets too complex. but relying on that is
obviously unportable.
If you run it in an actual shell, do you filter/reject more complex
syntax in some way, so that there's some hope that desktop files written
by KDE users will be portable to other environments?
that would presume an enviroment which is even more thorough than kde
in this regard (something that has historically not happened for all i
know), and yet we'd need to know the exact limits of said environment
(which means we'd have to implement at least the parser side of it).
this obviously makes no sense to me.
Post by Simon McVittie
Post by Oswald Buddenhagen
Post by Simon McVittie
If you need to use shell constructs like if...fi, ``, ${} then you
Exec=sh -c "if foo; then bar; else baz; fi"
this is actually horrible advice, as it is pretty much guaranteed to
overtax the automatic quoting of expandos inside the nested command.
If by "expando" you mean "field code" in the desktop entry spec's jargon
(%f and so on), that's fine: field code expansion inside a quoted argument
is undefined behaviour according to the desktop entry spec anyway, so
implementations are not required to do something sensible when they
encounter it.
one of the unfortunate things about the standard, because kde's
implementation exhibits perfectly guaranteed and safe behavior.
Post by Simon McVittie
(GLib's implementation substitutes field codes (wrapped in shell-style
quoting) before doing shell-style parsing, so it is possible to get a
"garbage in, garbage out" situation.)
this is in effect comparable with kde's implementation, except that for
kde a smaller subset of possible inputs is garbage.
Post by Simon McVittie
Post by Oswald Buddenhagen
i find it unfortunate that the shared
subset which is considered valid is comparably small.
I think that's a feature more than a bug, actually: it means implementors
are allowed to avoid using an actual shell (the implementation in GLib
never does, and still isn't particularly large).
"Don't ever run a shell" is a good way to be confident that you've
avoided shell injection attacks.
this is a non-argument unless you assume incompetence on the
implementor's side (which i guess would be mostly reasonable ;).
i for one will make the claim that my expander is safe for the
documented supported syntax. and outside it ... well, the user can
always just use sh -c to subvert any quoting guarantees.

Loading...