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

[valentin.haenel at gmx.de (Valentin Haenel)]

* Ben Boeckel <mathstuf at gmail.com> [2012-10-29]:
> 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).

Yeah, better safe than sorry. :-D

V-