Fix CVE-2024-49768
(cherry picked from commit 5af4633e2f3117ae86c489674a2e0cf597f204f4)
This commit is contained in:
parent
22f017ac68
commit
3e8ab5c7fc
346
backport-CVE-2024-49768.patch
Normal file
346
backport-CVE-2024-49768.patch
Normal file
@ -0,0 +1,346 @@
|
||||
From 6943dcf556610ece2ff3cddb39e59a05ef110661 Mon Sep 17 00:00:00 2001
|
||||
From: Delta Regeer <bertjw@regeer.org>
|
||||
Date: Sat, 26 Oct 2024 22:10:36 -0600
|
||||
Subject: [PATCH 1/4] Make DummySock() look more like an actual socket
|
||||
|
||||
This forces DummySock() to look like a properly connected socket where
|
||||
there is a buffer that is read from by the remote, and a buffer that is
|
||||
written to by the remote.
|
||||
|
||||
The local side does the opposite, this way data written by the local
|
||||
side can be read by the remote without operating on the same buffer.
|
||||
---
|
||||
tests/test_channel.py | 57 +++++++++++++++++++++++++++++++++----------
|
||||
1 file changed, 44 insertions(+), 13 deletions(-)
|
||||
|
||||
diff --git a/tests/test_channel.py b/tests/test_channel.py
|
||||
index 8467ae7..7d677e9 100644
|
||||
--- a/tests/test_channel.py
|
||||
+++ b/tests/test_channel.py
|
||||
@@ -18,7 +18,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
map = {}
|
||||
inst = self._makeOne(sock, "127.0.0.1", adj, map=map)
|
||||
inst.outbuf_lock = DummyLock()
|
||||
- return inst, sock, map
|
||||
+ return inst, sock.local(), map
|
||||
|
||||
def test_ctor(self):
|
||||
inst, _, map = self._makeOneWithMap()
|
||||
@@ -218,7 +218,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
def send(_):
|
||||
return 0
|
||||
|
||||
- sock.send = send
|
||||
+ sock.remote.send = send
|
||||
|
||||
wrote = inst.write_soon(b"a")
|
||||
self.assertEqual(wrote, 1)
|
||||
@@ -236,7 +236,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
def send(_):
|
||||
return 0
|
||||
|
||||
- sock.send = send
|
||||
+ sock.remote.send = send
|
||||
|
||||
outbufs = inst.outbufs
|
||||
wrote = inst.write_soon(wrapper)
|
||||
@@ -270,7 +270,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
def send(_):
|
||||
return 0
|
||||
|
||||
- sock.send = send
|
||||
+ sock.remote.send = send
|
||||
|
||||
inst.adj.outbuf_high_watermark = 3
|
||||
inst.current_outbuf_count = 4
|
||||
@@ -286,7 +286,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
def send(_):
|
||||
return 0
|
||||
|
||||
- sock.send = send
|
||||
+ sock.remote.send = send
|
||||
|
||||
inst.adj.outbuf_high_watermark = 3
|
||||
inst.total_outbufs_len = 4
|
||||
@@ -315,7 +315,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
inst.connected = False
|
||||
raise Exception()
|
||||
|
||||
- sock.send = send
|
||||
+ sock.remote.send = send
|
||||
|
||||
inst.adj.outbuf_high_watermark = 3
|
||||
inst.total_outbufs_len = 4
|
||||
@@ -345,7 +345,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
inst.connected = False
|
||||
raise Exception()
|
||||
|
||||
- sock.send = send
|
||||
+ sock.remote.send = send
|
||||
|
||||
wrote = inst.write_soon(b"xyz")
|
||||
self.assertEqual(wrote, 3)
|
||||
@@ -376,7 +376,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
inst.total_outbufs_len = len(inst.outbufs[0])
|
||||
inst.adj.send_bytes = 1
|
||||
inst.adj.outbuf_high_watermark = 2
|
||||
- sock.send = lambda x, do_close=True: False
|
||||
+ sock.remote.send = lambda x, do_close=True: False
|
||||
inst.will_close = False
|
||||
inst.last_activity = 0
|
||||
result = inst.handle_write()
|
||||
@@ -400,7 +400,7 @@ class TestHTTPChannel(unittest.TestCase):
|
||||
|
||||
def test__flush_some_full_outbuf_socket_returns_zero(self):
|
||||
inst, sock, map = self._makeOneWithMap()
|
||||
- sock.send = lambda x: False
|
||||
+ sock.remote.send = lambda x: False
|
||||
inst.outbufs[0].append(b"abc")
|
||||
inst.total_outbufs_len = sum(len(x) for x in inst.outbufs)
|
||||
result = inst._flush_some()
|
||||
@@ -907,7 +907,8 @@ class DummySock:
|
||||
closed = False
|
||||
|
||||
def __init__(self):
|
||||
- self.sent = b""
|
||||
+ self.local_sent = b""
|
||||
+ self.remote_sent = b""
|
||||
|
||||
def setblocking(self, *arg):
|
||||
self.blocking = True
|
||||
@@ -925,14 +926,44 @@ class DummySock:
|
||||
self.closed = True
|
||||
|
||||
def send(self, data):
|
||||
- self.sent += data
|
||||
+ self.remote_sent += data
|
||||
return len(data)
|
||||
|
||||
def recv(self, buffer_size):
|
||||
- result = self.sent[:buffer_size]
|
||||
- self.sent = self.sent[buffer_size:]
|
||||
+ result = self.local_sent[:buffer_size]
|
||||
+ self.local_sent = self.local_sent[buffer_size:]
|
||||
return result
|
||||
|
||||
+ def local(self):
|
||||
+ outer = self
|
||||
+
|
||||
+ class LocalDummySock:
|
||||
+ def send(self, data):
|
||||
+ outer.local_sent += data
|
||||
+ return len(data)
|
||||
+
|
||||
+ def recv(self, buffer_size):
|
||||
+ result = outer.remote_sent[:buffer_size]
|
||||
+ outer.remote_sent = outer.remote_sent[buffer_size:]
|
||||
+ return result
|
||||
+
|
||||
+ def close(self):
|
||||
+ outer.closed = True
|
||||
+
|
||||
+ @property
|
||||
+ def sent(self):
|
||||
+ return outer.remote_sent
|
||||
+
|
||||
+ @property
|
||||
+ def closed(self):
|
||||
+ return outer.closed
|
||||
+
|
||||
+ @property
|
||||
+ def remote(self):
|
||||
+ return outer
|
||||
+
|
||||
+ return LocalDummySock()
|
||||
+
|
||||
|
||||
class DummyLock:
|
||||
notified = False
|
||||
--
|
||||
Gitee
|
||||
|
||||
|
||||
From 7e7f11e61d358ab1cb853fcadf2b46b1f00f5993 Mon Sep 17 00:00:00 2001
|
||||
From: Delta Regeer <bertjw@regeer.org>
|
||||
Date: Sat, 26 Oct 2024 22:12:14 -0600
|
||||
Subject: [PATCH 2/4] Add a new test to validate the lookahead race condition
|
||||
|
||||
---
|
||||
tests/test_channel.py | 55 ++++++++++++++++++++++++++++++++++++++++++-
|
||||
1 file changed, 54 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/tests/test_channel.py b/tests/test_channel.py
|
||||
index 7d677e9..d798091 100644
|
||||
--- a/tests/test_channel.py
|
||||
+++ b/tests/test_channel.py
|
||||
@@ -805,11 +805,12 @@ class TestHTTPChannelLookahead(TestHTTPChannel):
|
||||
)
|
||||
return [body]
|
||||
|
||||
- def _make_app_with_lookahead(self):
|
||||
+ def _make_app_with_lookahead(self, recv_bytes=8192):
|
||||
"""
|
||||
Setup a channel with lookahead and store it and the socket in self
|
||||
"""
|
||||
adj = DummyAdjustments()
|
||||
+ adj.recv_bytes = recv_bytes
|
||||
adj.channel_request_lookahead = 5
|
||||
channel, sock, map = self._makeOneWithMap(adj=adj)
|
||||
channel.server.application = self.app_check_disconnect
|
||||
@@ -901,6 +902,58 @@ class TestHTTPChannelLookahead(TestHTTPChannel):
|
||||
self.assertEqual(data.split("\r\n")[-1], "finished")
|
||||
self.assertEqual(self.request_body, b"x")
|
||||
|
||||
+ def test_lookahead_bad_request_drop_extra_data(self):
|
||||
+ """
|
||||
+ Send two requests, the first one being bad, split on the recv_bytes
|
||||
+ limit, then emulate a race that could happen whereby we read data from
|
||||
+ the socket while the service thread is cleaning up due to an error
|
||||
+ processing the request.
|
||||
+ """
|
||||
+
|
||||
+ invalid_request = [
|
||||
+ "GET / HTTP/1.1",
|
||||
+ "Host: localhost:8080",
|
||||
+ "Content-length: -1",
|
||||
+ "",
|
||||
+ ]
|
||||
+
|
||||
+ invalid_request_len = len("".join([x + "\r\n" for x in invalid_request]))
|
||||
+
|
||||
+ second_request = [
|
||||
+ "POST / HTTP/1.1",
|
||||
+ "Host: localhost:8080",
|
||||
+ "Content-Length: 1",
|
||||
+ "",
|
||||
+ "x",
|
||||
+ ]
|
||||
+
|
||||
+ full_request = invalid_request + second_request
|
||||
+
|
||||
+ self._make_app_with_lookahead(recv_bytes=invalid_request_len)
|
||||
+ self._send(*full_request)
|
||||
+ self.channel.handle_read()
|
||||
+ self.assertEqual(len(self.channel.requests), 1)
|
||||
+ self.channel.server.tasks[0].service()
|
||||
+ self.assertTrue(self.channel.close_when_flushed)
|
||||
+ # Read all of the next request
|
||||
+ self.channel.handle_read()
|
||||
+ self.channel.handle_read()
|
||||
+ # Validate that there is no more data to be read
|
||||
+ self.assertEqual(self.sock.remote.local_sent, b"")
|
||||
+ # Validate that we dropped the data from the second read, and did not
|
||||
+ # create a new request
|
||||
+ self.assertEqual(len(self.channel.requests), 0)
|
||||
+ data = self.sock.recv(256).decode("ascii")
|
||||
+ self.assertFalse(self.channel.readable())
|
||||
+ self.assertTrue(self.channel.writable())
|
||||
+
|
||||
+ # Handle the write, which will close the socket
|
||||
+ self.channel.handle_write()
|
||||
+ self.assertTrue(self.sock.closed)
|
||||
+
|
||||
+ data = self.sock.recv(256)
|
||||
+ self.assertEqual(len(data), 0)
|
||||
+
|
||||
|
||||
class DummySock:
|
||||
blocking = False
|
||||
--
|
||||
Gitee
|
||||
|
||||
|
||||
From f4ba1c260cf17156b582c6252496213ddc96b591 Mon Sep 17 00:00:00 2001
|
||||
From: Delta Regeer <bertjw@regeer.org>
|
||||
Date: Sat, 26 Oct 2024 22:13:08 -0600
|
||||
Subject: [PATCH 3/4] Fix a race condition on recv_bytes boundary when request
|
||||
is invalid
|
||||
|
||||
A remote client may send a request that is exactly recv_bytes long,
|
||||
followed by a secondary request using HTTP pipelining.
|
||||
|
||||
When request lookahead is disabled (default) we won't read any more
|
||||
requests, and when the first request fails due to a parsing error, we
|
||||
simply close the connection.
|
||||
|
||||
However when request lookahead is enabled, it is possible to process and
|
||||
receive the first request, start sending the error message back to the
|
||||
client while we read the next request and queue it. This will allow the
|
||||
secondar request to be serviced by the worker thread while the
|
||||
connection should be closed.
|
||||
|
||||
The fix here checks if we should not have read the data in the first
|
||||
place (because the conection is going to be torn down) while we hold the
|
||||
`requests_lock` which means the service thread can't be in the middle of
|
||||
flipping the `close_when_flushed` flag.
|
||||
---
|
||||
src/waitress/channel.py | 11 ++++++++++-
|
||||
1 file changed, 10 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/waitress/channel.py b/src/waitress/channel.py
|
||||
index 3860ed5..f4d9677 100644
|
||||
--- a/src/waitress/channel.py
|
||||
+++ b/src/waitress/channel.py
|
||||
@@ -140,7 +140,7 @@ class HTTPChannel(wasyncore.dispatcher):
|
||||
# 1. We're not already about to close the connection.
|
||||
# 2. We're not waiting to flush remaining data before closing the
|
||||
# connection
|
||||
- # 3. There are not too many tasks already queued
|
||||
+ # 3. There are not too many tasks already queued (if lookahead is enabled)
|
||||
# 4. There's no data in the output buffer that needs to be sent
|
||||
# before we potentially create a new task.
|
||||
|
||||
@@ -196,6 +196,15 @@ class HTTPChannel(wasyncore.dispatcher):
|
||||
return False
|
||||
|
||||
with self.requests_lock:
|
||||
+ # Don't bother processing anymore data if this connection is about
|
||||
+ # to close. This may happen if readable() returned True, on the
|
||||
+ # main thread before the service thread set the close_when_flushed
|
||||
+ # flag, and we read data but our service thread is attempting to
|
||||
+ # shut down the connection due to an error. We want to make sure we
|
||||
+ # do this while holding the request_lock so that we can't race
|
||||
+ if self.will_close or self.close_when_flushed:
|
||||
+ return False
|
||||
+
|
||||
while data:
|
||||
if self.request is None:
|
||||
self.request = self.parser_class(self.adj)
|
||||
--
|
||||
Gitee
|
||||
|
||||
|
||||
From 810a435f9e9e293bd3446a5ce2df86f59c4e7b1b Mon Sep 17 00:00:00 2001
|
||||
From: Delta Regeer <bertjw@regeer.org>
|
||||
Date: Sat, 26 Oct 2024 22:22:32 -0600
|
||||
Subject: [PATCH 4/4] Add documentation for channel_request_lookahead
|
||||
|
||||
---
|
||||
docs/arguments.rst | 14 ++++++++++++++
|
||||
1 file changed, 14 insertions(+)
|
||||
|
||||
diff --git a/docs/arguments.rst b/docs/arguments.rst
|
||||
index 0b6ca45..b8a856a 100644
|
||||
--- a/docs/arguments.rst
|
||||
+++ b/docs/arguments.rst
|
||||
@@ -314,3 +314,17 @@ url_prefix
|
||||
be stripped of the prefix.
|
||||
|
||||
Default: ``''``
|
||||
+
|
||||
+channel_request_lookahead
|
||||
+ Sets the amount of requests we can continue to read from the socket, while
|
||||
+ we are processing current requests. The default value won't allow any
|
||||
+ lookahead, increase it above ``0`` to enable.
|
||||
+
|
||||
+ When enabled this inserts a callable ``waitress.client_disconnected`` into
|
||||
+ the environment that allows the task to check if the client disconnected
|
||||
+ while waiting for the response at strategic points in the execution and to
|
||||
+ cancel the operation.
|
||||
+
|
||||
+ Default: ``0``
|
||||
+
|
||||
+ .. versionadded:: 2.0.0
|
||||
--
|
||||
Gitee
|
||||
|
||||
@ -1,11 +1,14 @@
|
||||
%global _empty_manifest_terminate_build 0
|
||||
Name: python-waitress
|
||||
Version: 2.1.2
|
||||
Release: 1
|
||||
Release: 2
|
||||
Summary: Waitress WSGI server
|
||||
License: ZPLv2.1
|
||||
URL: https://github.com/Pylons/waitress
|
||||
Source0: https://files.pythonhosted.org/packages/72/83/c3de9799e2305898b02ea67bcd125ad06f271e2a82cc86fe66b7bf4e6f63/waitress-2.1.2.tar.gz
|
||||
|
||||
Patch0001: backport-CVE-2024-49768.patch
|
||||
|
||||
BuildArch: noarch
|
||||
|
||||
Requires: python3-Sphinx
|
||||
@ -45,7 +48,7 @@ is also known to run on PyPy 3 (version 3.7 compatible python) on UNIX. It
|
||||
supports HTTP/1.0 and HTTP/1.1.
|
||||
|
||||
%prep
|
||||
%autosetup -n waitress-%{version}
|
||||
%autosetup -n waitress-%{version} -p1
|
||||
|
||||
%build
|
||||
%py3_build
|
||||
@ -85,6 +88,9 @@ mv %{buildroot}/doclist.lst .
|
||||
%{_docdir}/*
|
||||
|
||||
%changelog
|
||||
* Tue Oct 29 2024 liningjie <liningjie@xfusion.com> - 2.1.2-2
|
||||
- Fix CVE-2024-49768
|
||||
|
||||
* Wed Sep 28 2022 guozhengxin <guozhengxin@kylinos.cn> - 2.1.2-1
|
||||
- Upgrade package to version 2.1.2
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user