[Geany-Devel] Escaping replacement for placeholder in build commands
lists.ban at herbesfolles.org
Sat Dec 15 13:52:37 UTC 2012
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:
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:
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
argv = gcc
argv = holy crap.c -c -o holy
argv = crap.o
(if my shell unquoting skills are correct ;))
What the user actually expected was:
argv = gcc
argv = holy "crap.c
argv = -c
argv = -o
argv = 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
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 :)
More information about the Devel