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