potential issue re in memory django file uploading.
|Reported by:||db||Owned by:||nobody|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Description (last modified by russellm)
As per my original email to the django users mailing list:
Ok so I was looking through the code and I saw this (in
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
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 ..."
And the various follow ups, I propose the following fix:
--- 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
There is a second problem not fixed by this and that is that there is no bound of temporary uploaded files.
As I understand it an attacker can abuse gzip user requests, if
mod_deflate is enabled (AND configured to decompress incoming user
requests - this is not the default) in apache2 with a user gziped
So an attack could do effectively have a file like this:
f = open("rar", "w") string = "" for i in range(0, 10000000): string += " " + "1" f.write(string) f.close()
ls -lah 20M 2010-08-29 17:15 rar
(except replace write with append and do it a lot more ;) ) and then
send it gziped as in the request body.
Just for fun ;)
ls -lah 19K 2010-08-29 17:15 rar.gz
So django will receive the original 20M file (as the httpd has
uncompressed it for django ) afaik.
see Input Decompression at http://httpd.apache.org/docs/2.0/mod/mod_deflate.html"
So depending on the httpd and its configuration, this attack(s) are very possible.