git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] git-daemon extra paranoia

From
HAH. 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
Previous: Linus TorvaldsNext: Junio C Hamano
Message 11 of 12 in “git-daemon extra paranoia”
  1. git-daemon extra paranoiaH. Peter Anvin, Oct 18, 2005
  2. Junio C HamanoOct 18, 2005
  3. H. Peter AnvinOct 18, 2005
  4. H. Peter AnvinOct 18, 2005
  5. Revised - git-daemon extra paranoiaH. Peter Anvin, Oct 18, 2005
  6. Linus TorvaldsOct 18, 2005
  7. Junio C HamanoOct 18, 2005
  8. Linus TorvaldsOct 18, 2005
  9. H. Peter AnvinOct 19, 2005
  10. Linus TorvaldsOct 19, 2005
  11. H. Peter AnvinOct 19, 2005
  12. Junio C HamanoOct 19, 2005

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.