[Python-checkins] r51204 - in python/branches/release24-maint: Lib/cgi.py Lib/test/output/test_cgi Lib/test/test_cgi.py Misc/NEWS

Fri Aug 11 09:14:39 CEST 2006

Author: georg.brandl
Date: Fri Aug 11 09:14:38 2006
New Revision: 51204
Modified:
 python/branches/release24-maint/Lib/cgi.py
 python/branches/release24-maint/Lib/test/output/test_cgi
 python/branches/release24-maint/Lib/test/test_cgi.py
 python/branches/release24-maint/Misc/NEWS
Log:
Chris McDonough's patch to defend against certain DoS attacks on FieldStorage.
SF bug #1112549. (backport from rev. 51190)
Modified: python/branches/release24-maint/Lib/cgi.py
==============================================================================

--- python/branches/release24-maint/Lib/cgi.py	(original)
+++ python/branches/release24-maint/Lib/cgi.py	Fri Aug 11 09:14:38 2006
@@ -247,6 +247,10 @@
 
 XXX This should really be subsumed by FieldStorage altogether -- no
 point in having two implementations of the same parsing algorithm.
+ Also, FieldStorage protects itself better against certain DoS attacks
+ by limiting the size of the data read in one chunk. The API here
+ does not support that kind of protection. This also affects parse()
+ since it can call parse_multipart().
 
 """
 boundary = ""
@@ -695,7 +699,7 @@
 def read_lines_to_eof(self):
 """Internal: read lines until EOF."""
 while 1:
- line = self.fp.readline()
+ line = self.fp.readline(1<<16)
 if not line:
 self.done = -1
 break
@@ -706,12 +710,13 @@
 next = "--" + self.outerboundary
 last = next + "--"
 delim = ""
+ last_line_lfend = True
 while 1:
- line = self.fp.readline()
+ line = self.fp.readline(1<<16)
 if not line:
 self.done = -1
 break
- if line[:2] == "--":
+ if line[:2] == "--" and last_line_lfend:
 strippedline = line.strip()
 if strippedline == next:
 break
@@ -722,11 +727,14 @@
 if line[-2:] == "\r\n":
 delim = "\r\n"
 line = line[:-2]
+ last_line_lfend = True
 elif line[-1] == "\n":
 delim = "\n"
 line = line[:-1]
+ last_line_lfend = True
 else:
 delim = ""
+ last_line_lfend = False
 self.__write(odelim + line)
 
 def skip_lines(self):
@@ -735,18 +743,20 @@
 return
 next = "--" + self.outerboundary
 last = next + "--"
+ last_line_lfend = True
 while 1:
- line = self.fp.readline()
+ line = self.fp.readline(1<<16)
 if not line:
 self.done = -1
 break
- if line[:2] == "--":
+ if line[:2] == "--" and last_line_lfend:
 strippedline = line.strip()
 if strippedline == next:
 break
 if strippedline == last:
 self.done = 1
 break
+ last_line_lfend = line.endswith('\n')
 
 def make_file(self, binary=None):
 """Overridable: return a readable & writable file.
Modified: python/branches/release24-maint/Lib/test/output/test_cgi
==============================================================================
--- python/branches/release24-maint/Lib/test/output/test_cgi	(original)
+++ python/branches/release24-maint/Lib/test/output/test_cgi	Fri Aug 11 09:14:38 2006
@@ -38,3 +38,5 @@
 Testing log
 Testing initlog 1
 Testing log 2
+Test FieldStorage methods that use readline
+Test basic FieldStorage multipart parsing
Modified: python/branches/release24-maint/Lib/test/test_cgi.py
==============================================================================
--- python/branches/release24-maint/Lib/test/test_cgi.py	(original)
+++ python/branches/release24-maint/Lib/test/test_cgi.py	Fri Aug 11 09:14:38 2006
@@ -2,6 +2,8 @@
 import cgi
 import os
 import sys
+import tempfile
+from StringIO import StringIO
 
 class HackedSysModule:
 # The regression test will have real values in sys.argv, which
@@ -203,4 +205,71 @@
 cgi.initlog("%s", "Testing log 3")
 cgi.log("Testing log 4")
 
+ print "Test FieldStorage methods that use readline"
+ # FieldStorage uses readline, which has the capacity to read all
+ # contents of the input file into memory; we use readline's size argument
+ # to prevent that for files that do not contain any newlines in
+ # non-GET/HEAD requests
+ class TestReadlineFile:
+ def __init__(self, file):
+ self.file = file
+ self.numcalls = 0
+
+ def readline(self, size=None):
+ self.numcalls += 1
+ if size:
+ return self.file.readline(size)
+ else:
+ return self.file.readline()
+
+ def __getattr__(self, name):
+ file = self.__dict__['file']
+ a = getattr(file, name)
+ if not isinstance(a, int):
+ setattr(self, name, a)
+ return a
+
+ f = TestReadlineFile(tempfile.TemporaryFile())
+ f.write('x' * 256 * 1024)
+ f.seek(0)
+ env = {'REQUEST_METHOD':'PUT'}
+ fs = cgi.FieldStorage(fp=f, environ=env)
+ # if we're not chunking properly, readline is only called twice
+ # (by read_binary); if we are chunking properly, it will be called 5 times
+ # as long as the chunksize is 1 << 16.
+ verify(f.numcalls > 2)
+
+ print "Test basic FieldStorage multipart parsing"
+ env = {'REQUEST_METHOD':'POST', 'CONTENT_TYPE':'multipart/form-data; boundary=---------------------------721837373350705526688164684', 'CONTENT_LENGTH':'558'}
+ postdata = r"""-----------------------------721837373350705526688164684
+Content-Disposition: form-data; name="id"
+
+1234
+-----------------------------721837373350705526688164684
+Content-Disposition: form-data; name="title"
+
+
+-----------------------------721837373350705526688164684
+Content-Disposition: form-data; name="file"; filename="test.txt"
+Content-Type: text/plain
+
+Testing 123.
+
+-----------------------------721837373350705526688164684
+Content-Disposition: form-data; name="submit"
+
+ Add 
+-----------------------------721837373350705526688164684--
+"""
+ fs = cgi.FieldStorage(fp=StringIO(postdata), environ=env)
+ verify(len(fs.list) == 4)
+ expect = [{'name':'id', 'filename':None, 'value':'1234'},
+ {'name':'title', 'filename':None, 'value':''},
+ {'name':'file', 'filename':'test.txt','value':'Testing 123.\n'},
+ {'name':'submit', 'filename':None, 'value':' Add '}]
+ for x in range(len(fs.list)):
+ for k, exp in expect[x].items():
+ got = getattr(fs.list[x], k)
+ verify(got == exp)
+
 main()
Modified: python/branches/release24-maint/Misc/NEWS
==============================================================================
--- python/branches/release24-maint/Misc/NEWS	(original)
+++ python/branches/release24-maint/Misc/NEWS	Fri Aug 11 09:14:38 2006
@@ -72,6 +72,8 @@
 Library
 -------
 
+- Bug #1112549, DoS attack on cgi.FieldStorage.
+
 - Bug #1257728: Complain about missing VS 2003 in the error message
 of msvccompiler, and mention Cygwin as an alternative.
 
