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

[Colomban Wendling]

Le 15/12/2012 06:58, Matthew Brush a écrit :
> 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

Thanks Matthew.  And sorry Lex, I didn't think you might not have got it.

>>>   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 :(

Calm down Lex, I'm not trying to break your whole Geany.  Breathe.
Inspire; Expire.  Ok ;)

> 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.

That's it.  Another example like Matthew's one:  what I want to do is
only to make sure that e.g. "%f" is never replaced by something that
will be interpreted as a command.  Those placeholders (%f, %d, %e and
%p) represent filenames or paths and never user-typed commands, so they
should never be interpreted as a command.

For example, the simple command:

	gcc "%f" -c -o "%e.o"

Given the current file named:

	holy "crap.c

would currently expand to:

	gcc "holy "crap.c" -c -o "holy "crap.o"

Which, huh, doesn't mean what you want at all!  The commands will be
understood as:

argv[0] = gcc
argv[1] = holy crap.c -c -o holy
argv[2] = crap.o

(if my shell unquoting skills are correct ;))

What the user actually expected was:

argv[0] = gcc
argv[1] = holy "crap.c
argv[2] = -c
argv[3] = -o
argv[4] = holy "crap.o

Obviously, it's not the same :(

So again, what I want is to make sure the placeholders (%f & co.) are
properly escaped/quoted/whatever so they never get misinterpreted like
above.  And as I said, to actually achieve this we have to somewhat
understand the quoting in the user command because we can't escape the
same inside or outside shell quotes -- e.g. %f (without quotes) cannot
be escaped the same way as "%f" (with the quotes).

But still nothing really clever trying to understand a meaning, just
knowing the basics like '' and "" are quotes, and \ is an escape so we
can know how to escape.  Actually the implementation I proposed simply
closes any open quote before replacing a quoted placeholder, and then
re-opens it.

I.e. for the above example, it would replace as:

	gcc ""'holy "crap.c'"" -c -o ""'holy "crap.o'""

That's a bit ugly but you don't have to see it, and it's perfectly safe.
 Also, just to make sure you see what I mean, if the user command didn't
quote the %f and %e, and the filename simply contained a space:

command:		gcc %f -c -o %e.o
filename:		file name.c
current replacement:	gcc file name.c -c -o file name.o
fixed replacement:	gcc 'file name.c' -c -o 'file name'.o

You see? :)

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

Yes it is, although it's not really annoying because commands have
quotes around placeholders and filenames generally don't include "
literals.  But try compiling a file whose name contains a " literal and
you'll see it's broken ;)


Hope it's clearer now :)

Cheers,
Colomban