Eric Blake <eblake@redhat.com> wrote on 09/24/2010 06:16:35 PM:


> On 09/24/2010 02:22 PM, Stefan Berger wrote:
> > I just tried the TCK test without and with double-escaping in libvirtd
> > and double-escaping does seem to be necessary otherwise `ls` and $(ls)
> > do get executed and their results end up in the comment. The spaces
> > are preserved, though, so I can revert the change to IFS.
>
> Hmm.
>
> > "res=`eval \"$cmd\"" CMD_SEPARATOR
>
> > +        virBufferVSprintf(buf,
> > +                          " -m comment --comment \\\"%s\\\"",
> > +                          cmt);
>
> Thinking about it more:
>
[...]
>
> My suggestion is to assign cmd using '' rather than "" (fewer things to
> quote), as well as moving the eval outside of the `` (so it becomes
> obvious which \ are interpreted by eval rather than by ``:
>
> cmd='iptables -m comment --comment '\''user  $comment'\''
> eval res=\`"$cmd"\`
> res=`iptables -m comment --comment 'user  $comment'`
>
> And the nice part of that is the implementation:
>
> virBufferVSprintf(buf, " -m comment --comment '%s'",
>    escapeSingleQuotes(user_comment));
>
> virBufferVSprintf(cmd, "cmd='%s'\nres=\\`\"$cmd\"\\`",
>    escapeSingleQuotes(buf));
>

Also I followed this. I had to write it like this here to reflect what you wrote further above:

virBufferVSprintf(buf, " -m comment --comment '\''%s'\''",
   shellEscapeString(user_comment));

and shellEscapeString() needs to escape ' as well as `, otherwise I can execute commands.

Thanks for the help.

>
> On further thought, gnulib might be doing:
>
> #define strchr rpl_strchr
>
> on platforms where strchr is broken, so using #undef strchr is too
> risky.  So I'd recommend sticking with (strchr)(a, b), which still works
> if gnulib has to replace a broken strchr.

Ok. This also works. Wished they left a note in the man pages about this...


   Stefan