Re: [PATCH] git-daemon extra paranoia

From: H. Peter Anvin <hpa@zytor.com>
Date: Oct 19, 2005, 00:21 UTC
Message-ID: <435591A3.7030708@zytor.com>
In-Reply-To: <Pine.LNX.4.64.0510181517280.3369@g5.osdl.org>

Linus Torvalds wrote:

Show 21 quoted lines

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