[#MODPYTHON-40] FieldStorage : don't stream file uploads to memory

mod_python

FieldStorage : don't stream file uploads to memory

Created: 26/Mar/05 06:51 AM Updated: 05/Mar/06 01:49 PM

Return to search

Component/s:

None

Affects Version/s:

3.1.4

Fix Version/s:

3.2.7

Time Tracking:

Not Specified

Resolution Date:

13/Nov/05 02:13 AM

Description

« Hide

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.

All

Comments

Work Log

Change History

Subversion Commits

Sort Order:

[ Permlink | « Hide ]

Nicolas Lehuen added a comment - 26/Mar/05 06:53 AM

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

Repository	Revision	Date	User	Message
ASF	#160259	Wed Apr 06 06:24:07 UTC 2005	nlehuen	Fix for ~~MODPYTHON-40~~ by Barry Pearce.
				Files Changed
				MODIFY /httpd/mod_python/trunk/lib/python/mod_python/util.py MODIFY /httpd/mod_python/trunk/CREDITS

[ Permlink | « Hide ]

Nicolas Lehuen added a comment - 06/Apr/05 03:25 PM

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 made changes - 06/Apr/05 03:25 PM

Field	Original Value	New Value
Status	Open [ 1 ]	Resolved [ 5 ]
Resolution		Fixed [ 1 ]
Fix Version/s		3.2.0 [ 11060 ]

[ Permlink | « Hide ]

Nicolas Lehuen added a comment - 07/Nov/05 04:41 PM

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 <amarrero@mitre.org> 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 made changes - 07/Nov/05 04:41 PM

Resolution	Fixed [ 1 ]
Status	Resolved [ 5 ]	Reopened [ 4 ]

Repository	Revision	Date	User	Message
ASF	#332073	Wed Nov 09 16:07:27 UTC 2005	jgallacher	Split Nicolas' fileupload tests into 3 separate tests as each one exercises a different aspect of FieldStorage.read_to_boundary. Changing the read_to_boundary implementation to fix one problem could result in a regression. The 3 tests are test_fileupload, test_fileupload_embedded_cr and test_fileupload_split_boundary. Take a look at test/test.py for more information. Ref ~~MODPYTHON-40~~
				Files Changed
				MODIFY /httpd/mod_python/trunk/test/test.py

Repository	Revision	Date	User	Message
ASF	#332771	Sat Nov 12 13:21:17 UTC 2005	nlehuen	- Fix for ~~MODPYTHON-40~~ - tweaked req_readlines : checking a string for zero length should not require strcmp() it to "".
				Files Changed
				MODIFY /httpd/mod_python/trunk/lib/python/mod_python/util.py MODIFY /httpd/mod_python/trunk/src/requestobject.c

[ Permlink | « Hide ]

Nicolas Lehuen added a comment - 13/Nov/05 02:13 AM

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.

Nicolas Lehuen made changes - 13/Nov/05 02:13 AM

Status	Reopened [ 4 ]	Resolved [ 5 ]
Resolution		Fixed [ 1 ]

Graham Dumpleton made changes - 05/Mar/06 01:49 PM

Status

Resolved [ 5 ]

Closed [ 6 ]