> I looked at the code, and this is what happens for ${run {}}. Based
> on the example I first pasted above:
>
> 12:42:35 25840 expanding: /bin/cat $local_part /etc/passwd
> 12:42:35 25840 result: /bin/cat /etc/passwd
> 12:42:35 25840 direct command:
> 12:42:35 25840 argv[0] = /bin/cat
> 12:42:35 25840 argv[1] = /etc/passwd
> 12:42:35 25840 expanding: ${run {/bin/cat $local_part /etc/passwd}}
>
> 1) In the code for the ${run command, the entire command is expanded.
> This explains how you get from line one to line two.
> 2) The code calls transport_set_up_command() with the entire
> *already*expanded* command (as a simple string), passing a flag saying
> not to expand it/args any more. If one of the args was quoted, the
> quotes still exist in the string that gets passed to the function.
> 3) The transport_set_up_command() splits the command and args by
> spaces, assigning values to a temporary argv variable. Since the
> unquoted $local_part in my example expanded to nothing, the two spaces
> between the original arg[0] and arg[2] are simply skipped.
> 4) If one of the arguments is quoted, the quotes are removed by a
> dequoting function, and what's left of that arg is put in the
> temporary argv (including the empty string).
I think that this is dangerous and security-sensitive behavior.
Because the word splitting and dequoting happens *after* the initial
variable expansion, your ${run} command is vulnerable to variables with
arbitrary values. If an attacker can inject values with spaces and/or
quotes in them, they can sidestep and defeat whatever quoting you do in
the ${run} invocation itself.
At a minimum I believe that the documentation should specifically
call this out and tell you that you should always use ${quote:...} on
basically all arguments to ${run}, especially including potentially
attacker-controlled values like $return_path and $local_part.
(I now need to do this in our Exim configuration, for example.)
I think it should also tell you that the ever-popular (but
mistaken[*]):
${run sh -c "... $something $something2 ..."}
should actually be written:
${run sh -c ${quote: ... $something $something2 ...}}
(assuming that I got the use of ${quote} right here). Unfortunately this
leaves you exposed to various issues with the values of those variables,
but that's pretty much life; I just don't see any way to get Exim to
escape the $-expansions inside the quotes in a sh-safe way (so that they
stay as one word even if they have, eg, a ' in them).
- cks
[*: the right way to do this is to write a script that takes whatever
variables as arguments, probably in ${quote}, and then does the
appropriate careful invocations of shell actions itself. This
avoids a pile of bear traps in shell command line quoting.
]
