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

[Matthew Brush]

On 12-12-14 08:34 PM, Lex Trotman wrote:
> On 15 December 2012 03:53, Colomban Wendling <lists.ban at herbesfolles.org> wrote:
>> 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.
>
> Hi Colomban,
>
> No, a reference would be useful ;).
>

It was sent directly to Geany developers, regarding:
https://bugs.gentoo.org/show_bug.cgi?id=446986

>>   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).
>
> I don't think Geany should be interpreting what a user meant by their
> command, there are too many edge cases that don't conform to the
> "usual" gcc or other rules.  That space may be meant to be there,
> Geany has no way of knowing.  Also what if the command has globs in
> it, quoting them stops them working :(
>

I think the idea is to just escape the filenames being replaced into %f 
et al placeholders, not the whole command en masse. IIRC, the only thing 
placed into the placeholders is filenames/paths, which should be able to 
be safely escaped/sanitized without messing up the whole command.

>>
>> So, how to fix it?
>
> Don't, its not broke :)
>

The concern as raised in the bug report('s link) above is that one could 
(somehow) inject malicious code in the command.

FWIW, the general consensus seemed to be that while bad things could 
happen, from a security perspective, it's no worse than using `make` or 
GCC or whatever directly with malicious filenames/misquoting. The fact 
that the user has to enter the command manually (or have $HOME hacked) 
and also explicitly initiate the command with a maliciously-named file 
open pretty much negates the _security_ concerns.

>>
>> [...]
>> So, I wrote a not-that-trivial replacement of
>> `build_replace_placeholder()` (patch attached) that takes care of the it's no worse than using `make` or GCC or whatever directly with malicious filenames. T
>> replacement quoting (using `g_shell_quote()`) and quotes in the input.
>
> I am not in a situation where I can try, and reading it doesn't make
> sense??  Can you explain what you are trying to do.
>

IIUC, the idea is to avoid replacing something like this:

   gcc -c "%f"

With the following:

   gcc -c "foo.c"; rm -rf /; cat /etc/passwd"

Where the %f placeholder gets replaced with the document's 
maliciously-named file ('foo.c"; rm -rf /; cat "/etc/passwd') without 
being escaped/sanitized.

Disclaimer: I haven't state my own opinion in this email and so 
shouldn't be disagreed or argued with about the validity of the concerns 
that were raised by others, just sharing the details FYI :)

Cheers,
Matthew Brush