From 8c0a6cd7918719c5a636640d1c93e1609c6c6ce0 Mon Sep 17 00:00:00 2001 From: Ousret Date: Thu, 17 Nov 2022 01:40:19 +0100 Subject: [PATCH 1/2] Prevent issue in HTTPResponse().read() when decoded_content is True and then False Provided it has initialized eligible decoder(decompressor) and did decode once (cherry picked from commit cefd1dbba6a20ea4f017e6e472f9ada3a8a743e0) --- changelog/2800.bugfix.rst | 1 + src/urllib3/response.py | 12 ++++++++++++ test/test_response.py | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 changelog/2800.bugfix.rst diff --git a/changelog/2800.bugfix.rst b/changelog/2800.bugfix.rst new file mode 100644 index 00000000..9dcf1eec --- /dev/null +++ b/changelog/2800.bugfix.rst @@ -0,0 +1 @@ +Prevented issue in HTTPResponse().read() when decoded_content is True and then False. \ No newline at end of file diff --git a/src/urllib3/response.py b/src/urllib3/response.py index f945c41c..81ef4455 100644 --- a/src/urllib3/response.py +++ b/src/urllib3/response.py @@ -412,6 +412,7 @@ class HTTPResponse(io.IOBase): self.reason = reason self.strict = strict self.decode_content = decode_content + self._has_decoded_content = False self.retries = retries self.enforce_content_length = enforce_content_length self.auto_close = auto_close @@ -587,6 +588,11 @@ class HTTPResponse(io.IOBase): Decode the data passed in and potentially flush the decoder. """ if not decode_content: + if self._has_decoded_content: + raise RuntimeError( + "Calling read(decode_content=False) is not supported after " + "read(decode_content=True) was called." + ) return data if max_length is None or flush_decoder: @@ -595,6 +601,7 @@ class HTTPResponse(io.IOBase): try: if self._decoder: data = self._decoder.decompress(data, max_length=max_length) + self._has_decoded_content = True except self.DECODER_ERROR_CLASSES as e: content_encoding = self.headers.get("content-encoding", "").lower() raise DecodeError( @@ -822,6 +829,11 @@ class HTTPResponse(io.IOBase): else: # do not waste memory on buffer when not decoding if not decode_content: + if self._has_decoded_content: + raise RuntimeError( + "Calling read(decode_content=False) is not supported after " + "read(decode_content=True) was called." + ) return data decoded_data = self._decode( diff --git a/test/test_response.py b/test/test_response.py index 33f570c4..2614960f 100644 --- a/test/test_response.py +++ b/test/test_response.py @@ -710,6 +710,41 @@ class TestResponse(object): next(reader) assert re.match("I/O operation on closed file.?", str(ctx.value)) + def test_read_with_illegal_mix_decode_toggle(self): + compress = zlib.compressobj(6, zlib.DEFLATED, -zlib.MAX_WBITS) + data = compress.compress(b"foo") + data += compress.flush() + + fp = BytesIO(data) + + resp = HTTPResponse( + fp, headers={"content-encoding": "deflate"}, preload_content=False + ) + + assert resp.read(1) == b"f" + + with pytest.raises( + RuntimeError, + match=( + r"Calling read\(decode_content=False\) is not supported after " + r"read\(decode_content=True\) was called" + ), + ): + resp.read(1, decode_content=False) + + def test_read_with_mix_decode_toggle(self): + compress = zlib.compressobj(6, zlib.DEFLATED, -zlib.MAX_WBITS) + data = compress.compress(b"foo") + data += compress.flush() + + fp = BytesIO(data) + + resp = HTTPResponse( + fp, headers={"content-encoding": "deflate"}, preload_content=False + ) + resp.read(1, decode_content=False) + assert resp.read(1, decode_content=True) == b"o" + def test_streaming(self): fp = BytesIO(b"foo") resp = HTTPResponse(fp, preload_content=False) -- 2.52.0 From 868d351ffcd32b0aa30fb94db61b2dd51c6c231b Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Wed, 7 Jan 2026 18:07:30 +0200 Subject: [PATCH 2/2] Security fix for CVE-2026-21441 * Stop decoding response content during redirects needlessly * Rename the new query parameter * Add a changelog entry (cherry picked from commit 8864ac407bba8607950025e0979c4c69bc7abc7b) --- CHANGES.rst | 3 +++ dummyserver/handlers.py | 8 +++++++- src/urllib3/response.py | 6 +++++- test/with_dummyserver/test_connectionpool.py | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 943c7679..f712291e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,6 +8,9 @@ Backports compressed HTTP content ("decompression bombs") leading to excessive resource consumption even when a small amount of data was requested. Reading small chunks of compressed data is safer and much more efficient now. +- Fixed a high-severity security issue where decompression-bomb safeguards of + the streaming API were bypassed when HTTP redirects were followed. + (`GHSA-38jv-5279-wg99 `__) 1.26.19 (2024-06-17) -------------------- diff --git a/dummyserver/handlers.py b/dummyserver/handlers.py index acd181d2..dea01682 100644 --- a/dummyserver/handlers.py +++ b/dummyserver/handlers.py @@ -188,9 +188,15 @@ class TestingApp(RequestHandler): status = "%s Redirect" % status.decode("latin-1") elif isinstance(status, bytes): status = status.decode("latin-1") + compressed = request.params.get("compressed") == b"true" headers = [("Location", target)] - return Response(status=status, headers=headers) + if compressed: + headers.append(("Content-Encoding", "gzip")) + data = gzip.compress(b"foo") + else: + data = b"" + return Response(data, status=status, headers=headers) def not_found(self, request): return Response("Not found", status="404 Not Found") diff --git a/src/urllib3/response.py b/src/urllib3/response.py index 81ef4455..1357d65c 100644 --- a/src/urllib3/response.py +++ b/src/urllib3/response.py @@ -480,7 +480,11 @@ class HTTPResponse(io.IOBase): Unread data in the HTTPResponse connection blocks the connection from being released back to the pool. """ try: - self.read() + self.read( + # Do not spend resources decoding the content unless + # decoding has already been initiated. + decode_content=self._has_decoded_content, + ) except (HTTPError, SocketError, BaseSSLError, HTTPException): pass diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py index cde027b9..6e74883a 100644 --- a/test/with_dummyserver/test_connectionpool.py +++ b/test/with_dummyserver/test_connectionpool.py @@ -464,6 +464,25 @@ class TestConnectionPool(HTTPDummyServerTestCase): assert r.status == 200 assert r.data == b"Dummy server!" + @mock.patch("urllib3.response.GzipDecoder.decompress") + def test_no_decoding_with_redirect_when_preload_disabled( + self, gzip_decompress + ): + """ + Test that urllib3 does not attempt to decode a gzipped redirect + response when `preload_content` is set to `False`. + """ + with HTTPConnectionPool(self.host, self.port) as pool: + # Three requests are expected: two redirects and one final / 200 OK. + response = pool.request( + "GET", + "/redirect", + fields={"target": "/redirect?compressed=true", "compressed": "true"}, + preload_content=False, + ) + assert response.status == 200 + gzip_decompress.assert_not_called() + def test_303_redirect_makes_request_lose_body(self): with HTTPConnectionPool(self.host, self.port) as pool: response = pool.request( -- 2.52.0