diff options
Diffstat (limited to 'user/py3-twisted/CVE-2020-10108-and-2020-10109.patch')
| -rw-r--r-- | user/py3-twisted/CVE-2020-10108-and-2020-10109.patch | 260 |
1 files changed, 260 insertions, 0 deletions
diff --git a/user/py3-twisted/CVE-2020-10108-and-2020-10109.patch b/user/py3-twisted/CVE-2020-10108-and-2020-10109.patch new file mode 100644 index 000000000..9a58f5584 --- /dev/null +++ b/user/py3-twisted/CVE-2020-10108-and-2020-10109.patch @@ -0,0 +1,260 @@ +From 4a7d22e490bb8ff836892cc99a1f54b85ccb0281 Mon Sep 17 00:00:00 2001 +From: Mark Williams <mrw@enotuniq.org> +Date: Sun, 16 Feb 2020 19:00:10 -0800 +Subject: [PATCH] Fix several request smuggling attacks. + +1. Requests with multiple Content-Length headers were allowed (thanks +to Jake Miller from Bishop Fox and ZeddYu Lu) and now fail with a 400; + +2. Requests with a Content-Length header and a Transfer-Encoding +header honored the first header (thanks to Jake Miller from Bishop +Fox) and now fail with a 400; + +3. Requests whose Transfer-Encoding header had a value other than +"chunked" and "identity" (thanks to ZeddYu Lu) were allowed and now fail +with a 400. +--- + src/twisted/web/http.py | 64 +++++++--- + src/twisted/web/newsfragments/9770.bugfix | 1 + + src/twisted/web/test/test_http.py | 137 ++++++++++++++++++++++ + 3 files changed, 187 insertions(+), 15 deletions(-) + create mode 100644 src/twisted/web/newsfragments/9770.bugfix + +diff --git a/src/twisted/web/http.py b/src/twisted/web/http.py +index f0fb05b4d69..06d830fe30f 100644 +--- a/src/twisted/web/http.py ++++ b/src/twisted/web/http.py +@@ -2171,6 +2171,51 @@ def _finishRequestBody(self, data): + self.allContentReceived() + self._dataBuffer.append(data) + ++ def _maybeChooseTransferDecoder(self, header, data): ++ """ ++ If the provided header is C{content-length} or ++ C{transfer-encoding}, choose the appropriate decoder if any. ++ ++ Returns L{True} if the request can proceed and L{False} if not. ++ """ ++ ++ def fail(): ++ self._respondToBadRequestAndDisconnect() ++ self.length = None ++ ++ # Can this header determine the length? ++ if header == b'content-length': ++ try: ++ length = int(data) ++ except ValueError: ++ fail() ++ return False ++ newTransferDecoder = _IdentityTransferDecoder( ++ length, self.requests[-1].handleContentChunk, self._finishRequestBody) ++ elif header == b'transfer-encoding': ++ # XXX Rather poorly tested code block, apparently only exercised by ++ # test_chunkedEncoding ++ if data.lower() == b'chunked': ++ length = None ++ newTransferDecoder = _ChunkedTransferDecoder( ++ self.requests[-1].handleContentChunk, self._finishRequestBody) ++ elif data.lower() == b'identity': ++ return True ++ else: ++ fail() ++ return False ++ else: ++ # It's not a length related header, so exit ++ return True ++ ++ if self._transferDecoder is not None: ++ fail() ++ return False ++ else: ++ self.length = length ++ self._transferDecoder = newTransferDecoder ++ return True ++ + + def headerReceived(self, line): + """ +@@ -2196,21 +2241,10 @@ def headerReceived(self, line): + + header = header.lower() + data = data.strip() +- if header == b'content-length': +- try: +- self.length = int(data) +- except ValueError: +- self._respondToBadRequestAndDisconnect() +- self.length = None +- return False +- self._transferDecoder = _IdentityTransferDecoder( +- self.length, self.requests[-1].handleContentChunk, self._finishRequestBody) +- elif header == b'transfer-encoding' and data.lower() == b'chunked': +- # XXX Rather poorly tested code block, apparently only exercised by +- # test_chunkedEncoding +- self.length = None +- self._transferDecoder = _ChunkedTransferDecoder( +- self.requests[-1].handleContentChunk, self._finishRequestBody) ++ ++ if not self._maybeChooseTransferDecoder(header, data): ++ return False ++ + reqHeaders = self.requests[-1].requestHeaders + values = reqHeaders.getRawHeaders(header) + if values is not None: +diff --git a/src/twisted/web/newsfragments/9770.bugfix b/src/twisted/web/newsfragments/9770.bugfix +new file mode 100644 +index 00000000000..4f1be97de8a +--- /dev/null ++++ b/src/twisted/web/newsfragments/9770.bugfix +@@ -0,0 +1 @@ ++Fix several request smuggling attacks: requests with multiple Content-Length headers were allowed (thanks to Jake Miller from Bishop Fox and ZeddYu Lu) and now fail with a 400; requests with a Content-Length header and a Transfer-Encoding header honored the first header (thanks to Jake Miller from Bishop Fox) and now fail with a 400; requests whose Transfer-Encoding header had a value other than "chunked" and "identity" (thanks to ZeddYu Lu) were allowed and now fail a 400. +\ No newline at end of file +diff --git a/src/twisted/web/test/test_http.py b/src/twisted/web/test/test_http.py +index 0a0db09b750..578cb500cda 100644 +--- a/src/twisted/web/test/test_http.py ++++ b/src/twisted/web/test/test_http.py +@@ -2252,6 +2252,143 @@ def process(self): + self.flushLoggedErrors(AttributeError) + + ++ def assertDisconnectingBadRequest(self, request): ++ """ ++ Assert that the given request bytes fail with a 400 bad ++ request without calling L{Request.process}. ++ ++ @param request: A raw HTTP request ++ @type request: L{bytes} ++ """ ++ class FailedRequest(http.Request): ++ processed = False ++ def process(self): ++ FailedRequest.processed = True ++ ++ channel = self.runRequest(request, FailedRequest, success=False) ++ self.assertFalse(FailedRequest.processed, "Request.process called") ++ self.assertEqual( ++ channel.transport.value(), ++ b"HTTP/1.1 400 Bad Request\r\n\r\n") ++ self.assertTrue(channel.transport.disconnecting) ++ ++ ++ def test_duplicateContentLengths(self): ++ """ ++ A request which includes multiple C{content-length} headers ++ fails with a 400 response without calling L{Request.process}. ++ """ ++ self.assertRequestRejected([ ++ b'GET /a HTTP/1.1', ++ b'Content-Length: 56', ++ b'Content-Length: 0', ++ b'Host: host.invalid', ++ b'', ++ b'', ++ ]) ++ ++ ++ def test_duplicateContentLengthsWithPipelinedRequests(self): ++ """ ++ Two pipelined requests, the first of which includes multiple ++ C{content-length} headers, trigger a 400 response without ++ calling L{Request.process}. ++ """ ++ self.assertRequestRejected([ ++ b'GET /a HTTP/1.1', ++ b'Content-Length: 56', ++ b'Content-Length: 0', ++ b'Host: host.invalid', ++ b'', ++ b'', ++ b'GET /a HTTP/1.1', ++ b'Host: host.invalid', ++ b'', ++ b'', ++ ]) ++ ++ ++ def test_contentLengthAndTransferEncoding(self): ++ """ ++ A request that includes both C{content-length} and ++ C{transfer-encoding} headers fails with a 400 response without ++ calling L{Request.process}. ++ """ ++ self.assertRequestRejected([ ++ b'GET /a HTTP/1.1', ++ b'Transfer-Encoding: chunked', ++ b'Content-Length: 0', ++ b'Host: host.invalid', ++ b'', ++ b'', ++ ]) ++ ++ ++ def test_contentLengthAndTransferEncodingWithPipelinedRequests(self): ++ """ ++ Two pipelined requests, the first of which includes both ++ C{content-length} and C{transfer-encoding} headers, triggers a ++ 400 response without calling L{Request.process}. ++ """ ++ self.assertRequestRejected([ ++ b'GET /a HTTP/1.1', ++ b'Transfer-Encoding: chunked', ++ b'Content-Length: 0', ++ b'Host: host.invalid', ++ b'', ++ b'', ++ b'GET /a HTTP/1.1', ++ b'Host: host.invalid', ++ b'', ++ b'', ++ ]) ++ ++ ++ def test_unknownTransferEncoding(self): ++ """ ++ A request whose C{transfer-encoding} header includes a value ++ other than C{chunked} or C{identity} fails with a 400 response ++ without calling L{Request.process}. ++ """ ++ self.assertRequestRejected([ ++ b'GET /a HTTP/1.1', ++ b'Transfer-Encoding: unknown', ++ b'Host: host.invalid', ++ b'', ++ b'', ++ ]) ++ ++ ++ def test_transferEncodingIdentity(self): ++ """ ++ A request with a valid C{content-length} and a ++ C{transfer-encoding} whose value is C{identity} succeeds. ++ """ ++ body = [] ++ ++ class SuccessfulRequest(http.Request): ++ processed = False ++ def process(self): ++ body.append(self.content.read()) ++ self.setHeader(b'content-length', b'0') ++ self.finish() ++ ++ request = b'''\ ++GET / HTTP/1.1 ++Host: host.invalid ++Content-Length: 2 ++Transfer-Encoding: identity ++ ++ok ++''' ++ channel = self.runRequest(request, SuccessfulRequest, False) ++ self.assertEqual(body, [b'ok']) ++ self.assertEqual( ++ channel.transport.value(), ++ b'HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n', ++ ) ++ ++ + + class QueryArgumentsTests(unittest.TestCase): + def testParseqs(self): |