From b0ae7e3e156ac6f4a30ac4a54af0bffb707b008d Mon Sep 17 00:00:00 2001 From: Renata Ravanelli Date: Fri, 15 Sep 2023 12:41:06 -0300 Subject: [PATCH 5/6] This patch is a backport of commit d9bdfa0 From: Bert JW Regeer Date: Sat, 12 Mar 2022 18:48:26 -0700 Subject: [PATCH] Validate chunk size in Chunked Encoding are HEXDIG RFC7230 states that a chunk-size should be 1*HEXDIG, this is now validated before passing the resulting string to int() which would also parse other formats for hex, such as: `0x01` as `1` and `+0x01` as `1`. This would lead to a potential for a frontend proxy server and waitress to disagree on where a chunk started and ended, thereby potentially leading to request smuggling. With the increased validation if the size is not just hex digits, Waitress now returns a Bad Request and stops processing the request. Signed-off-by: Renata Ravanelli --- src/waitress/receiver.py | 19 ++++++++++++++----- tests/test_functional.py | 22 ++++++++++++++++++++++ tests/test_receiver.py | 12 ++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/waitress/receiver.py b/src/waitress/receiver.py index 106dbc7..9e4bffe 100644 --- a/src/waitress/receiver.py +++ b/src/waitress/receiver.py @@ -150,12 +150,21 @@ class ChunkedReceiver(object): self.all_chunks_received = True break + line = line[:semi] - try: - sz = int(line.strip(), 16) # hexadecimal - except ValueError: # garbage in input - self.error = BadRequest("garbage in chunked encoding input") - sz = 0 + + # Remove any whitespace + line = line.strip() + + if not ONLY_HEXDIG_RE.match(line): + self.error = BadRequest("Invalid chunk size") + self.all_chunks_received = True + + break + + # Can not fail due to matching against the regular + # expression above + sz = int(line.strip(), 16) # hexadecimal if sz > 0: # Start a new chunk. diff --git a/tests/test_functional.py b/tests/test_functional.py index 853942c..448e0c0 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -345,6 +345,28 @@ class EchoTests(object): self.send_check_error(to_send) self.assertRaises(ConnectionClosed, read_http, fp) + def test_broken_chunked_encoding_invalid_hex(self): + control_line = b"0x20\r\n" # 20 hex = 32 dec + s = b"This string has 32 characters.\r\n" + to_send = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send += control_line + s + b"\r\n" + self.connect() + self.sock.send(to_send) + with self.sock.makefile("rb", 0) as fp: + line, headers, response_body = read_http(fp) + self.assertline(line, "400", "Bad Request", "HTTP/1.1") + cl = int(headers["content-length"]) + self.assertEqual(cl, len(response_body)) + self.assertIn(b"Invalid chunk size", response_body) + self.assertEqual( + sorted(headers.keys()), + ["connection", "content-length", "content-type", "date", "server"], + ) + self.assertEqual(headers["content-type"], "text/plain") + # connection has been closed + self.send_check_error(to_send) + self.assertRaises(ConnectionClosed, read_http, fp) + def test_broken_chunked_encoding_invalid_extension(self): control_line = b"20;invalid=\r\n" # 20 hex = 32 dec s = b"This string has 32 characters.\r\n" diff --git a/tests/test_receiver.py b/tests/test_receiver.py index a6261ea..17328d4 100644 --- a/tests/test_receiver.py +++ b/tests/test_receiver.py @@ -262,6 +262,18 @@ class TestChunkedReceiverParametrized: assert result == len(data) assert inst.error == None + @pytest.mark.parametrize("invalid_size", [b"0x04", b"+0x04", b"x04", b"+04"]) + def test_received_invalid_size(self, invalid_size): + from waitress.utilities import BadRequest + + buf = DummyBuffer() + inst = self._makeOne(buf) + data = invalid_size + b"\r\ntest\r\n" + result = inst.received(data) + assert result == len(data) + assert inst.error.__class__ == BadRequest + assert inst.error.body == "Invalid chunk size" + class DummyBuffer(object): def __init__(self, data=None): -- 2.39.2 (Apple Git-143)