[PATCHv2 2/3] Add ability to authorize viewing a repository

[mathstuf at gmail.com (Ben Boeckel)]

On Sun, Oct 28, 2012 at 01:16:36 GMT, Jason A. Donenfeld wrote:
> On Sat, Oct 27, 2012 at 7:00 PM, Ben Boeckel <mathstuf at gmail.com> wrote:
>> Single quote the arguments to the executable. This is ripe for code
>> execution (remote_user is under attacker's control).
>
> Was going to mention this myself, but you beat me too it. Dead on.
> Correctamundo.
>
> Please double double tripe triple check your code before submitting things.

Working on the uzbl shell scripts (where we require POSIX compatibility
as well) has made these problems stick out like sore thumbs :) . Alas,
secure shell coding isn't the default mode for many programmers :( .
I'll look at the other shell code in cgit this weekend.

> While we're on the topic, is "system" the best way to be calling this?
> Since an auth helper gets called for each and every request, it seems
> like it'd be cleanest if we could just fork/exec/wait ourselves,
> passing the options in to execv. This way we don't have to fire up a
> shell interpreter each time.

Well, that would require that we either shell-split the option (messy
and error prone in and of itself) or effectively require trampoline
scripts for everything due to enforced arguments. Though...something
taking these arguments naturally is unlikely. +1 for fork/exec here.

--Ben