diff options
author | Bert JW Regeer <bertjw@regeer.org> | 2022-03-12 18:48:26 -0700 |
---|---|---|
committer | Bert JW Regeer <bertjw@regeer.org> | 2022-03-12 19:48:25 -0700 |
commit | d9bdfa0cf210f6daf017d7c5a3cc149bdec8a9a7 (patch) | |
tree | fa22c813705ef57369ad39a8427b1279bdabb9dd /tests | |
parent | d032a669682838b26d6a1a1b513b9da83b0e0f90 (diff) | |
download | waitress-d9bdfa0cf210f6daf017d7c5a3cc149bdec8a9a7.tar.gz |
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.
Diffstat (limited to 'tests')
-rw-r--r-- | tests/test_functional.py | 22 | ||||
-rw-r--r-- | tests/test_receiver.py | 12 |
2 files changed, 34 insertions, 0 deletions
diff --git a/tests/test_functional.py b/tests/test_functional.py index 9e33fc0..60eb24a 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -364,6 +364,28 @@ class EchoTests: 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 a3b6f99..8e43cc8 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: def __init__(self, data=None): |