[Geany-Devel] Escaping replacement for placeholder in build commands

Colomban Wendling lists.ban at herbesfolles.org
Fri Dec 14 16:53:06 UTC 2012


Hi guys,

You probably saw the mail from Jan Lieskovsky about the "security issue"
of not escaping filenames and other placeholders in the build commands.
 Although I don't really think it's important from the security POV, I
think it would be great to fix it for users not to get weird errors if
their files actually contain some weird characters (even only spaces if
their build command misses quoting around the placeholder).

So, how to fix it?

What comes to mind immediately is to escape the placeholder
replacements.  It would work, but we need to take a little more care
than that, because the build command may or may not already have the
placeholder quoted (like gcc -c "%f" -o %e.o).

The other solution I thought about was not to build another string but
directly an `argv` vector, but it's not really doable I think because we
want to be able to replace placeholders not only in argv but also in
directory paths & friends.  And actually it doesn't really fix anything
since we don't want a placeholder to correspond to a whole argument
anyway (like int %e.o).


So, I wrote a not-that-trivial replacement of
`build_replace_placeholder()` (patch attached) that takes care of the
replacement quoting (using `g_shell_quote()`) and quotes in the input.

Apart some more testing, I had some doubts about Windows compatibility
here.  Will the windows spawn code deal correctly with the escapes?  If
not, how to escape for Windows too?

Voila, so could you test, and what do you think?


Cheers,
Colomban


PS: my patch also fixes replacing of a placeholder in an previous
replacement, e.g. if the replacement for %f contains the literal %e it
won't be replaced.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Escape-replacements-in-build-commands.patch
Type: text/x-patch
Size: 3922 bytes
Desc: not available
URL: <http://lists.geany.org/pipermail/devel/attachments/20121214/d20118bc/attachment.bin>


More information about the Devel mailing list