File CVE-2020-10108-http-req-headers.patch of Package python-Twisted.26707

Overview Repositories Revisions Requests Users Attributes Meta

File CVE-2020-10108-http-req-headers.patch of Package python-Twisted.26707

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.
---
 twisted/web/http.py                   |   64 ++++++++++++---
 twisted/web/newsfragments/9770.bugfix |    1 
 twisted/web/test/test_http.py         |  137 ++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+), 15 deletions(-)
 create mode 100644 src/twisted/web/newsfragments/9770.bugfix

Index: Twisted-15.2.1/twisted/web/http.py
===================================================================
--- Twisted-15.2.1.orig/twisted/web/http.py
+++ Twisted-15.2.1/twisted/web/http.py
@@ -1812,6 +1812,51 @@ class HTTPChannel(basic.LineReceiver, po
         self.allContentReceived()
         self.setLineMode(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():
+            _respondToBadRequestAndDisconnect(self.transport)
+            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):
         """
@@ -1830,21 +1875,10 @@ class HTTPChannel(basic.LineReceiver, po
         header, data = line.split(b':', 1)
         header = header.lower()
         data = data.strip()
-        if header == b'content-length':
-            if not data.isdigit():
-                return fail()
-            try:
-                self.length = _hexint(data)
-            except ValueError:
-                return fail()
-            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:
Index: Twisted-15.2.1/twisted/web/newsfragments/9770.bugfix
===================================================================
--- /dev/null
+++ Twisted-15.2.1/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
Index: Twisted-15.2.1/twisted/web/test/test_http.py
===================================================================
--- Twisted-15.2.1.orig/twisted/web/test/test_http.py
+++ Twisted-15.2.1/twisted/web/test/test_http.py
@@ -1254,6 +1254,143 @@ Hello,
         self.assertTrue(channel.transport.disconnecting)
 
 
+    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):

Places

File CVE-2020-10108-http-req-headers.patch of Package python-Twisted.26707

Places