Re: [PATCH] git-daemon extra paranoia
- From
- H. Peter Anvin <hpa@zytor.com>
- Date
- Oct 19, 2005, 00:43 UTC
- Message-ID
- <435596CB.6070401@zytor.com>
- In-Reply-To
- <Pine.LNX.4.64.0510181728490.3369@g5.osdl.org>
Linus Torvalds wrote:
Show 20 quoted lines
> And just appending ".git" is _not_ badly designed/specified. I did think > about the boundary cases, and it's entirely safe: > > - it can't result in "surprises": if the original pathname doesn't exist, > then even if there is a race and it got created in between the two > chdir's as a directory and the name had a slash at the end, adding > ".git" is actually safe even if it succeeds: it won't take us anywhere > surprising. At worst it will take us to the ".git" directory of a newly > added git archive, but that's what we wanted anyway, so.. > > - you can't create ".." with it - even if the passed-in filename ended > with "xyz/.", you'll end up with a perfectly safe "xyz/..git", so any > safety checks that were done on the original pathname are still valid > when appending ".git" to it. > > - and exactly because we don't append slashes or anything like that, the > end result won't even have anything ambiguous like "//" in it. > > So it really doesn't have any downsides that I can see. >
Consider the whitelist/blacklist scenario I described in the previous email. You have:
whitelist: /pub/scm blacklist: /pub/scm/foo/bar.git
If you can bypass the blacklist by using the pathname /pub/scm/foo/bar, that's bad.
Show 10 quoted lines
> >>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. > > Sure, you could do that, and just replace path_ok + chdir with a > "safe_chdir()". I don't really see the point, unless you want to walk the > path one component at a time, though (which is really quite expensive). >
The only reason to do that is to make it less likely that a future programmer would screw it up.
-hpa