From 5cf72c7d9f60e96a3ca65e410098e11d8053749d Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sat, 26 Oct 2024 22:13:08 -0600 Subject: [PATCH] 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. CVE: CVE-2024-49768 Upstream-Status: Backport [https://github.com/Pylons/waitress/commit/f4ba1c260cf17156b582c6252496213ddc96b591] Signed-off-by: Gyorgy Sarvari --- 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 eb59dd3..756adce 100644 --- a/src/waitress/channel.py +++ b/src/waitress/channel.py @@ -147,7 +147,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. @@ -203,6 +203,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)