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

[mathstuf at gmail.com (Ben Boeckel)]

On Mon, Oct 29, 2012 at 10:43:47 +0100, Valentin Haenel wrote:
> I added the single quotes as suggested. When I looked at the code
> initially, I was reasoning that the remote_user is set by the
> authentication part, in our case this is Apache, which in turn asks
> LDAP. Furthermore, Apache sets the remote_user and forward to cgit only
> if the user is actually a valid user. So my assumption was, that
> remote_user is not under the attackers control.
> 
> I guess I need some more help to understand why I am mistaken about
> this. Is it the case that the assumption fails, if an attacker can
> inject something into LDAP he may be able to pass through apache
> successfully and then have his exploit, which is in remote_user, be
> executed on the machine which is running cgit?

Okay, that makes it sound to me as if it makes sense to have a setting
for the variable.

We should quote it since the provenance is outside of cgit. If
preventing code exection is a couple of characters added on our side,
it'll mean we don't have to worry about bugs in apache, nginx, lighttpd,
slapd, and every other piece of software which might be involved.
Remember, we need only give attackers one hole and we're done. Even if
"that's impossible" is the gut reaction, that's a target attackers can
knowningly try to thread arbitrary strings through towards.

It also means we're guarded against obscure webserver setups which is
always a good thing (http://git.example.org/~foobar giving the user's
view being turned into a user's view is possible).

--Ben