parent
11cf8ee703
commit
a602f97a8b
@ -0,0 +1,126 @@
|
||||
From b3b4d0847c0b22a6f2b12090d8b6b79c4cdea95c Mon Sep 17 00:00:00 2001
|
||||
From: Bert JW Regeer <bertjw@regeer.org>
|
||||
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<extension>" + CHUNK_EXT_NAME + ")(?:=(?P<value>" + 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<name>" + TOKEN + "):" + OWS + "(?P<value>" + 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
|
||||
|
@ -1,30 +0,0 @@
|
||||
From 0278f7a9ca9e65c877a34b5c122dbdda9f78c23d Mon Sep 17 00:00:00 2001
|
||||
From: Carl George <carl@george.computer>
|
||||
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
|
||||
|
@ -0,0 +1,104 @@
|
||||
From 4105558a82b9d4fd7d68b1887dc22f6a0b627b5f Mon Sep 17 00:00:00 2001
|
||||
From: Bert JW Regeer <bertjw@regeer.org>
|
||||
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
|
||||
|
@ -0,0 +1,72 @@
|
||||
From 42bd030d29b392baed1d427916200df75f4a4a12 Mon Sep 17 00:00:00 2001
|
||||
From: Bert JW Regeer <bertjw@regeer.org>
|
||||
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
|
||||
|
@ -0,0 +1,135 @@
|
||||
From 7661d0826c9d0f197e66feed5b306b56c90255c4 Mon Sep 17 00:00:00 2001
|
||||
From: Bert JW Regeer <bertjw@regeer.org>
|
||||
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
|
||||
|
@ -0,0 +1,112 @@
|
||||
From 4f0c74f6aab47c599d33d36cd783b5fa330384d9 Mon Sep 17 00:00:00 2001
|
||||
From: Bert JW Regeer <bertjw@regeer.org>
|
||||
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
|
||||
|
@ -0,0 +1,64 @@
|
||||
From 92c5f8b8dbfc73780f8404b225b1282d58c5cd96 Mon Sep 17 00:00:00 2001
|
||||
From: Bert JW Regeer <bertjw@regeer.org>
|
||||
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
|
||||
|
@ -0,0 +1,43 @@
|
||||
From 6e0af1e0e01f7c9a9a83431b99a82b0de5c6a5da Mon Sep 17 00:00:00 2001
|
||||
From: Carl George <carlwgeorge@gmail.com>
|
||||
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
|
||||
|
@ -0,0 +1,32 @@
|
||||
From 4f0407051486b5e01a148ca53f361dd802d88c59 Mon Sep 17 00:00:00 2001
|
||||
From: Carl George <carlwgeorge@gmail.com>
|
||||
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
|
||||
|
Loading…
Reference in new issue