From 096629d1b4ef8255d304bb7f7efba3e295263546 Mon Sep 17 00:00:00 2001 From: Carl George Date: Tue, 25 Jun 2024 23:38:55 -0500 Subject: [PATCH] Backport upstream fix for CVE-2022-24761 rhbz#2065791 --- ...lar-expressions-for-Chunked-Encoding.patch | 126 ++++++++++++++++ 0001-Skip-test_in_generator.patch | 30 ---- ...ore-strict-in-parsing-Content-Length.patch | 104 ++++++++++++++ ...-invalid-chunked-encoding-chunk-size.patch | 72 ++++++++++ ...-when-receiving-back-Chunk-Extension.patch | 135 ++++++++++++++++++ ...-size-in-Chunked-Encoding-are-HEXDIG.patch | 112 +++++++++++++++ ...-calls-to-.strip-in-Chunked-Encoding.patch | 64 +++++++++ 0007-Backport-security-fix-note.patch | 43 ++++++ ...ail-inconsistently-during-mock-build.patch | 32 +++++ python-waitress.spec | 22 ++- 10 files changed, 707 insertions(+), 33 deletions(-) create mode 100644 0001-Add-new-regular-expressions-for-Chunked-Encoding.patch delete mode 100644 0001-Skip-test_in_generator.patch create mode 100644 0002-Be-more-strict-in-parsing-Content-Length.patch create mode 100644 0003-Update-tests-to-remove-invalid-chunked-encoding-chunk-size.patch create mode 100644 0004-Error-when-receiving-back-Chunk-Extension.patch create mode 100644 0005-Validate-chunk-size-in-Chunked-Encoding-are-HEXDIG.patch create mode 100644 0006-Remove-extraneous-calls-to-.strip-in-Chunked-Encoding.patch create mode 100644 0007-Backport-security-fix-note.patch create mode 100644 0008-Skip-tests-that-fail-inconsistently-during-mock-build.patch diff --git a/0001-Add-new-regular-expressions-for-Chunked-Encoding.patch b/0001-Add-new-regular-expressions-for-Chunked-Encoding.patch new file mode 100644 index 0000000..23f3a5f --- /dev/null +++ b/0001-Add-new-regular-expressions-for-Chunked-Encoding.patch @@ -0,0 +1,126 @@ +From b3b4d0847c0b22a6f2b12090d8b6b79c4cdea95c Mon Sep 17 00:00:00 2001 +From: Bert JW Regeer +Date: Sat, 12 Mar 2022 18:30:30 -0700 +Subject: [PATCH 1/8] Add new regular expressions for Chunked Encoding + +This also moves some regular expressions for QUOTED_PAIR/QUOTED_STRING +into this module from utilities so that they may be reused. + +(cherry picked from commit e75b0d9afbea8a933f8f5f11d279e661cbfd676b) +--- + waitress/rfc7230.py | 27 ++++++++++++++++++++++++++- + waitress/utilities.py | 28 +++------------------------- + 2 files changed, 29 insertions(+), 26 deletions(-) + +diff --git a/waitress/rfc7230.py b/waitress/rfc7230.py +index cd33c90..4c4c0a9 100644 +--- a/waitress/rfc7230.py ++++ b/waitress/rfc7230.py +@@ -7,6 +7,9 @@ import re + + from .compat import tobytes + ++HEXDIG = "[0-9a-fA-F]" ++DIGIT = "[0-9]" ++ + WS = "[ \t]" + OWS = WS + "{0,}?" + RWS = WS + "{1,}?" +@@ -27,6 +30,12 @@ TOKEN = TCHAR + "{1,}" + # ; visible (printing) characters + VCHAR = r"\x21-\x7e" + ++# The '\\' between \x5b and \x5d is needed to escape \x5d (']') ++QDTEXT = "[\t \x21\x23-\x5b\\\x5d-\x7e" + OBS_TEXT + "]" ++ ++QUOTED_PAIR = r"\\" + "([\t " + VCHAR + OBS_TEXT + "])" ++QUOTED_STRING = '"(?:(?:' + QDTEXT + ")|(?:" + QUOTED_PAIR + '))*"' ++ + # header-field = field-name ":" OWS field-value OWS + # field-name = token + # field-value = *( field-content / obs-fold ) +@@ -45,8 +54,24 @@ FIELD_CONTENT = FIELD_VCHAR + "+(?:[ \t]+" + FIELD_VCHAR + "+)*" + # Which allows the field value here to just see if there is even a value in the first place + FIELD_VALUE = "(?:" + FIELD_CONTENT + ")?" + +-HEADER_FIELD = re.compile( ++# chunk-ext = *( ";" chunk-ext-name [ "=" chunk-ext-val ] ) ++# chunk-ext-name = token ++# chunk-ext-val = token / quoted-string ++ ++CHUNK_EXT_NAME = TOKEN ++CHUNK_EXT_VAL = "(?:" + TOKEN + ")|(?:" + QUOTED_STRING + ")" ++CHUNK_EXT = ( ++ "(?:;(?P" + CHUNK_EXT_NAME + ")(?:=(?P" + CHUNK_EXT_VAL + "))?)*" ++) ++ ++# Pre-compiled regular expressions for use elsewhere ++ONLY_HEXDIG_RE = re.compile(tobytes("^" + HEXDIG + "+$")) ++ONLY_DIGIT_RE = re.compile(tobytes("^" + DIGIT + "+$")) ++HEADER_FIELD_RE = re.compile( + tobytes( + "^(?P" + TOKEN + "):" + OWS + "(?P" + FIELD_VALUE + ")" + OWS + "$" + ) + ) ++QUOTED_PAIR_RE = re.compile(QUOTED_PAIR) ++QUOTED_STRING_RE = re.compile(QUOTED_STRING) ++CHUNK_EXT_RE = re.compile(tobytes("^" + CHUNK_EXT + "$")) +diff --git a/waitress/utilities.py b/waitress/utilities.py +index 556bed2..fa59657 100644 +--- a/waitress/utilities.py ++++ b/waitress/utilities.py +@@ -22,7 +22,7 @@ import re + import stat + import time + +-from .rfc7230 import OBS_TEXT, VCHAR ++from .rfc7230 import QUOTED_PAIR_RE, QUOTED_STRING_RE + + logger = logging.getLogger("waitress") + queue_logger = logging.getLogger("waitress.queue") +@@ -216,32 +216,10 @@ def parse_http_date(d): + return retval + + +-# RFC 5234 Appendix B.1 "Core Rules": +-# VCHAR = %x21-7E +-# ; visible (printing) characters +-vchar_re = VCHAR +- +-# RFC 7230 Section 3.2.6 "Field Value Components": +-# quoted-string = DQUOTE *( qdtext / quoted-pair ) DQUOTE +-# qdtext = HTAB / SP /%x21 / %x23-5B / %x5D-7E / obs-text +-# obs-text = %x80-FF +-# quoted-pair = "\" ( HTAB / SP / VCHAR / obs-text ) +-obs_text_re = OBS_TEXT +- +-# The '\\' between \x5b and \x5d is needed to escape \x5d (']') +-qdtext_re = "[\t \x21\x23-\x5b\\\x5d-\x7e" + obs_text_re + "]" +- +-quoted_pair_re = r"\\" + "([\t " + vchar_re + obs_text_re + "])" +-quoted_string_re = '"(?:(?:' + qdtext_re + ")|(?:" + quoted_pair_re + '))*"' +- +-quoted_string = re.compile(quoted_string_re) +-quoted_pair = re.compile(quoted_pair_re) +- +- + def undquote(value): + if value.startswith('"') and value.endswith('"'): + # So it claims to be DQUOTE'ed, let's validate that +- matches = quoted_string.match(value) ++ matches = QUOTED_STRING_RE.match(value) + + if matches and matches.end() == len(value): + # Remove the DQUOTE's from the value +@@ -249,7 +227,7 @@ def undquote(value): + + # Remove all backslashes that are followed by a valid vchar or + # obs-text +- value = quoted_pair.sub(r"\1", value) ++ value = QUOTED_PAIR_RE.sub(r"\1", value) + + return value + elif not value.startswith('"') and not value.endswith('"'): +-- +2.45.2 + diff --git a/0001-Skip-test_in_generator.patch b/0001-Skip-test_in_generator.patch deleted file mode 100644 index 1a66d68..0000000 --- a/0001-Skip-test_in_generator.patch +++ /dev/null @@ -1,30 +0,0 @@ -From 0278f7a9ca9e65c877a34b5c122dbdda9f78c23d Mon Sep 17 00:00:00 2001 -From: Carl George -Date: Tue, 9 May 2023 23:01:49 -0500 -Subject: [PATCH] Skip test_in_generator - -This test fails during a mock build. - -Traceback (most recent call last): - File "/builddir/build/BUILD/waitress-1.4.3-nodocs/waitress/tests/test_functional.py", line 1207, in test_in_generator - self.assertRaises(ConnectionClosed, read_http, fp) -AssertionError: ConnectionClosed not raised ---- - waitress/tests/test_functional.py | 1 + - 1 file changed, 1 insertion(+) - -diff --git a/waitress/tests/test_functional.py b/waitress/tests/test_functional.py -index 8f4b262..c748f3a 100644 ---- a/waitress/tests/test_functional.py -+++ b/waitress/tests/test_functional.py -@@ -1193,6 +1193,7 @@ class InternalServerErrorTests(object): - self.send_check_error(to_send) - self.assertRaises(ConnectionClosed, read_http, fp) - -+ @unittest.skip('fails in mock build') - def test_in_generator(self): - to_send = "GET /in_generator HTTP/1.1\r\n\r\n" - to_send = tobytes(to_send) --- -2.40.1 - diff --git a/0002-Be-more-strict-in-parsing-Content-Length.patch b/0002-Be-more-strict-in-parsing-Content-Length.patch new file mode 100644 index 0000000..5089549 --- /dev/null +++ b/0002-Be-more-strict-in-parsing-Content-Length.patch @@ -0,0 +1,104 @@ +From 4105558a82b9d4fd7d68b1887dc22f6a0b627b5f Mon Sep 17 00:00:00 2001 +From: Bert JW Regeer +Date: Sat, 12 Mar 2022 18:32:24 -0700 +Subject: [PATCH 2/8] 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. + +(cherry picked from commit 1f6059f4c4a3a0b256b4027eda64fb9fc311b0a6) +--- + waitress/parser.py | 13 +++++++------ + waitress/tests/test_parser.py | 24 ++++++++++++++++++++++++ + 2 files changed, 31 insertions(+), 6 deletions(-) + +diff --git a/waitress/parser.py b/waitress/parser.py +index fef8a3d..500730e 100644 +--- a/waitress/parser.py ++++ b/waitress/parser.py +@@ -20,8 +20,9 @@ import re + from io import BytesIO + + from waitress.buffers import OverflowableBuffer +-from waitress.compat import tostr, unquote_bytes_to_wsgi, urlparse ++from waitress.compat import tostr, tobytes, unquote_bytes_to_wsgi, urlparse + from waitress.receiver import ChunkedReceiver, FixedStreamReceiver ++from waitress.rfc7230 import HEADER_FIELD_RE, ONLY_DIGIT_RE + from waitress.utilities import ( + BadRequest, + RequestEntityTooLarge, +@@ -29,7 +30,6 @@ from waitress.utilities import ( + ServerNotImplemented, + find_double_newline, + ) +-from .rfc7230 import HEADER_FIELD + + + class ParsingError(Exception): +@@ -208,7 +208,7 @@ class HTTPRequestParser(object): + + 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") +@@ -298,11 +298,12 @@ class HTTPRequestParser(object): + 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(tobytes(cl)): + raise ParsingError("Content-Length is invalid") + ++ cl = int(cl) + self.content_length = cl + if cl > 0: + buf = OverflowableBuffer(self.adj.inbuf_overflow) +diff --git a/waitress/tests/test_parser.py b/waitress/tests/test_parser.py +index 91837c7..eabf353 100644 +--- a/waitress/tests/test_parser.py ++++ b/waitress/tests/test_parser.py +@@ -194,6 +194,30 @@ class TestHTTPRequestParser(unittest.TestCase): + else: # pragma: nocover + self.assertTrue(False) + ++ def test_parse_header_bad_content_length_plus(self): ++ from waitress.parser import ParsingError ++ ++ 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): ++ from waitress.parser import ParsingError ++ ++ 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): + from waitress.parser import ParsingError + +-- +2.45.2 + diff --git a/0003-Update-tests-to-remove-invalid-chunked-encoding-chunk-size.patch b/0003-Update-tests-to-remove-invalid-chunked-encoding-chunk-size.patch new file mode 100644 index 0000000..9d43105 --- /dev/null +++ b/0003-Update-tests-to-remove-invalid-chunked-encoding-chunk-size.patch @@ -0,0 +1,72 @@ +From 42bd030d29b392baed1d427916200df75f4a4a12 Mon Sep 17 00:00:00 2001 +From: Bert JW Regeer +Date: Sat, 12 Mar 2022 18:35:01 -0700 +Subject: [PATCH 3/8] Update tests to remove invalid chunked encoding + chunk-size + +RFC7230 states the following: + + chunk = chunk-size [ chunk-ext ] CRLF + chunk-data CRLF + chunk-size = 1*HEXDIG + +Where chunk-ext is: + + chunk-ext = *( ";" chunk-ext-name [ "=" chunk-ext-val ] ) + +Only if there is a chunk-ext should there be a `;` after the 1*HEXDIG. +And a chunk-ext that is empty is invalid. + +(cherry picked from commit 884bed167d09c3d5fdf0730e2ca2564eefdd4534) +--- + waitress/tests/test_functional.py | 6 +++--- + waitress/tests/test_parser.py | 2 +- + 2 files changed, 4 insertions(+), 4 deletions(-) + +diff --git a/waitress/tests/test_functional.py b/waitress/tests/test_functional.py +index 8f4b262..33f1317 100644 +--- a/waitress/tests/test_functional.py ++++ b/waitress/tests/test_functional.py +@@ -301,7 +301,7 @@ class EchoTests(object): + self.assertFalse("transfer-encoding" in headers) + + def test_chunking_request_with_content(self): +- control_line = b"20;\r\n" # 20 hex = 32 dec ++ control_line = b"20\r\n" # 20 hex = 32 dec + s = b"This string has 32 characters.\r\n" + expected = s * 12 + header = tobytes("GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n") +@@ -320,7 +320,7 @@ class EchoTests(object): + self.assertFalse("transfer-encoding" in headers) + + def test_broken_chunked_encoding(self): +- control_line = "20;\r\n" # 20 hex = 32 dec ++ control_line = "20\r\n" # 20 hex = 32 dec + s = "This string has 32 characters.\r\n" + to_send = "GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send += control_line + s + "\r\n" +@@ -344,7 +344,7 @@ class EchoTests(object): + self.assertRaises(ConnectionClosed, read_http, fp) + + def test_broken_chunked_encoding_missing_chunk_end(self): +- control_line = "20;\r\n" # 20 hex = 32 dec ++ control_line = "20\r\n" # 20 hex = 32 dec + s = "This string has 32 characters.\r\n" + to_send = "GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send += control_line + s +diff --git a/waitress/tests/test_parser.py b/waitress/tests/test_parser.py +index eabf353..420f280 100644 +--- a/waitress/tests/test_parser.py ++++ b/waitress/tests/test_parser.py +@@ -152,7 +152,7 @@ class TestHTTPRequestParser(unittest.TestCase): + b"Transfer-Encoding: chunked\r\n" + b"X-Foo: 1\r\n" + b"\r\n" +- b"1d;\r\n" ++ b"1d\r\n" + b"This string has 29 characters\r\n" + b"0\r\n\r\n" + ) +-- +2.45.2 + diff --git a/0004-Error-when-receiving-back-Chunk-Extension.patch b/0004-Error-when-receiving-back-Chunk-Extension.patch new file mode 100644 index 0000000..1f9b707 --- /dev/null +++ b/0004-Error-when-receiving-back-Chunk-Extension.patch @@ -0,0 +1,135 @@ +From 7661d0826c9d0f197e66feed5b306b56c90255c4 Mon Sep 17 00:00:00 2001 +From: Bert JW Regeer +Date: Sat, 12 Mar 2022 18:42:51 -0700 +Subject: [PATCH 4/8] Error when receiving back Chunk Extension + +Waitress discards chunked extensions and does no further processing on +them, however it failed to validate that the chunked encoding extension +did not contain invalid data. + +We now validate that if there are any chunked extensions that they are +well-formed, if they are not and contain invalid characters, then +Waitress will now correctly return a Bad Request and stop any further +processing of the request. + +(cherry picked from commit d032a669682838b26d6a1a1b513b9da83b0e0f90) +--- + waitress/receiver.py | 11 ++++++++++- + waitress/tests/test_functional.py | 22 ++++++++++++++++++++++ + waitress/tests/test_receiver.py | 31 +++++++++++++++++++++++++++++++ + 3 files changed, 63 insertions(+), 1 deletion(-) + +diff --git a/waitress/receiver.py b/waitress/receiver.py +index 5d1568d..106dbc7 100644 +--- a/waitress/receiver.py ++++ b/waitress/receiver.py +@@ -14,6 +14,7 @@ + """Data Chunk Receiver + """ + ++from waitress.rfc7230 import CHUNK_EXT_RE, ONLY_HEXDIG_RE + from waitress.utilities import BadRequest, find_double_newline + + +@@ -110,6 +111,7 @@ class ChunkedReceiver(object): + s = b"" + else: + self.chunk_end = b"" ++ + if pos == 0: + # Chop off the terminating CR LF from the chunk + s = s[2:] +@@ -140,7 +142,14 @@ class ChunkedReceiver(object): + semi = line.find(b";") + + if semi >= 0: +- # discard extension info. ++ extinfo = line[semi:] ++ valid_ext_info = CHUNK_EXT_RE.match(extinfo) ++ ++ if not valid_ext_info: ++ self.error = BadRequest("Invalid chunk extension") ++ self.all_chunks_received = True ++ ++ break + line = line[:semi] + try: + sz = int(line.strip(), 16) # hexadecimal +diff --git a/waitress/tests/test_functional.py b/waitress/tests/test_functional.py +index 33f1317..b1aac96 100644 +--- a/waitress/tests/test_functional.py ++++ b/waitress/tests/test_functional.py +@@ -343,6 +343,28 @@ class EchoTests(object): + 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" ++ 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) ++ fp = self.sock.makefile("rb", 0) ++ 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 extension", 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_missing_chunk_end(self): + control_line = "20\r\n" # 20 hex = 32 dec + s = "This string has 32 characters.\r\n" +diff --git a/waitress/tests/test_receiver.py b/waitress/tests/test_receiver.py +index b4910bb..e5d31a3 100644 +--- a/waitress/tests/test_receiver.py ++++ b/waitress/tests/test_receiver.py +@@ -226,6 +226,37 @@ class TestChunkedReceiver(unittest.TestCase): + self.assertEqual(inst.error, None) + + ++class TestChunkedReceiverParametrized: ++ def _makeOne(self, buf): ++ from waitress.receiver import ChunkedReceiver ++ ++ return ChunkedReceiver(buf) ++ ++ def test_received_invalid_extensions(self): ++ from waitress.utilities import BadRequest ++ ++ for invalid_extension in [b"\n", b"invalid=", b"\r", b"invalid = true"]: ++ buf = DummyBuffer() ++ inst = self._makeOne(buf) ++ data = b"4;" + invalid_extension + b"\r\ntest\r\n" ++ result = inst.received(data) ++ assert result == len(data) ++ assert inst.error.__class__ == BadRequest ++ assert inst.error.body == "Invalid chunk extension" ++ ++ def test_received_valid_extensions(self): ++ # While waitress may ignore extensions in Chunked Encoding, we do want ++ # to make sure that we don't fail when we do encounter one that is ++ # valid ++ for valid_extension in [b"test", b"valid=true", b"valid=true;other=true"]: ++ buf = DummyBuffer() ++ inst = self._makeOne(buf) ++ data = b"4;" + valid_extension + b"\r\ntest\r\n" ++ result = inst.received(data) ++ assert result == len(data) ++ assert inst.error == None ++ ++ + class DummyBuffer(object): + def __init__(self, data=None): + if data is None: +-- +2.45.2 + diff --git a/0005-Validate-chunk-size-in-Chunked-Encoding-are-HEXDIG.patch b/0005-Validate-chunk-size-in-Chunked-Encoding-are-HEXDIG.patch new file mode 100644 index 0000000..292dd1a --- /dev/null +++ b/0005-Validate-chunk-size-in-Chunked-Encoding-are-HEXDIG.patch @@ -0,0 +1,112 @@ +From 4f0c74f6aab47c599d33d36cd783b5fa330384d9 Mon Sep 17 00:00:00 2001 +From: Bert JW Regeer +Date: Sat, 12 Mar 2022 18:48:26 -0700 +Subject: [PATCH 5/8] 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. + +(cherry picked from commit d9bdfa0cf210f6daf017d7c5a3cc149bdec8a9a7) +--- + waitress/receiver.py | 19 ++++++++++++++----- + waitress/tests/test_functional.py | 22 ++++++++++++++++++++++ + waitress/tests/test_receiver.py | 12 ++++++++++++ + 3 files changed, 48 insertions(+), 5 deletions(-) + +diff --git a/waitress/receiver.py b/waitress/receiver.py +index 106dbc7..9e4bffe 100644 +--- a/waitress/receiver.py ++++ b/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/waitress/tests/test_functional.py b/waitress/tests/test_functional.py +index b1aac96..a7421c6 100644 +--- a/waitress/tests/test_functional.py ++++ b/waitress/tests/test_functional.py +@@ -343,6 +343,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) ++ fp = self.sock.makefile("rb", 0) ++ 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/waitress/tests/test_receiver.py b/waitress/tests/test_receiver.py +index e5d31a3..b539264 100644 +--- a/waitress/tests/test_receiver.py ++++ b/waitress/tests/test_receiver.py +@@ -256,6 +256,18 @@ class TestChunkedReceiverParametrized: + assert result == len(data) + assert inst.error == None + ++ def test_received_invalid_size(self, invalid_size): ++ from waitress.utilities import BadRequest ++ ++ for invalid_size in [b"0x04", b"+0x04", b"x04", b"+04"]: ++ 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.45.2 + diff --git a/0006-Remove-extraneous-calls-to-.strip-in-Chunked-Encoding.patch b/0006-Remove-extraneous-calls-to-.strip-in-Chunked-Encoding.patch new file mode 100644 index 0000000..e634e37 --- /dev/null +++ b/0006-Remove-extraneous-calls-to-.strip-in-Chunked-Encoding.patch @@ -0,0 +1,64 @@ +From 92c5f8b8dbfc73780f8404b225b1282d58c5cd96 Mon Sep 17 00:00:00 2001 +From: Bert JW Regeer +Date: Sat, 12 Mar 2022 19:16:23 -0700 +Subject: [PATCH 6/8] Remove extraneous calls to .strip() in Chunked Encoding + +To be valid chunked encoding we should not be removing any whitespace as +the standard does not allow for optional whitespace. + +If whitespace is encountered in the wrong place, it should lead to a 400 +Bad Request instead. + +(cherry picked from commit bd22869c143a3f1284f271399524676efbafa655) +--- + waitress/receiver.py | 6 +----- + waitress/tests/test_receiver.py | 2 +- + 2 files changed, 2 insertions(+), 6 deletions(-) + +diff --git a/waitress/receiver.py b/waitress/receiver.py +index 9e4bffe..806ff87 100644 +--- a/waitress/receiver.py ++++ b/waitress/receiver.py +@@ -135,7 +135,6 @@ class ChunkedReceiver(object): + line = s[:pos] + s = s[pos + 2 :] + self.control_line = b"" +- line = line.strip() + + if line: + # Begin a new chunk. +@@ -153,9 +152,6 @@ class ChunkedReceiver(object): + + line = line[:semi] + +- # Remove any whitespace +- line = line.strip() +- + if not ONLY_HEXDIG_RE.match(line): + self.error = BadRequest("Invalid chunk size") + self.all_chunks_received = True +@@ -164,7 +160,7 @@ class ChunkedReceiver(object): + + # Can not fail due to matching against the regular + # expression above +- sz = int(line.strip(), 16) # hexadecimal ++ sz = int(line, 16) # hexadecimal + + if sz > 0: + # Start a new chunk. +diff --git a/waitress/tests/test_receiver.py b/waitress/tests/test_receiver.py +index b539264..fd192c1 100644 +--- a/waitress/tests/test_receiver.py ++++ b/waitress/tests/test_receiver.py +@@ -259,7 +259,7 @@ class TestChunkedReceiverParametrized: + def test_received_invalid_size(self, invalid_size): + from waitress.utilities import BadRequest + +- for invalid_size in [b"0x04", b"+0x04", b"x04", b"+04"]: ++ for invalid_size in [b"0x04", b"+0x04", b"x04", b"+04", b" 04", b" 0x04"]: + buf = DummyBuffer() + inst = self._makeOne(buf) + data = invalid_size + b"\r\ntest\r\n" +-- +2.45.2 + diff --git a/0007-Backport-security-fix-note.patch b/0007-Backport-security-fix-note.patch new file mode 100644 index 0000000..5ea6b0a --- /dev/null +++ b/0007-Backport-security-fix-note.patch @@ -0,0 +1,43 @@ +From 6e0af1e0e01f7c9a9a83431b99a82b0de5c6a5da Mon Sep 17 00:00:00 2001 +From: Carl George +Date: Tue, 25 Jun 2024 22:40:57 -0500 +Subject: [PATCH 7/8] Backport security fix note + +--- + CHANGES.txt | 23 +++++++++++++++++++++++ + 1 file changed, 23 insertions(+) + +diff --git a/CHANGES.txt b/CHANGES.txt +index 701c2b0..f9d4c42 100644 +--- a/CHANGES.txt ++++ b/CHANGES.txt +@@ -1,3 +1,26 @@ ++Security Bugfix ++~~~~~~~~~~~~~~~ ++ ++- Waitress now validates that chunked encoding extensions are valid, and don't ++ contain invalid characters that are not allowed. They are still skipped/not ++ processed, but if they contain invalid data we no longer continue in and ++ return a 400 Bad Request. This stops potential HTTP desync/HTTP request ++ smuggling. Thanks to Zhang Zeyu for reporting this issue. See ++ https://github.com/Pylons/waitress/security/advisories/GHSA-4f7p-27jc-3c36 ++ ++- Waitress now validates that the chunk length is only valid hex digits when ++ parsing chunked encoding, and values such as ``0x01`` and ``+01`` are no ++ longer supported. This stops potential HTTP desync/HTTP request smuggling. ++ Thanks to Zhang Zeyu for reporting this issue. See ++ https://github.com/Pylons/waitress/security/advisories/GHSA-4f7p-27jc-3c36 ++ ++- Waitress now validates that the Content-Length sent by a remote contains only ++ digits in accordance with RFC7230 and will return a 400 Bad Request when the ++ Content-Length header contains invalid data, such as ``+10`` which would ++ previously get parsed as ``10`` and accepted. This stops potential HTTP ++ desync/HTTP request smuggling Thanks to Zhang Zeyu for reporting this issue. See ++ https://github.com/Pylons/waitress/security/advisories/GHSA-4f7p-27jc-3c36 ++ + 1.4.3 (2020-02-02) + ------------------ + +-- +2.45.2 + diff --git a/0008-Skip-tests-that-fail-inconsistently-during-mock-build.patch b/0008-Skip-tests-that-fail-inconsistently-during-mock-build.patch new file mode 100644 index 0000000..682f997 --- /dev/null +++ b/0008-Skip-tests-that-fail-inconsistently-during-mock-build.patch @@ -0,0 +1,32 @@ +From 4f0407051486b5e01a148ca53f361dd802d88c59 Mon Sep 17 00:00:00 2001 +From: Carl George +Date: Tue, 25 Jun 2024 22:55:20 -0500 +Subject: [PATCH 8/8] Skip tests that fail inconsistently during mock build + +--- + waitress/tests/test_functional.py | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/waitress/tests/test_functional.py b/waitress/tests/test_functional.py +index a7421c6..d846d06 100644 +--- a/waitress/tests/test_functional.py ++++ b/waitress/tests/test_functional.py +@@ -1224,6 +1224,7 @@ class InternalServerErrorTests(object): + self.send_check_error(to_send) + self.assertRaises(ConnectionClosed, read_http, fp) + ++ @unittest.skip('fails inconsistently during mock build') + def test_after_write_cb(self): + to_send = "GET /after_write_cb HTTP/1.1\r\n\r\n" + to_send = tobytes(to_send) +@@ -1237,6 +1238,7 @@ class InternalServerErrorTests(object): + self.send_check_error(to_send) + self.assertRaises(ConnectionClosed, read_http, fp) + ++ @unittest.skip('fails inconsistently during mock build') + def test_in_generator(self): + to_send = "GET /in_generator HTTP/1.1\r\n\r\n" + to_send = tobytes(to_send) +-- +2.45.2 + diff --git a/python-waitress.spec b/python-waitress.spec index 7cc6241..c635d14 100644 --- a/python-waitress.spec +++ b/python-waitress.spec @@ -4,7 +4,7 @@ Name: python-%{srcname} Version: 1.4.3 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Waitress WSGI server License: ZPLv2.1 @@ -20,8 +20,21 @@ Source0: v%{version}-nodocs.tar.gz # Source1: generate-tarball.sh -# downstream only patch -Patch0: 0001-Skip-test_in_generator.patch +# https://github.com/Pylons/waitress/commit/e75b0d9afbea8a933f8f5f11d279e661cbfd676b +Patch1: 0001-Add-new-regular-expressions-for-Chunked-Encoding.patch +# https://github.com/Pylons/waitress/commit/1f6059f4c4a3a0b256b4027eda64fb9fc311b0a6 +Patch2: 0002-Be-more-strict-in-parsing-Content-Length.patch +# https://github.com/Pylons/waitress/commit/884bed167d09c3d5fdf0730e2ca2564eefdd4534 +Patch3: 0003-Update-tests-to-remove-invalid-chunked-encoding-chunk-size.patch +# https://github.com/Pylons/waitress/commit/d032a669682838b26d6a1a1b513b9da83b0e0f90 +Patch4: 0004-Error-when-receiving-back-Chunk-Extension.patch +# https://github.com/Pylons/waitress/commit/d9bdfa0cf210f6daf017d7c5a3cc149bdec8a9a7 +Patch5: 0005-Validate-chunk-size-in-Chunked-Encoding-are-HEXDIG.patch +# https://github.com/Pylons/waitress/commit/bd22869c143a3f1284f271399524676efbafa655 +Patch6: 0006-Remove-extraneous-calls-to-.strip-in-Chunked-Encoding.patch +# downstream only patches +Patch7: 0007-Backport-security-fix-note.patch +Patch8: 0008-Skip-tests-that-fail-inconsistently-during-mock-build.patch BuildArch: noarch @@ -83,6 +96,9 @@ PYTHONPATH=%{buildroot}%{python3_sitelib} nosetests-%{python3_version} %{srcname %{python3_sitelib}/%{srcname}-*.egg-info/ %changelog +* Wed Jun 26 2024 Carl George - 1.4.3-2 +- Backport upstream fix for CVE-2022-24761 rhbz#2065791 + * Wed May 10 2023 Carl George - 1.4.3-1 - Update to version 1.4.3 - Resolves: rhbz#1791421 CVE-2019-16785