On 28 August 2010 23:21, dave b <db.pub.m...@gmail.com> wrote:
> On 28 August 2010 23:09, dave b <db.pub.m...@gmail.com> wrote:
>> On 28 August 2010 22:46, Graham Dumpleton <graham.dumple...@gmail.com> wrote:
>>>
>>>
>>> On Aug 28, 7:58 pm, "david b." <db.pub.m...@gmail.com> wrote:
>>>> Ok so I was looking through the code and I saw this (in
>>>> django/core/files/uploadhandler.py) :
>>>>
>>>> FileUploadHandler
>>>> ...
>>>>
>>>>    def new_file(self, field_name, file_name, content_type,
>>>> content_length, charset=None):
>>>>        """
>>>>        Signal that a new file has been started.
>>>>
>>>>        Warning: As with any data from the client, you should not trust
>>>>        content_length (and sometimes won't even get it).
>>>>        """
>>>>
>>>> So the content_length we control right? - Maybe I missed something but
>>>> ... I can say I want to upload a small file then upload a file that
>>>> triggers an oom condition / use a lot of memory no ? ...
>>>>
>>>> And then this.
>>>>
>>>> class MemoryFileUploadHandler(FileUploadHandler):
>>>>    """
>>>>    File upload handler to stream uploads into memory (used for small
>>>> files).
>>>>    """
>>>>
>>>>    def handle_raw_input(self, input_data, META, content_length,
>>>> boundary, encoding=None):
>>>>        """
>>>>        Use the content_length to signal whether or not this handler
>>>> should be in use.
>>>>        """
>>>>        # Check the content-length header to see if we should
>>>>        # If the post is too large, we cannot use the Memory handler.
>>>>        if content_length > settings.FILE_UPLOAD_MAX_MEMORY_SIZE:
>>>>            self.activated = False
>>>>        else:
>>>>            self.activated = True
>>>>
>>>>    def new_file(self, *args, **kwargs):
>>>>        super(MemoryFileUploadHandler, self).new_file(*args, **kwargs)
>>>>        if self.activated:
>>>>            self.file = StringIO()
>>>>            raise StopFutureHandlers()
>>>>
>>>>    def receive_data_chunk(self, raw_data, start):
>>>>        """
>>>>        Add the data to the StringIO file.
>>>>        """
>>>>        if self.activated:
>>>>            self.file.write(raw_data)
>>>>        else:
>>>>            return raw_data
>>>>
>>>>    def file_complete(self, file_size):
>>>>        """
>>>>        Return a file object if we're activated.
>>>>        """
>>>>        if not self.activated:
>>>>            return
>>>>
>>>>        self.file.seek(0)
>>>>        return InMemoryUploadedFile(
>>>>            file = self.file,
>>>>            field_name = self.field_name,
>>>>            name = self.file_name,
>>>>            content_type = self.content_type,
>>>>            size = file_size,
>>>>
>>>> There is a regression test for this  BUT --> in the test suite there
>>>> is # A small file (under the 5M quota)
>>>> which is governed by
>>>> (django/tests/regressiontests/file_uploads/uploadhandler.py)
>>>>
>>>> def receive_data_chunk(self, raw_data, start):
>>>>        self.total_upload += len(raw_data)
>>>>        if self.total_upload >= self.QUOTA:
>>>>           raise StopUpload(connection_reset=True)
>>>>        return raw_data
>>>>
>>>> So obviously my proposed attack is to simply say "content length is
>>>> tiny" and "this file is actually HUGE".
>>>> I hope I missed something :) I don't really want this to occur ...
>>>
>>> A decent web server such as Apache (under mod_wsgi) will stop reading
>>> the original content at the content length specified in the request.
>>> Thus not possible to force more than content length down to the
>>> application level.
>>>
>>> Graham
>>
>> The documentation and code  in django suggests that this is not the
>> case. So lets assume we are not using apache but another httpd of some
>> sort - then this problem will be present.
>> http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4 seems to
>> say otherwise from my reading.
>
> Just to clarify this - I meant that the http content length header
> item is *not* required - as per
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4 (also see
> 4.4.2), so I do not believe that apache would do what you said :) -
> there is a default limit in a apache of around 2gb for the attackers
> file to reach though.
>

Woop I hit send a bit early, I meant to also include:
So now the attack can now just not specify the content length and um...

>>> None < 2621440
True


We pass the check ;) as far as I know. Look if I really have missed
something do tell me :)

-- 
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-us...@googlegroups.com.
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en.

Reply via email to