From: H. Peter Anvin Date: Wed, 19 Oct 2005 00:43:55 GMT Subject: Re: [PATCH] git-daemon extra paranoia Message-ID: <435596CB.6070401@zytor.com> In-Reply-To: Linus Torvalds wrote: > 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. > >>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