id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
14192	potential issue re in memory django file uploading.	db	nobody	"
As per my original email to the django users mailing list:

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

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.
Also:


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
request body.

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 ;)
gzip rar
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. "		closed	Core (Other)	1.2		invalid	security, dos		Unreviewed	1	0	0	0	0	0