File upload crash with "TemporaryFileUploadHandler object has no attribute 'file'" error

[Michal Čihař]

Unfortunately I don't have a reproducer and I'm still on 1.10 on production, but I still think it's worth reporting.

On file upload following error is raised:

Traceback:  

File "/usr/lib/python2.7/dist-packages/django/core/handlers/exception.py" in inner
  42.             response = get_response(request)

File "/usr/lib/python2.7/dist-packages/django/core/handlers/base.py" in _legacy_get_response
  249.             response = self._get_response(request)

File "/usr/lib/python2.7/dist-packages/django/core/handlers/base.py" in _get_response
  178.             response = middleware_method(request, callback, callback_args, callback_kwargs)

File "/usr/lib/python2.7/dist-packages/django/middleware/csrf.py" in process_view
  260.                     request_csrf_token = request.POST.get('csrfmiddlewaretoken', '')

File "/usr/lib/python2.7/dist-packages/django/core/handlers/wsgi.py" in _get_post
  128.             self._load_post_and_files()

File "/usr/lib/python2.7/dist-packages/django/http/request.py" in _load_post_and_files
  299.                 self._post, self._files = self.parse_file_upload(self.META, data)

File "/usr/lib/python2.7/dist-packages/django/http/request.py" in parse_file_upload
  258.         return parser.parse()

File "/usr/lib/python2.7/dist-packages/django/http/multipartparser.py" in parse
  163.                     self.handle_file_complete(old_field_name, counters)

File "/usr/lib/python2.7/dist-packages/django/http/multipartparser.py" in handle_file_complete
  301.             file_obj = handler.file_complete(counters[i])

File "/usr/lib/python2.7/dist-packages/django/core/files/uploadhandler.py" in file_complete
  153.         self.file.seek(0)

Exception Type: AttributeError at /dictionaries/kallithea/be/upload/
Exception Value: 'TemporaryFileUploadHandler' object has no attribute 'file'

[Claude Paroz]

You only included the traceback, not the error itself. Could you add the error?

[Michal Čihař]

Sorry, just added that.

It seems that TemporaryFileUploadHandler.new_file was never called, but TemporaryFileUploadHandler.file_complete was.

And the upload handlers settings is unchanged, so using defaults:

FILE_UPLOAD_HANDLERS = [
    'django.core.files.uploadhandler.MemoryFileUploadHandler',
    'django.core.files.uploadhandler.TemporaryFileUploadHandler',
]

[Michal Čihař]

Also I should probably mention that this was discovered once our project was publicly exposed at HackerOne, so it's possible that the payload has been tampered to trigger this.

[Tim Graham]

Closing as "needsinfo" until we have steps to reproduce.

[Jirka Vejrazka]

I've recently seen the same bug (Django 1.11). It was also during a security review. I managed to save some POST arguments and I guess the problem is caused by the completely invalid file and name arguments, namely:

"file": "'", "name": "'",

I'll try to create a test case tomorrow if I have time.

[Michael Brown]

I figured out how to reproduce this bug. It occurs whenever an uploaded filename ends in "/".

I have a pull request open here: ​https://github.com/django/django/pull/13035

This is caused by a blank filename after it is sanitized. Although MultiPartParser.parse() sanitizes and checks for empty filenames before handling the uploaded file, the filename can later be blank after it is sanitized in django.core.files.uploadedfile.UploadedFile._set_name.

For UploadedFile objects, the truthiness is False when filename is blank, so the MemoryFileUploadHandler returns an object that evaluates to False. In MultiPartParser.handle_file_complete, the return value from handler.file_complete() is checked for truthiness. That handler is erroneously skipped because of the blank filename and the next handler is called, even though the next handler never had new_file() called on it.

My patch adds another sanitize step for the filename in MultiPartParser.parse(), which is the same step performed in UploadedFile:

file_name = os.path.basename(file_name)

It also checks for None instead of truthiness to determine when handlers are skipped in MultiPartParser.handle_file_complete():

             file_obj = handler.file_complete(counters[i])
-            if file_obj:
+            if file_obj is not None:

I don't think any documentation updates are needed, since the docs for FileUploadHandler.file_complete() say "Handlers may also return None to indicate that the UploadedFile object should come from subsequent upload handlers." I don't know if this could be a backward compatibility problem for 3rd-party code though.

[Andrew Brown]

I worked with Michael to help track this down. In the process, I came up with this curl command that reproduces the bug. Since the multipart request parsing happens so early in the request processing, I believe this will trigger a 500 error on any django view on any deployment that uses the default set of file upload handlers.

curl http://localhost:8080/ -H "Content-Type: multipart/form-data; boundary=BoUnDaRy" --data-binary $'Content-Disposition: form-data; name="foo"; filename="foo/"\r\n\r\nfoo\r\n--BoUnDaRy\r\n' -b csrftoken=foo

(change the url to point at your local django dev server)

Once patched, this command should error out at the lack of a csrf token in the form data.

[Mariusz Felisiak]

Thanks! Great investigation.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 36db4dd:

Fixed #28132 -- Made MultiPartParser ignore filenames with trailing slash.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 45ec013:

[3.1.x] Fixed #28132 -- Made MultiPartParser ignore filenames with trailing slash.

Backport of 36db4dd937ae11c5b687c5d2e5fa3c27e4140001 from master