- deny more potentially malicious requests with BadRequest (#1247)

CHANGES.rst

@@ -13,7 +13,7 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 - Enable ZMI History tab for ``OFS.Image.File``.
   (`#396 <https://github.com/zopefoundation/Zope/pull/396>`_)
 
-- Fix requests from spam/pentests to return BadRequest/400 errors
+- Deny some spam/pentests requests with BadRequest/400 or NotFound/404 errors.
 
 - Fix a ``ResourceWarning`` emitted when uploading large files.
   (`#1242 <https://github.com/zopefoundation/Zope/issues/1242>`_)

src/ZPublisher/HTTPRequest.py

@@ -29,6 +29,7 @@
 from AccessControl.tainted import should_be_tainted as base_should_be_tainted
 from AccessControl.tainted import taint_string
 from multipart import Headers
+from multipart import MultipartError
 from multipart import MultipartParser
 from multipart import parse_options_header
 from zExceptions import BadRequest
@@ -499,7 +500,10 @@ def processInputs(
 
         meth = None
 
-        self._fs = fs = ZopeFieldStorage(fp, environ)
+        try:
+            self._fs = fs = ZopeFieldStorage(fp, environ)
+        except MultipartError as exc:
+            raise BadRequest(exc)
         self._file = fs.file
 
         if 'HTTP_SOAPACTION' in environ:
@@ -545,8 +549,15 @@ def processInputs(
                 if key is None:
                     continue
                 character_encoding = ""
-                key = item.name.encode("latin-1").decode(
-                    item.name_charset or self.charset)
+
+                try:
+                    key = item.name.encode("latin-1").decode(
+                        item.name_charset or self.charset)
+                except UnicodeDecodeError as exc:
+                    # Incoming request contains fields with names that cannot
+                    # be represented in the server character set. Potentially
+                    # malicious, so raise BadRequest.
+                    raise BadRequest(exc)
 
                 if hasattr(item, 'file') and \
                    hasattr(item, 'filename') and \

src/ZPublisher/tests/testHTTPRequest.py

@@ -844,6 +844,20 @@ def test_processInputs_xmlrpc_method(self):
         with self.assertRaises(BadRequest):
             req.processInputs()
 
+    def test_processInputs_xmlrpc_ResponseError(self):
+        # xmlrpc ResponseErrors should be caught and converted to
+        # zExceptions.BadRequest to prevent error messages from spam or pen
+        # test requests.
+        TEST_METHOD_CALL = (
+            b'<?xml version="1.0"?>'
+            b'<unknown><foo>test</foo></unknown>'
+        )
+        environ = self._makePostEnviron(body=TEST_METHOD_CALL)
+        environ['CONTENT_TYPE'] = 'text/xml'
+        req = self._makeOne(stdin=BytesIO(TEST_METHOD_CALL), environ=environ)
+        with self.assertRaises(BadRequest):
+            req.processInputs()
+
     def test_processInputs_SOAP(self):
         # ZPublisher does not really have SOAP support
         # all it does is put the body into ``SOAPXML``
@@ -968,6 +982,22 @@ def test_processInputs_unspecified_file(self):
         self.assertEqual(f.filename, "")
         self.assertEqual(list(f), [])
 
+    def test_processInputs_no_multipart_boundaries(self):
+        s = BytesIO(TEST_NO_BOUNDARIES)
+        environ = self._makePostEnviron(body=TEST_NO_BOUNDARIES)
+        req = self._makeOne(stdin=s, environ=environ)
+        with self.assertRaises(BadRequest):
+            req.processInputs()
+
+    def test_processInputs_invalid_field_name(self):
+        # Request fields with names that cannot be decoded should raise
+        # a BadRequest
+        s = BytesIO(TEST_FIELD_INVALID_NAME)
+        environ = self._makePostEnviron(body=TEST_FIELD_INVALID_NAME)
+        req = self._makeOne(stdin=s, environ=environ)
+        with self.assertRaises(BadRequest):
+            req.processInputs()
+
     def test__authUserPW_simple(self):
         user_id = 'user'
         password = 'password'
@@ -1668,3 +1698,19 @@ def __init__(self, file):
 %s
 --12345--
 ''' % 'äöü'.encode("latin-1")
+
+TEST_NO_BOUNDARIES = b'''
+Content-Disposition: form-data; name="smallfile"; filename="smallfile"
+Content-Type: application/octet-stream
+
+test
+'''
+
+TEST_FIELD_INVALID_NAME = b'''
+--12345
+Content-Disposition: form-data; name="%sxxx"
+Content-Type: text/plain; charset=latin-1
+
+test
+--12345--
+''' % chr(173).encode("latin-1")