Improve validation of Transfer-Encoding

Waitress only supports a single Transfer-Encoding and that is chunked.
We will read the whole request into a temporary buffer and then remove
the header and set the Content-Length.

However HTTP desync/HTTP request smuggling attacks could potentially
provide multiple Transfer-Encoding headers that would not get
appropriately treated by waitress.

Waitress now treats the header as potentially containing multiple
values, and validates that the last encoding listed is "chunked".

At this time Waitress does not support any other encodings, and all
other requests will be rejected with a 501 Not Implemented error.

waitress/parser.py

@@ -26,6 +26,7 @@
     BadRequest,
     RequestEntityTooLarge,
     RequestHeaderFieldsTooLarge,
+    ServerNotImplemented,
     find_double_newline,
 )
 
@@ -34,6 +35,10 @@ class ParsingError(Exception):
     pass
 
 
+class TransferEncodingNotImplemented(Exception):
+    pass
+
+
 class HTTPRequestParser(object):
     """A structure that collects the HTTP request.
 
@@ -126,31 +131,39 @@ def received(self, data):
                     except ParsingError as e:
                         self.error = BadRequest(e.args[0])
                         self.completed = True
+                    except TransferEncodingNotImplemented as e:
+                        self.error = ServerNotImplemented(e.args[0])
+                        self.completed = True
                     else:
                         if self.body_rcv is None:
                             # no content-length header and not a t-e: chunked
                             # request
                             self.completed = True
+
                         if self.content_length > 0:
                             max_body = self.adj.max_request_body_size
                             # we won't accept this request if the content-length
                             # is too large
+
                             if self.content_length >= max_body:
                                 self.error = RequestEntityTooLarge(
                                     "exceeds max_body of %s" % max_body
                                 )
                                 self.completed = True
                 self.headers_finished = True
+
                 return consumed
 
             # Header not finished yet.
             self.header_plus = s
+
             return datalen
         else:
             # In body.
             consumed = br.received(data)
             self.body_bytes_received += consumed
             max_body = self.adj.max_request_body_size
+
             if self.body_bytes_received >= max_body:
                 # this will only be raised during t-e: chunked requests
                 self.error = RequestEntityTooLarge("exceeds max_body of %s" % max_body)
@@ -162,6 +175,7 @@ def received(self, data):
             elif br.completed:
                 # The request (with the body) is ready to use.
                 self.completed = True
+
                 if self.chunked:
                     # We've converted the chunked transfer encoding request
                     # body into a normal request body, so we know its content
@@ -241,10 +255,31 @@ def parse_header(self, header_plus):
             # should not see the HTTP_TRANSFER_ENCODING header; we pop it
             # here
             te = headers.pop("TRANSFER_ENCODING", "")
-            if te.lower() == "chunked":
+
+            encodings = [encoding.strip().lower() for encoding in te.split(",") if encoding]
+
+            for encoding in encodings:
+                # Out of the transfer-codings listed in
+                # https://tools.ietf.org/html/rfc7230#section-4 we only support
+                # chunked at this time.
+
+                # Note: the identity transfer-coding was removed in RFC7230:
+                # https://tools.ietf.org/html/rfc7230#appendix-A.2 and is thus
+                # not supported
+                if encoding not in {"chunked"}:
+                    raise TransferEncodingNotImplemented(
+                        "Transfer-Encoding requested is not supported."
+                    )
+
+            if encodings and encodings[-1] == "chunked":
                 self.chunked = True
                 buf = OverflowableBuffer(self.adj.inbuf_overflow)
                 self.body_rcv = ChunkedReceiver(buf)
+            elif encodings:  # pragma: nocover
+                raise TransferEncodingNotImplemented(
+                    "Transfer-Encoding requested is not supported."
+                )
+
             expect = headers.get("EXPECT", "").lower()
             self.expect_continue = expect == "100-continue"
             if connection.lower() == "close":

waitress/tests/test_parser.py

@@ -53,6 +53,21 @@ def test_received_bad_host_header(self):
         self.assertTrue(self.parser.completed)
         self.assertEqual(self.parser.error.__class__, BadRequest)
 
+    def test_received_bad_transfer_encoding(self):
+        from waitress.utilities import ServerNotImplemented
+        data = (
+            b"GET /foobar HTTP/1.1\r\n"
+            b"Transfer-Encoding: foo\r\n"
+            b"\r\n"
+            b"1d;\r\n"
+            b"This string has 29 characters\r\n"
+            b"0\r\n\r\n"
+        )
+        result = self.parser.received(data)
+        self.assertEqual(result, 48)
+        self.assertTrue(self.parser.completed)
+        self.assertEqual(self.parser.error.__class__, ServerNotImplemented)
+
     def test_received_nonsense_nothing(self):
         data = b"\r\n\r\n"
         result = self.parser.received(data)
@@ -196,6 +211,31 @@ def test_parse_header_11_te_chunked(self):
         self.parser.parse_header(data)
         self.assertEqual(self.parser.body_rcv.__class__.__name__, "ChunkedReceiver")
 
+
+    def test_parse_header_transfer_encoding_invalid(self):
+        from waitress.parser import TransferEncodingNotImplemented
+
+        data = b"GET /foobar HTTP/1.1\r\ntransfer-encoding: gzip\r\n"
+
+        try:
+            self.parser.parse_header(data)
+        except TransferEncodingNotImplemented as e:
+            self.assertIn("Transfer-Encoding requested is not supported.", e.args[0])
+        else:  # pragma: nocover
+            self.assertTrue(False)
+
+    def test_parse_header_transfer_encoding_invalid_multiple(self):
+        from waitress.parser import TransferEncodingNotImplemented
+
+        data = b"GET /foobar HTTP/1.1\r\ntransfer-encoding: gzip\r\ntransfer-encoding: chunked\r\n"
+
+        try:
+            self.parser.parse_header(data)
+        except TransferEncodingNotImplemented as e:
+            self.assertIn("Transfer-Encoding requested is not supported.", e.args[0])
+        else:  # pragma: nocover
+            self.assertTrue(False)
+
     def test_parse_header_11_expect_continue(self):
         data = b"GET /foobar HTTP/1.1\r\nexpect: 100-continue\r\n"
         self.parser.parse_header(data)

waitress/utilities.py

@@ -302,3 +302,8 @@ class RequestEntityTooLarge(BadRequest):
 class InternalServerError(Error):
     code = 500
     reason = "Internal Server Error"
+
+
+class ServerNotImplemented(Error):
+    code = 501
+    reason = "Not Implemented"