Skip to content

Commit

Permalink
Validate chunk size in Chunked Encoding are HEXDIG
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
digitalresistor committed Mar 13, 2022
1 parent d032a66 commit d9bdfa0
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 5 deletions.
19 changes: 14 additions & 5 deletions src/waitress/receiver.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,21 @@ def received(self, s):
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.
Expand Down
22 changes: 22 additions & 0 deletions tests/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,28 @@ def test_broken_chunked_encoding(self):
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"
Expand Down
12 changes: 12 additions & 0 deletions tests/test_receiver.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,18 @@ def test_received_valid_extensions(self, valid_extension):
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):
Expand Down

0 comments on commit d9bdfa0

Please sign in to comment.