From: H. Peter Anvin <hpa@zytor.com>
Date: Wed, 19 Oct 2005 00:21:55 GMT
Subject: Re: [PATCH] git-daemon extra paranoia
Message-ID: <435591A3.7030708@zytor.com>
In-Reply-To: <Pine.LNX.4.64.0510181517280.3369@g5.osdl.org>

Linus Torvalds wrote:
> 
> Hmm. The "not ending in /" is a bad test. 
> 
> Especially in light of the fact that the git-pack protocol quite by design 
> tends to add a ".git" to the end as a fallback, so that a user that wants 
> to specify a particular directory _without_ that fallback needs to have 
> the slash at the end.
> 
> Now, git-daemon hasn't implemented that, but I think that was just a 
> mistake that grew out of it not getting a lot of testing, since it wasn't 
> used much. I personally use the "without the final .git" version quite 
> often, because it just looks so much nicer for the user.
> 
> In fact, here's a patch that makes git-daemon allow it, and thus match the 
> behaviour of the ssh transport.
> 
> The logic is simple: if the original "chdir()" fails, try another one with 
> ".git" appended. This is in _addition_ to doing the 'chdir(".git")' later, 
> so that if you have a checked-out git repository in /home/linux-2.6.git, 
> then doing a
> 

This is also exactly the kind of DWIM that tends to result in the kind 
of security holes I described earlier.

The DWIM aspect is fine, of course, but it has to be done up front: 
instead of doing just chdir(), each path should be validated through 
path_ok() before even being considered for chdir().  Perhaps the right 
thing to do is to combine the two functions.

	-hpa