From 1f6059f4c4a3a0b256b4027eda64fb9fc311b0a6 Mon Sep 17 00:00:00 2001 From: Bert JW Regeer Date: Sat, 12 Mar 2022 18:32:24 -0700 Subject: [PATCH] Be more strict in parsing Content-Length Validate that we are only parsing digits and nothing else. RFC7230 is explicit in that the Content-Length can only exist of 1*DIGIT and may not include any additional sign information. The Python int() function parses `+10` as `10` which means we were more lenient than the standard intended. --- src/waitress/parser.py | 12 ++++++------ tests/test_parser.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/waitress/parser.py b/src/waitress/parser.py index a6e4d982..ff16a402 100644 --- a/src/waitress/parser.py +++ b/src/waitress/parser.py @@ -23,6 +23,7 @@ from waitress.buffers import OverflowableBuffer from waitress.receiver import ChunkedReceiver, FixedStreamReceiver +from waitress.rfc7230 import HEADER_FIELD_RE, ONLY_DIGIT_RE from waitress.utilities import ( BadRequest, RequestEntityTooLarge, @@ -31,8 +32,6 @@ find_double_newline, ) -from .rfc7230 import HEADER_FIELD - def unquote_bytes_to_wsgi(bytestring): return unquote_to_bytes(bytestring).decode("latin-1") @@ -221,7 +220,7 @@ def parse_header(self, header_plus): headers = self.headers for line in lines: - header = HEADER_FIELD.match(line) + header = HEADER_FIELD_RE.match(line) if not header: raise ParsingError("Invalid header") @@ -317,11 +316,12 @@ def parse_header(self, header_plus): self.connection_close = True if not self.chunked: - try: - cl = int(headers.get("CONTENT_LENGTH", 0)) - except ValueError: + cl = headers.get("CONTENT_LENGTH", "0") + + if not ONLY_DIGIT_RE.match(cl.encode("latin-1")): raise ParsingError("Content-Length is invalid") + cl = int(cl) self.content_length = cl if cl > 0: diff --git a/tests/test_parser.py b/tests/test_parser.py index aacef26d..868c1225 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -193,6 +193,26 @@ def test_parse_header_bad_content_length(self): else: # pragma: nocover self.assertTrue(False) + def test_parse_header_bad_content_length_plus(self): + data = b"GET /foobar HTTP/8.4\r\ncontent-length: +10\r\n" + + try: + self.parser.parse_header(data) + except ParsingError as e: + self.assertIn("Content-Length is invalid", e.args[0]) + else: # pragma: nocover + self.assertTrue(False) + + def test_parse_header_bad_content_length_minus(self): + data = b"GET /foobar HTTP/8.4\r\ncontent-length: -10\r\n" + + try: + self.parser.parse_header(data) + except ParsingError as e: + self.assertIn("Content-Length is invalid", e.args[0]) + else: # pragma: nocover + self.assertTrue(False) + def test_parse_header_multiple_content_length(self): data = b"GET /foobar HTTP/8.4\r\ncontent-length: 10\r\ncontent-length: 20\r\n"