On 29 August 2010 13:33, Graham Dumpleton <graham.dumple...@gmail.com> wrote:
>
>
> On Aug 29, 1:17 pm, dave b <db.pub.m...@gmail.com> wrote:
>> On 29 August 2010 08:28, Steve Holden <holden...@gmail.com> wrote:
>>
>> > On 8/28/2010 6:10 PM, Graham Dumpleton wrote:
>> >> On Aug 28, 11:21 pm, dave b <db.pub.m...@gmail.com> wrote:
>> >>>>>> So obviously my proposed attack is to simply say "content length is
>> >>>>>> tiny" and "this file is actually HUGE".
>> > [...]
>> >> All up, I would suggest you are getting worked up over nothing.
>> > +1
>>
>> Yes I have :) it "works for me tm".
>> Also, you have to consider the other problem. If the file is > 2.5 mb
>> it can be put in /tmp and this has no size limits which again is going
>> to make the system slower and can be used to attack it? in either case
>> there seem to be real protections against this in django core as far
>> as I can see.
>
> Use Apache/mod_wsgi and you can say:
>
>  LimitRequestBody 1000000
>
> and Apache/mod_wsgi will give back a HTTP_REQUEST_ENTITY_TOO_LARGE
> error when it goes over that size before it even passes the request to
> Django and even before any of the request content is read by Apache.
>
> So, add the protections where most appropriate if you want to outright
> block requests with large content. If your issue is efficient handling
> of large posts, where you do want to handle them, then that is an
> issue for Django.
>
> Note that other Apache modules by which you can host Django may not
> work properly in honouring LimitRequestBody directive of Apache. The
> mod_python module for example doesn't really get it right, causing an
> exception when request content tries to be read by Django application,
> resulting in a malformed error response.

Yes I understand that both apache and some the mods have limits.
However, this doesn't stop a persistent attacker abusing these
relatively high limits.
IMHO I feel that django should be able to put a cap on the largest
size temporary file size possible. In addition, /tmp is a fine place
to store temporary files if the size may not be known.

I propose the following in psudo code:

1. start reading the file if we are receiving it (regardless of the
default 2.5 mb limit)
2. once it goes over 2.5mb / the configured default  shift to the next
available storage option.

In addition, as you are pointing out that it is really not possible
for django to handle chunked requests and the content length field
must be specified then you should have no problems with the following
patch (in either case this is safe because by default it will revert
back to the temporary file storage) :


--- django/core/files/uploadhandler.py.orig     2010-08-29 13:50:17.000000000 
+1000
+++ django/core/files/uploadhandler.py  2010-08-29 14:01:15.000000000 +1000
@@ -153,7 +153,7 @@
         """
         # 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:
+       if content_length is None or content_length >
settings.FILE_UPLOAD_MAX_MEMORY_SIZE:
             self.activated = False
         else:
             self.activated = True
@@ -170,6 +170,7 @@
         """
         if self.activated:
             self.file.write(raw_data)
+           self.file.truncate(settings.FILE_UPLOAD_MAX_MEMORY_SIZE)
         else:
             return raw_data

http://pastebin.com/5vgGMb5z

-- 
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