[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH 3/3] verify_path: consider dos drive prefix
From:       Johannes Sixt <j6t () kdbg ! org>
Date:       2011-05-30 20:23:21
Message-ID: 4DE3FCB9.1010401 () kdbg ! org
[Download RAW message or body]

Am 30.05.2011 11:32, schrieb Erik Faye-Lund:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]