[PATCH 2 of 3 V6] hgweb: teach archive how to download a specific directory or file

Angel Ezquerra angel.ezquerra at gmail.com
Tue Mar 12 17:45:58 CDT 2013


On Tue, Mar 12, 2013 at 11:10 PM, Angel Ezquerra
<angel.ezquerra at gmail.com> wrote:
> On Tue, Mar 12, 2013 at 11:08 PM, Kevin Bullock
> <kbullock+mercurial at ringworld.org> wrote:
>> On 12 Mar 2013, at 4:47 PM, Angel Ezquerra wrote:
>>
>>> On Tue, Mar 12, 2013 at 6:29 PM, Pierre-Yves David
>>> <pierre-yves.david at logilab.fr> wrote:
>>>> On Wed, Feb 27, 2013 at 06:06:33PM +0100, Angel Ezquerra wrote:
>>>>> +        if ':' in file:
>>>>> +            msg = 'Archive pattern not allowed: %s' % file
>>>>> +            raise ErrorResponse(HTTP_FORBIDDEN, msg)
>>>>> +        pats = ['path:' + file]
>>>>
>>>> What about file with ":" is there name ?
>>>
>>> I'd say that a user that created such a file deserves to not be able
>>> to download it ;-)
>>>
>>> Jokes aside, the alternative is to explicitly test for the different
>>> patterns (e.g. relpath:, relre:, etc). The problem with that is that
>>> if we ever introduce a new pattern we may forget to update this filter
>>> and then let a malicious user use such a pattern.
>>>
>>> So I don't know what is the best solution to this... Suggestions?
>>
>> If you unconditionally prefix the filename with 'path:', it will be interpreted as a path (even if it contains a ':' later in the name). Then you'd presumably fall back on a 'file not found' error instead of the 403 Forbidden, which might even be preferable from a security standpoint.
>>
>
> OK, I will do as you suggest. Thanks!
>
> Angel

Ummm, actually that does not work that well because the server will
simply return an empty archive file.

In fact I was already unconditionally prepending 'path:' to the
requested file so I was wrong and even if in the future we add a new
pattern type that we forget to filter out there is no risk of a
malicious user doing something evil (he will just get an empty
archive).

For the rest of users I think it would be best to keep the 403 error
because it explains what is wrong explicitly.

So what if I just filter out the known patterns?

Angel


More information about the Mercurial-devel mailing list