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

[Lex Trotman]

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

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

>
> So, how to fix it?

Don't, its not broke :)

>
> What comes to mind immediately is to escape the placeholder

Do you mean escape or quote above, the rest of the sentence seems to
suggest quoting not escaping.

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

As I said above, you can't know what is meant, sure the "usual" case
may be an error, but you can't tell.

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

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.

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

I suspect it will break anything but the "usual" commands, and thats bad :(

Cheers
Lex

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

How do you know its a literal?  Ok, in this case it "probably" is, but still ...

>
> _______________________________________________
> Devel mailing list
> Devel at lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
>