diff --git a/CHANGES.rst b/CHANGES.rst index 73e2613c5b..9cb289092c 100644 --- a/CHANGES.rst +++ b/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 `_) -- 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 `_) diff --git a/src/ZPublisher/HTTPRequest.py b/src/ZPublisher/HTTPRequest.py index aeff9a940c..ceb618b8d9 100644 --- a/src/ZPublisher/HTTPRequest.py +++ b/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 \ diff --git a/src/ZPublisher/tests/testHTTPRequest.py b/src/ZPublisher/tests/testHTTPRequest.py index 2581a45752..c6b1c0da1c 100644 --- a/src/ZPublisher/tests/testHTTPRequest.py +++ b/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'' + b'test' + ) + 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")