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

Re: [PATCH 3/3] verify_path: consider dos drive prefix

From
Johannes Sixt <j6t@kdbg.org>
Date
May 30, 2011, 20:23 UTC
Message-ID
<4DE3FCB9.1010401@kdbg.org>
In-Reply-To
<BANLkTikdeq7cuhi0uo7Q6wqDJK3nxjmP-g@mail.gmail.com>
Am 30.05.2011 11:32, schrieb Erik Faye-Lund:
Show 41 quoted lines
> On Fri, May 27, 2011 at 8:58 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Am 27.05.2011 18:00, schrieb Erik Faye-Lund:
>>> If someone manage to create a repo with a 'C:' entry in the
>>> root-tree, files can be written outside of the working-dir. This
>>> opens up a can-of-worms of exploits.
>>>
>>> Fix it by explicitly checking for a dos drive prefix when verifying
>>> a paht. While we're at it, make sure that paths beginning with '\' is
>>> considered absolute as well.
>>
>> I think we do agree that the only way to avoid the security breach is to
>> check a path before it is used to write a file. In practice, it means to
>> disallow paths in the top-most level of the index that are two
>> characters long and are letter-colon.
>>
>> IMHO, it is pointless to avoid that an evil path enters the repository,
>> because there are so many and a few more ways to create an evil repository.
>>
> 
> Yes, but this patch doesn't prevent that; it prevents an evil path
> from entering the index and from being checked out if the index is
> evil.
> 
>>> diff --git a/read-cache.c b/read-cache.c
>>> index f38471c..68faa51 100644
>>> --- a/read-cache.c
>>> +++ b/read-cache.c
>>> @@ -753,11 +753,14 @@ int verify_path(const char *path)
>>>  {
>>>       char c;
>>>
>>> +     if (has_dos_drive_prefix(path))
>>> +             return 0;
>>> +
>>
>> Isn't verify_path used to avoid that a bogus path enters the index? (I
>> don't know, I'm not familiar with this infrastructure.)
>>
> 
> Yes, it's being used to do that. But it's also being used when reading
> the index into memory, which is "the good stuf" for our purposes.

OK, I agree with the changes proposed in this patch. git reset and git checkout go through this function via unpack_trees(). Are there other ways to write a file, e.g., in merge-recursive?

-- Hannes
Previous: Theo NiessinkNext: Junio C Hamano
Message 15 of 20 in “do not write files outside of work-dir”
  1. 0/3 do not write files outside of work-dirErik Faye-Lund, May 27, 2011
  2. 1/3 A Windows path starting with a backslash is absoluteErik Faye-Lund, May 27, 2011
  3. 2/3 real_path: do not assume '/' is the path seperatorErik Faye-Lund, May 27, 2011
  4. 3/3 verify_path: consider dos drive prefixErik Faye-Lund, May 27, 2011
  5. Johannes SixtMay 27, 2011
  6. Erik Faye-LundMay 30, 2011
  7. Theo NiessinkMay 30, 2011
  8. Erik Faye-LundMay 30, 2011
  9. Junio C HamanoJun 7, 2011
  10. Erik Faye-LundJun 7, 2011
  11. Erik Faye-LundJun 7, 2011
  12. Junio C HamanoJun 7, 2011
  13. Erik Faye-LundJun 7, 2011
  14. Theo NiessinkJun 7, 2011
  15. Johannes SixtMay 30, 2011
  16. Junio C HamanoMay 27, 2011
  17. Johannes SchindelinMay 27, 2011
  18. Junio C HamanoMay 27, 2011
  19. TaitJun 1, 2011
  20. Johannes SixtJun 1, 2011

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.