<br><tt><font size=2>Eric Blake <eblake@redhat.com> wrote on 09/24/2010
06:16:35 PM:<br>
<br>
<br>
> On 09/24/2010 02:22 PM, Stefan Berger wrote:<br>
> > I just tried the TCK test without and with double-escaping in
libvirtd<br>
> > and double-escaping does seem to be necessary otherwise `ls`
and $(ls)<br>
> > do get executed and their results end up in the comment. The
spaces<br>
> > are preserved, though, so I can revert the change to IFS.<br>
> <br>
> Hmm.<br>
> <br>
> > "res=`eval \"$cmd\"" CMD_SEPARATOR<br>
> <br>
> > +        virBufferVSprintf(buf,<br>
> > +                  
       " -m comment --comment \\\"%s\\\"",<br>
> > +                  
       cmt);<br>
> <br>
> Thinking about it more:<br>
> <br>
[...] <br>
> <br>
> My suggestion is to assign cmd using '' rather than "" (fewer
things to <br>
> quote), as well as moving the eval outside of the `` (so it becomes
<br>
> obvious which \ are interpreted by eval rather than by ``:<br>
> <br>
> cmd='iptables -m comment --comment '\''user  $comment'\''<br>
> eval res=\`"$cmd"\`<br>
> res=`iptables -m comment --comment 'user  $comment'`<br>
> <br>
> And the nice part of that is the implementation:<br>
> <br>
> virBufferVSprintf(buf, " -m comment --comment '%s'",<br>
>    escapeSingleQuotes(user_comment));<br>
> <br>
> virBufferVSprintf(cmd, "cmd='%s'\nres=\\`\"$cmd\"\\`",<br>
>    escapeSingleQuotes(buf));<br>
> </font></tt>
<br>
<br><tt><font size=2>Also I followed this. I had to write it like this
here to reflect what you wrote further above:</font></tt>
<br>
<br><tt><font size=2>virBufferVSprintf(buf, " -m comment --comment
'\''%s'\''",<br>
    shellEscapeString(user_comment));<br>
</font></tt>
<br><tt><font size=2>and shellEscapeString() needs to escape ' as well
as `, otherwise I can execute commands.</font></tt>
<br>
<br><tt><font size=2>Thanks for the help.</font></tt>
<br>
<br><tt><font size=2>> <br>
> On further thought, gnulib might be doing:<br>
> <br>
> #define strchr rpl_strchr<br>
> <br>
> on platforms where strchr is broken, so using #undef strchr is too
<br>
> risky.  So I'd recommend sticking with (strchr)(a, b), which
still works <br>
> if gnulib has to replace a broken strchr.</font></tt>
<br>
<br><tt><font size=2>Ok. This also works. Wished they left a note in the
man pages about this... </font></tt>
<br>
<br>
<br><tt><font size=2>   Stefan</font></tt>