fail-safe on unsupported request framing

If we promise wsgi.input_terminated, we better get it right - or not at all.
* chunked encoding on HTTP <= 1.1
* chunked not last transfer coding
* multiple chinked codings
* any unknown codings (yes, this too! because we do not detect unusual syntax that is still chunked)
* empty coding (plausibly harmless, but not see in real life anyway - refused, for the moment)

gunicorn/config.py

@@ -2344,3 +2344,21 @@ class HeaderMap(Setting):
 
         .. versionadded:: 22.0.0
         """
+
+
+class TolerateDangerousFraming(Setting):
+    name = "tolerate_dangerous_framing"
+    section = "Server Mechanics"
+    cli = ["--tolerate-dangerous-framing"]
+    validator = validate_bool
+    action = "store_true"
+    default = False
+    desc = """\
+        Process requests with both Transfer-Encoding and Content-Length
+
+        This is known to induce vulnerabilities, but not strictly forbidden by RFC9112.
+
+        Use with care and only if necessary. May be removed in a future version.
+
+        .. versionadded:: 22.0.0
+        """

gunicorn/http/errors.py

@@ -73,6 +73,15 @@ class InvalidHeaderName(ParseException):
         return "Invalid HTTP header name: %r" % self.hdr
 
 
+class UnsupportedTransferCoding(ParseException):
+    def __init__(self, hdr):
+        self.hdr = hdr
+        self.code = 501
+
+    def __str__(self):
+        return "Unsupported transfer coding: %r" % self.hdr
+
+
 class InvalidChunkSize(IOError):
     def __init__(self, data):
         self.data = data

gunicorn/http/message.py

@@ -12,6 +12,7 @@ from gunicorn.http.errors import (
     InvalidHeader, InvalidHeaderName, NoMoreData,
     InvalidRequestLine, InvalidRequestMethod, InvalidHTTPVersion,
     LimitRequestLine, LimitRequestHeaders,
+    UnsupportedTransferCoding,
 )
 from gunicorn.http.errors import InvalidProxyLine, ForbiddenProxyRequest
 from gunicorn.http.errors import InvalidSchemeHeaders
@@ -39,6 +40,7 @@ class Message(object):
         self.trailers = []
         self.body = None
         self.scheme = "https" if cfg.is_ssl else "http"
+        self.must_close = False
 
         # set headers limits
         self.limit_request_fields = cfg.limit_request_fields
@@ -58,6 +60,9 @@ class Message(object):
         self.unreader.unread(unused)
         self.set_body_reader()
 
+    def force_close(self):
+        self.must_close = True
+
     def parse(self, unreader):
         raise NotImplementedError()
 
@@ -152,9 +157,47 @@ class Message(object):
                 content_length = value
             elif name == "TRANSFER-ENCODING":
                 if value.lower() == "chunked":
+                    # DANGER: transer codings stack, and stacked chunking is never intended
+                    if chunked:
+                        raise InvalidHeader("TRANSFER-ENCODING", req=self)
                     chunked = True
+                elif value.lower() == "identity":
+                    # does not do much, could still plausibly desync from what the proxy does
+                    # safe option: nuke it, its never needed
+                    if chunked:
+                        raise InvalidHeader("TRANSFER-ENCODING", req=self)
+                elif value.lower() == "":
+                    # lacking security review on this case
+                    # offer the option to restore previous behaviour, but refuse by default, for now
+                    self.force_close()
+                    if not self.cfg.tolerate_dangerous_framing:
+                        raise UnsupportedTransferCoding(value)
+                # DANGER: do not change lightly; ref: request smuggling
+                # T-E is a list and we *could* support correctly parsing its elements
+                #  .. but that is only safe after getting all the edge cases right
+                #  .. for which no real-world need exists, so best to NOT open that can of worms
+                else:
+                    self.force_close()
+                    # even if parser is extended, retain this branch:
+                    #  the "chunked not last" case remains to be rejected!
+                    raise UnsupportedTransferCoding(value)
 
         if chunked:
+            # two potentially dangerous cases:
+            #  a) CL + TE (TE overrides CL.. only safe if the recipient sees it that way too)
+            #  b) chunked HTTP/1.0 (always faulty)
+            if self.version < (1, 1):
+                # framing wonky, see RFC 9112 Section 6.1
+                self.force_close()
+                if not self.cfg.tolerate_dangerous_framing:
+                    raise InvalidHeader("TRANSFER-ENCODING", req=self)
+            if content_length is not None:
+                # we cannot be certain the message framing we understood matches proxy intent
+                #  -> whatever happens next, remaining input must not be trusted
+                self.force_close()
+                # either processing or rejecting is permitted in RFC 9112 Section 6.1
+                if not self.cfg.tolerate_dangerous_framing:
+                    raise InvalidHeader("CONTENT-LENGTH", req=self)
             self.body = Body(ChunkedReader(self, self.unreader))
         elif content_length is not None:
             try:
@@ -173,6 +216,8 @@ class Message(object):
             self.body = Body(EOFReader(self.unreader))
 
     def should_close(self):
+        if self.must_close:
+            return True
         for (h, v) in self.headers:
             if h == "CONNECTION":
                 v = v.lower().strip(" \t")

tests/requests/invalid/chunked_01.http

@@ -0,0 +1,12 @@
+POST /chunked_w_underscore_chunk_size HTTP/1.1\r\n
+Transfer-Encoding: chunked\r\n
+\r\n
+5\r\n
+hello\r\n
+6_0\r\n
+ world\r\n
+0\r\n
+\r\n
+POST /after HTTP/1.1\r\n
+Transfer-Encoding: identity\r\n
+\r\n

tests/requests/invalid/chunked_01.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import InvalidChunkSize
+request = InvalidChunkSize

tests/requests/invalid/chunked_02.http

@@ -0,0 +1,9 @@
+POST /chunked_with_prefixed_value HTTP/1.1\r\n
+Content-Length: 12\r\n
+Transfer-Encoding: \tchunked\r\n
+\r\n
+5\r\n
+hello\r\n
+6\r\n
+ world\r\n
+\r\n

tests/requests/invalid/chunked_02.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import InvalidHeader
+request = InvalidHeader

tests/requests/invalid/chunked_03.http

@@ -0,0 +1,8 @@
+POST /double_chunked HTTP/1.1\r\n
+Transfer-Encoding: identity, chunked, identity, chunked\r\n
+\r\n
+5\r\n
+hello\r\n
+6\r\n
+ world\r\n
+\r\n

tests/requests/invalid/chunked_03.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import UnsupportedTransferCoding
+request = UnsupportedTransferCoding

tests/requests/invalid/chunked_04.http

@@ -0,0 +1,11 @@
+POST /chunked_twice HTTP/1.1\r\n
+Transfer-Encoding: identity\r\n
+Transfer-Encoding: chunked\r\n
+Transfer-Encoding: identity\r\n
+Transfer-Encoding: chunked\r\n
+\r\n
+5\r\n
+hello\r\n
+6\r\n
+ world\r\n
+\r\n

tests/requests/invalid/chunked_04.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import InvalidHeader
+request = InvalidHeader

tests/requests/invalid/chunked_05.http

@@ -0,0 +1,11 @@
+POST /chunked_HTTP_1.0 HTTP/1.0\r\n
+Transfer-Encoding: chunked\r\n
+\r\n
+5\r\n
+hello\r\n
+6\r\n
+ world\r\n
+0\r\n
+Vary: *\r\n
+Content-Type: text/plain\r\n
+\r\n

tests/requests/invalid/chunked_05.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import InvalidHeader
+request = InvalidHeader

tests/requests/invalid/chunked_06.http

@@ -0,0 +1,9 @@
+POST /chunked_not_last HTTP/1.1\r\n
+Transfer-Encoding: chunked\r\n
+Transfer-Encoding: gzip\r\n
+\r\n
+5\r\n
+hello\r\n
+6\r\n
+ world\r\n
+\r\n

tests/requests/invalid/chunked_06.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import UnsupportedTransferCoding
+request = UnsupportedTransferCoding

tests/requests/invalid/chunked_08.http

@@ -0,0 +1,9 @@
+POST /chunked_not_last HTTP/1.1\r\n
+Transfer-Encoding: chunked\r\n
+Transfer-Encoding: identity\r\n
+\r\n
+5\r\n
+hello\r\n
+6\r\n
+ world\r\n
+\r\n

tests/requests/invalid/chunked_08.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import InvalidHeader
+request = InvalidHeader

tests/requests/invalid/nonascii_01.http

@@ -0,0 +1,4 @@
+GETß /germans.. HTTP/1.1\r\n
+Content-Length: 3\r\n
+\r\n
+ÄÄÄ

tests/requests/invalid/nonascii_01.py

@@ -0,0 +1,5 @@
+from gunicorn.config import Config
+from gunicorn.http.errors import InvalidRequestMethod
+
+cfg = Config()
+request = InvalidRequestMethod

tests/requests/invalid/nonascii_02.http

@@ -0,0 +1,4 @@
+GETÿ /french.. HTTP/1.1\r\n
+Content-Length: 3\r\n
+\r\n
+ÄÄÄ

tests/requests/invalid/nonascii_02.py

@@ -0,0 +1,5 @@
+from gunicorn.config import Config
+from gunicorn.http.errors import InvalidRequestMethod
+
+cfg = Config()
+request = InvalidRequestMethod

tests/requests/invalid/nonascii_04.http

@@ -0,0 +1,5 @@
+GET /french.. HTTP/1.1\r\n
+Content-Lengthÿ: 3\r\n
+Content-Length: 3\r\n
+\r\n
+ÄÄÄ

tests/requests/invalid/nonascii_04.py

@@ -0,0 +1,5 @@
+from gunicorn.config import Config
+from gunicorn.http.errors import InvalidHeaderName
+
+cfg = Config()
+request = InvalidHeaderName

tests/requests/invalid/prefix_01.http

@@ -0,0 +1,2 @@
+GET\0PROXY /foo HTTP/1.1\r\n
+\r\n

tests/requests/invalid/prefix_01.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import InvalidRequestMethod
+request = InvalidRequestMethod

tests/requests/invalid/prefix_02.http

@@ -0,0 +1,2 @@
+GET\0 /foo HTTP/1.1\r\n
+\r\n

tests/requests/invalid/prefix_02.py

@@ -0,0 +1,2 @@
+from gunicorn.http.errors import InvalidRequestMethod
+request = InvalidRequestMethod

tests/requests/invalid/prefix_03.http

@@ -0,0 +1,4 @@
+GET /stuff/here?foo=bar HTTP/1.1\r\n
+Content-Length: 0 1\r\n
+\r\n
+x

tests/requests/invalid/prefix_03.py

@@ -0,0 +1,5 @@
+from gunicorn.config import Config
+from gunicorn.http.errors import InvalidHeader
+
+cfg = Config()
+request = InvalidHeader

tests/requests/invalid/prefix_04.http

@@ -0,0 +1,5 @@
+GET /stuff/here?foo=bar HTTP/1.1\r\n
+Content-Length: 3 1\r\n
+\r\n
+xyz
+abc123

tests/requests/invalid/prefix_04.py

@@ -0,0 +1,5 @@
+from gunicorn.config import Config
+from gunicorn.http.errors import InvalidHeader
+
+cfg = Config()
+request = InvalidHeader

tests/requests/invalid/prefix_05.http

@@ -0,0 +1,4 @@
+GET: /stuff/here?foo=bar HTTP/1.1\r\n
+Content-Length: 3\r\n
+\r\n
+xyz

tests/requests/invalid/prefix_05.py

@@ -0,0 +1,5 @@
+from gunicorn.config import Config
+from gunicorn.http.errors import InvalidRequestMethod
+
+cfg = Config()
+request = InvalidRequestMethod

tests/requests/valid/025.http

@@ -1,5 +1,4 @@
 POST /chunked_cont_h_at_first HTTP/1.1\r\n
-Content-Length: -1\r\n
 Transfer-Encoding: chunked\r\n
 \r\n
 5; some; parameters=stuff\r\n
@@ -16,4 +15,10 @@ Content-Length: -1\r\n
 hello\r\n
 6; blahblah; blah\r\n
  world\r\n
-0\r\n
+0\r\n
+\r\n
+PUT /ignored_after_dangerous_framing HTTP/1.1\r\n
+Content-Length: 3\r\n
+\r\n
+foo\r\n
+\r\n

tests/requests/valid/025.py

@@ -1,9 +1,13 @@
+from gunicorn.config import Config
+
+cfg = Config()
+cfg.set("tolerate_dangerous_framing", True)
+
 req1 = {
     "method": "POST",
     "uri": uri("/chunked_cont_h_at_first"),
     "version": (1, 1),
     "headers": [
-        ("CONTENT-LENGTH", "-1"),
         ("TRANSFER-ENCODING", "chunked")
     ],
     "body": b"hello world"

tests/requests/valid/025compat.http

@@ -0,0 +1,18 @@
+POST /chunked_cont_h_at_first HTTP/1.1\r\n
+Transfer-Encoding: chunked\r\n
+\r\n
+5; some; parameters=stuff\r\n
+hello\r\n
+6; blahblah; blah\r\n
+ world\r\n
+0\r\n
+\r\n
+PUT /chunked_cont_h_at_last HTTP/1.1\r\n
+Transfer-Encoding: chunked\r\n
+Content-Length: -1\r\n
+\r\n
+5; some; parameters=stuff\r\n
+hello\r\n
+6; blahblah; blah\r\n
+ world\r\n
+0\r\n

tests/requests/valid/025compat.py

@@ -0,0 +1,27 @@
+from gunicorn.config import Config
+
+cfg = Config()
+cfg.set("tolerate_dangerous_framing", True)
+
+req1 = {
+    "method": "POST",
+    "uri": uri("/chunked_cont_h_at_first"),
+    "version": (1, 1),
+    "headers": [
+        ("TRANSFER-ENCODING", "chunked")
+    ],
+    "body": b"hello world"
+}
+
+req2 = {
+    "method": "PUT",
+    "uri": uri("/chunked_cont_h_at_last"),
+    "version": (1, 1),
+    "headers": [
+        ("TRANSFER-ENCODING", "chunked"),
+        ("CONTENT-LENGTH", "-1"),
+    ],
+    "body": b"hello world"
+}
+
+request = [req1, req2]

tests/requests/valid/029.http

@@ -1,6 +1,6 @@
 GET /stuff/here?foo=bar HTTP/1.1\r\n
-Transfer-Encoding: chunked\r\n
 Transfer-Encoding: identity\r\n
+Transfer-Encoding: chunked\r\n
 \r\n
 5\r\n
 hello\r\n

tests/requests/valid/029.py

@@ -7,8 +7,8 @@ request = {
     "uri": uri("/stuff/here?foo=bar"),
     "version": (1, 1),
     "headers": [
+        ('TRANSFER-ENCODING', 'identity'),
         ('TRANSFER-ENCODING', 'chunked'),
-        ('TRANSFER-ENCODING', 'identity')
     ],
     "body": b"hello"
 }

tests/treq.py

@@ -248,8 +248,10 @@ class request(object):
     def check(self, cfg, sender, sizer, matcher):
         cases = self.expect[:]
         p = RequestParser(cfg, sender(), None)
-        for req in p:
+        parsed_request_idx = -1
+        for parsed_request_idx, req in enumerate(p):
             self.same(req, sizer, matcher, cases.pop(0))
+        assert len(self.expect) == parsed_request_idx + 1
         assert not cases
 
     def same(self, req, sizer, matcher, exp):