Compare commits
7 Commits
| Author | SHA1 | Date |
|---|---|---|
Carl George
|
096629d1b4 | 5 months ago |
Carl George
|
c14c4300c4 | 2 years ago |
|
Carl George
|
8f76e3dc95 | 2 years ago |
Troy Dawson
|
dfbd1cc0d5 | 4 years ago |
Stephen Smoogen
|
5252149144 | 5 years ago |
|
Stephen Smoogen
|
527525cd2a | 5 years ago |
Mohan Boddu
|
0975dda135 | 5 years ago |
@ -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
|
||||
|
||||
@ -1,152 +0,0 @@
|
||||
From 86a7f4d2ea10ab96a3597f64b8662fbd741e2031 Mon Sep 17 00:00:00 2001
|
||||
From: Renata Ravanelli <renata.ravanelli@gmail.com>
|
||||
Date: Fri, 15 Sep 2023 12:40:31 -0300
|
||||
Subject: [PATCH 4/6] This patch is a backport of commit: d032a66
|
||||
|
||||
From: Bert JW Regeer <bertjw@regeer.org>
|
||||
|
||||
Date: Sat, 12 Mar 2022 18:42:51 -0700
|
||||
Subject: [PATCH] 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
|
||||
|
||||
Signed-off-by: Renata Ravanelli <renata.ravanelli@gmail.com>
|
||||
---
|
||||
src/waitress/receiver.py | 11 ++++++++++-
|
||||
tests/test_functional.py | 22 ++++++++++++++++++++++
|
||||
tests/test_receiver.py | 37 +++++++++++++++++++++++++++++++++++++
|
||||
3 files changed, 69 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/waitress/receiver.py b/src/waitress/receiver.py
|
||||
index 5d1568d..106dbc7 100644
|
||||
--- a/src/waitress/receiver.py
|
||||
+++ b/src/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/tests/test_functional.py b/tests/test_functional.py
|
||||
index 7a54b22..853942c 100644
|
||||
--- a/tests/test_functional.py
|
||||
+++ b/tests/test_functional.py
|
||||
@@ -345,6 +345,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)
|
||||
+ 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 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/tests/test_receiver.py b/tests/test_receiver.py
|
||||
index b4910bb..a6261ea 100644
|
||||
--- a/tests/test_receiver.py
|
||||
+++ b/tests/test_receiver.py
|
||||
@@ -1,5 +1,7 @@
|
||||
import unittest
|
||||
|
||||
+import pytest
|
||||
+
|
||||
|
||||
class TestFixedStreamReceiver(unittest.TestCase):
|
||||
def _makeOne(self, cl, buf):
|
||||
@@ -226,6 +228,41 @@ class TestChunkedReceiver(unittest.TestCase):
|
||||
self.assertEqual(inst.error, None)
|
||||
|
||||
|
||||
+class TestChunkedReceiverParametrized:
|
||||
+ def _makeOne(self, buf):
|
||||
+ from waitress.receiver import ChunkedReceiver
|
||||
+
|
||||
+ return ChunkedReceiver(buf)
|
||||
+
|
||||
+ @pytest.mark.parametrize(
|
||||
+ "invalid_extension", [b"\n", b"invalid=", b"\r", b"invalid = true"]
|
||||
+ )
|
||||
+ def test_received_invalid_extensions(self, invalid_extension):
|
||||
+ from waitress.utilities import BadRequest
|
||||
+
|
||||
+ 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"
|
||||
+
|
||||
+ @pytest.mark.parametrize(
|
||||
+ "valid_extension", [b"test", b"valid=true", b"valid=true;other=true"]
|
||||
+ )
|
||||
+ def test_received_valid_extensions(self, valid_extension):
|
||||
+ # 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
|
||||
+ 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.39.2 (Apple Git-143)
|
||||
|
||||
@ -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
|
||||
|
||||
@ -1 +1 @@
|
||||
SHA512 (v1.4.4-nodocs.tar.gz) = 246e066774f093caf174c2e7a054fedf9d09ce871524f6fdfd86bade89b858ff28ea0fd7347874303e473bf2527919beecc174264d5d8283030ab13c5942ef2d
|
||||
SHA512 (v1.4.3-nodocs.tar.gz) = c3749376e97d864874b1976b7f9f2688d3b55c56e33a01d968fc59a068a27ea14dd389d8ca4feb211afbfd0bb6848f6b8d483142e0b7a1b403f924fb7cb87f3c
|
||||
|
||||
Loading…
Reference in new issue