FieldStorage : don't stream file uploads to memory

• Key: MODPYTHON-40
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Nicolas Lehuen
• Affects Version/s: 3.1.4
• Fix Version/s: 3.2.7

Description

In mod_python.py/util.py, line 169, we stream a file upload to disk only if its Content-Disposition header features a filename attribute. Otherwise, the file is streamed to memory, thus opening a potential DoS attack by uploading very large files.

We should :

1) Always stream file upload to disk
2) Define a default maximum file size which could be overridable.
3) Allow for the user to specify in which directory file uploads should be made, with a default to a temporary directory / file.

Comments

[Nicolas Lehuen]

Two threads on this issue :

http://www.modpython.org/pipermail/mod_python/2005-March/017754.html

and

http://www.modpython.org/pipermail/mod_python/2005-March/017756.html

[Nicolas Lehuen]

Fix by Barry Pearce - see the python-dev archives on GMane (sorry, the message has not been indexed by GMane yet) for an explanation of the fixes.

[Nicolas Lehuen]

The fix has a bug - see http://www.modpython.org/pipermail/mod_python/2005-November/019468.html and the python-dev mailing list (GMane archive are not up to date, sorry).

Alexis Marrero <[e-mail]> has proposed a fix, inspired from what CherryPy does. I've added a few unit tests to the mix, with the help of Jim Gallacher who found a small file that could always break the file upload system.

[Nicolas Lehuen]

OK, this time I think the file upload problem is solved for good. I've
checked-in Alexis's code, with comments. Then I've done a quick
rewrite of the multipart/form-data parser found in
FieldStorage.__init__ and read_to_boundary so that it uses a regexp
for the boundary checks, with the hope that it simplify the code a
little bit (and remove thos nasty strip() calls). I've re-ran all
tests and everything seems OK.