Mitigate brotli and deflate decompression bombs DoS (#7134)

Co-authored-by: Eugenio Lacuesta <eugenio.lacuesta@gmail.com>
Co-authored-by: Andrey Rakhmatullin <wrar@wrar.name>

.github/workflows/tests-ubuntu.yml

@@ -62,6 +62,9 @@ jobs:
         - python-version: "3.13"
           env:
             TOXENV: botocore
+        - python-version: "3.13"
+          env:
+            TOXENV: mitmproxy
 
     steps:
     - uses: actions/checkout@v5

conftest.py

@@ -117,6 +117,16 @@ def requires_boto3(request):
         pytest.skip("boto3 is not installed")
 
 
+@pytest.fixture(autouse=True)
+def requires_mitmproxy(request):
+    if not request.node.get_closest_marker("requires_mitmproxy"):
+        return
+    try:
+        import mitmproxy  # noqa: F401, PLC0415
+    except ImportError:
+        pytest.skip("mitmproxy is not installed")
+
+
 def pytest_configure(config):
     if config.getoption("--reactor") == "asyncio":
         # Needed on Windows to switch from proactor to selector for Twisted reactor compatibility.

pyproject.toml

@@ -232,6 +232,7 @@ markers = [
     "requires_uvloop: marks tests as only enabled when uvloop is known to be working",
     "requires_botocore: marks tests that need botocore (but not boto3)",
     "requires_boto3: marks tests that need botocore and boto3",
+    "requires_mitmproxy: marks tests that need mitmproxy",
 ]
 filterwarnings = [
     "ignore::DeprecationWarning:twisted.web.static"

scrapy/downloadermiddlewares/httpcompression.py

@@ -31,14 +31,20 @@ logger = getLogger(__name__)
 ACCEPTED_ENCODINGS: list[bytes] = [b"gzip", b"deflate"]
 
 try:
-    try:
+    import brotli
-        import brotli  # noqa: F401
-    except ImportError:
-        import brotlicffi  # noqa: F401
 except ImportError:
     pass
 else:
-    ACCEPTED_ENCODINGS.append(b"br")
+    try:
+        brotli.Decompressor.can_accept_more_data
+    except AttributeError:  # pragma: no cover
+        warnings.warn(
+            "You have brotli installed. But 'br' encoding support now requires "
+            "brotli version >= 1.2.0. Please upgrade brotli version to make Scrapy "
+            "decode 'br' encoded responses.",
+        )
+    else:
+        ACCEPTED_ENCODINGS.append(b"br")
 
 try:
     import zstandard  # noqa: F401
@@ -116,13 +122,13 @@ class HttpCompressionMiddleware:
                     decoded_body, content_encoding = self._handle_encoding(
                         response.body, content_encoding, max_size
                     )
-                except _DecompressionMaxSizeExceeded:
+                except _DecompressionMaxSizeExceeded as e:
                     raise IgnoreRequest(
                         f"Ignored response {response} because its body "
-                        f"({len(response.body)} B compressed) exceeded "
+                        f"({len(response.body)} B compressed, "
-                        f"DOWNLOAD_MAXSIZE ({max_size} B) during "
+                        f"{e.decompressed_size} B decompressed so far) exceeded "
-                        f"decompression."
+                        f"DOWNLOAD_MAXSIZE ({max_size} B) during decompression."
-                    )
+                    ) from e
                 if len(response.body) < warn_size <= len(decoded_body):
                     logger.warning(
                         f"{response} body size after decompression "
@@ -202,7 +208,7 @@ class HttpCompressionMiddleware:
             f"from unsupported encoding(s) '{encodings_str}'."
         )
         if b"br" in encodings:
-            msg += " You need to install brotli or brotlicffi to decode 'br'."
+            msg += " You need to install brotli >= 1.2.0 to decode 'br'."
         if b"zstd" in encodings:
             msg += " You need to install zstandard to decode 'zstd'."
         logger.warning(msg)

scrapy/utils/_compression.py

@@ -1,42 +1,9 @@
 import contextlib
 import zlib
 from io import BytesIO
-from warnings import warn
-
-from scrapy.exceptions import ScrapyDeprecationWarning
-
-try:
-    try:
-        import brotli
-    except ImportError:
-        import brotlicffi as brotli
-except ImportError:
-    pass
-else:
-    try:
-        brotli.Decompressor.process
-    except AttributeError:
-        warn(
-            (
-                "You have brotlipy installed, and Scrapy will use it, but "
-                "Scrapy support for brotlipy is deprecated and will stop "
-                "working in a future version of Scrapy. brotlipy itself is "
-                "deprecated, it has been superseded by brotlicffi. Please, "
-                "uninstall brotlipy and install brotli or brotlicffi instead. "
-                "brotlipy has the same import name as brotli, so keeping both "
-                "installed is strongly discouraged."
-            ),
-            ScrapyDeprecationWarning,
-        )
-
-        def _brotli_decompress(decompressor, data):
-            return decompressor.decompress(data)
-
-    else:
-
-        def _brotli_decompress(decompressor, data):
-            return decompressor.process(data)
 
+with contextlib.suppress(ImportError):
+    import brotli
 
 with contextlib.suppress(ImportError):
     import zstandard
@@ -46,62 +13,64 @@ _CHUNK_SIZE = 65536  # 64 KiB
 
 
 class _DecompressionMaxSizeExceeded(ValueError):
-    pass
+    def __init__(self, decompressed_size: int, max_size: int) -> None:
+        self.decompressed_size = decompressed_size
+        self.max_size = max_size
+
+    def __str__(self) -> str:
+        return (
+            f"The number of bytes decompressed so far "
+            f"({self.decompressed_size} B) exceeded the specified maximum "
+            f"({self.max_size} B)."
+        )
+
+
+def _check_max_size(decompressed_size: int, max_size: int) -> None:
+    if max_size and decompressed_size > max_size:
+        raise _DecompressionMaxSizeExceeded(decompressed_size, max_size)
 
 
 def _inflate(data: bytes, *, max_size: int = 0) -> bytes:
     decompressor = zlib.decompressobj()
-    raw_decompressor = zlib.decompressobj(wbits=-15)
+    try:
-    input_stream = BytesIO(data)
+        first_chunk = decompressor.decompress(data, max_length=_CHUNK_SIZE)
+    except zlib.error:
+        # to work with raw deflate content that may be sent by microsoft servers.
+        decompressor = zlib.decompressobj(wbits=-15)
+        first_chunk = decompressor.decompress(data, max_length=_CHUNK_SIZE)
+    decompressed_size = len(first_chunk)
+    _check_max_size(decompressed_size, max_size)
     output_stream = BytesIO()
-    output_chunk = b"."
+    output_stream.write(first_chunk)
-    decompressed_size = 0
+    while decompressor.unconsumed_tail:
-    while output_chunk:
+        output_chunk = decompressor.decompress(
-        input_chunk = input_stream.read(_CHUNK_SIZE)
+            decompressor.unconsumed_tail, max_length=_CHUNK_SIZE
-        try:
+        )
-            output_chunk = decompressor.decompress(input_chunk)
-        except zlib.error:
-            if decompressor != raw_decompressor:
-                # ugly hack to work with raw deflate content that may
-                # be sent by microsoft servers. For more information, see:
-                # http://carsten.codimi.de/gzip.yaws/
-                # http://www.port80software.com/200ok/archive/2005/10/31/868.aspx
-                # http://www.gzip.org/zlib/zlib_faq.html#faq38
-                decompressor = raw_decompressor
-                output_chunk = decompressor.decompress(input_chunk)
-            else:
-                raise
         decompressed_size += len(output_chunk)
-        if max_size and decompressed_size > max_size:
+        _check_max_size(decompressed_size, max_size)
-            raise _DecompressionMaxSizeExceeded(
-                f"The number of bytes decompressed so far "
-                f"({decompressed_size} B) exceed the specified maximum "
-                f"({max_size} B)."
-            )
         output_stream.write(output_chunk)
-    output_stream.seek(0)
+    if tail := decompressor.flush():
-    return output_stream.read()
+        decompressed_size += len(tail)
+        _check_max_size(decompressed_size, max_size)
+        output_stream.write(tail)
+    return output_stream.getvalue()
 
 
 def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes:
     decompressor = brotli.Decompressor()
-    input_stream = BytesIO(data)
+    first_chunk = decompressor.process(data, output_buffer_limit=_CHUNK_SIZE)
+    decompressed_size = len(first_chunk)
+    _check_max_size(decompressed_size, max_size)
     output_stream = BytesIO()
-    output_chunk = b"."
+    output_stream.write(first_chunk)
-    decompressed_size = 0
+    while not decompressor.is_finished():
-    while output_chunk:
+        output_chunk = decompressor.process(b"", output_buffer_limit=_CHUNK_SIZE)
-        input_chunk = input_stream.read(_CHUNK_SIZE)
+        if not output_chunk:
-        output_chunk = _brotli_decompress(decompressor, input_chunk)
+            break
         decompressed_size += len(output_chunk)
-        if max_size and decompressed_size > max_size:
+        _check_max_size(decompressed_size, max_size)
-            raise _DecompressionMaxSizeExceeded(
-                f"The number of bytes decompressed so far "
-                f"({decompressed_size} B) exceed the specified maximum "
-                f"({max_size} B)."
-            )
         output_stream.write(output_chunk)
-    output_stream.seek(0)
+    return output_stream.getvalue()
-    return output_stream.read()
 
 
 def _unzstd(data: bytes, *, max_size: int = 0) -> bytes:
@@ -113,12 +82,6 @@ def _unzstd(data: bytes, *, max_size: int = 0) -> bytes:
     while output_chunk:
         output_chunk = stream_reader.read(_CHUNK_SIZE)
         decompressed_size += len(output_chunk)
-        if max_size and decompressed_size > max_size:
+        _check_max_size(decompressed_size, max_size)
-            raise _DecompressionMaxSizeExceeded(
-                f"The number of bytes decompressed so far "
-                f"({decompressed_size} B) exceed the specified maximum "
-                f"({max_size} B)."
-            )
         output_stream.write(output_chunk)
-    output_stream.seek(0)
+    return output_stream.getvalue()
-    return output_stream.read()

scrapy/utils/gz.py

@@ -5,7 +5,7 @@ from gzip import GzipFile
 from io import BytesIO
 from typing import TYPE_CHECKING
 
-from ._compression import _CHUNK_SIZE, _DecompressionMaxSizeExceeded
+from ._compression import _CHUNK_SIZE, _check_max_size
 
 if TYPE_CHECKING:
     from scrapy.http import Response
@@ -31,15 +31,9 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes:
                 break
             raise
         decompressed_size += len(chunk)
-        if max_size and decompressed_size > max_size:
+        _check_max_size(decompressed_size, max_size)
-            raise _DecompressionMaxSizeExceeded(
-                f"The number of bytes decompressed so far "
-                f"({decompressed_size} B) exceed the specified maximum "
-                f"({max_size} B)."
-            )
         output_stream.write(chunk)
-    output_stream.seek(0)
+    return output_stream.getvalue()
-    return output_stream.read()
 
 
 def gzip_magic_number(response: Response) -> bool:

tests/test_downloadermiddleware_httpcompression.py

@@ -52,11 +52,10 @@ FORMAT = {
 
 def _skip_if_no_br() -> None:
     try:
-        try:
+        import brotli  # noqa: PLC0415
-            import brotli  # noqa: F401,PLC0415
+
-        except ImportError:
+        brotli.Decompressor.can_accept_more_data
-            import brotlicffi  # noqa: F401,PLC0415
+    except (ImportError, AttributeError):
-    except ImportError:
         pytest.skip("no brotli support")
 
 
@@ -153,14 +152,9 @@ class TestHttpCompression:
 
     def test_process_response_br_unsupported(self):
         try:
-            try:
+            import brotli  # noqa: F401,PLC0415
-                import brotli  # noqa: F401,PLC0415
 
-                pytest.skip("Requires not having brotli support")
+            pytest.skip("Requires not having brotli support")
-            except ImportError:
-                import brotlicffi  # noqa: F401,PLC0415
-
-                pytest.skip("Requires not having brotli support")
         except ImportError:
             pass
         response = self._getresponse("br")
@@ -179,7 +173,7 @@ class TestHttpCompression:
                 (
                     "HttpCompressionMiddleware cannot decode the response for"
                     " http://scrapytest.org/ from unsupported encoding(s) 'br'."
-                    " You need to install brotli or brotlicffi to decode 'br'."
+                    " You need to install brotli >= 1.2.0 to decode 'br'."
                 ),
             ),
         )
@@ -511,15 +505,16 @@ class TestHttpCompression:
         self.assertStatsEqual("httpcompression/response_bytes", None)
 
     def _test_compression_bomb_setting(self, compression_id):
-        settings = {"DOWNLOAD_MAXSIZE": 10_000_000}
+        settings = {"DOWNLOAD_MAXSIZE": 1_000_000}
         crawler = get_crawler(Spider, settings_dict=settings)
         spider = crawler._create_spider("scrapytest.org")
         mw = HttpCompressionMiddleware.from_crawler(crawler)
         mw.open_spider(spider)
 
-        response = self._getresponse(f"bomb-{compression_id}")
+        response = self._getresponse(f"bomb-{compression_id}")  # 11_511_612 B
-        with pytest.raises(IgnoreRequest):
+        with pytest.raises(IgnoreRequest) as exc_info:
             mw.process_response(response.request, response)
+        assert exc_info.value.__cause__.decompressed_size < 1_100_000
 
     def test_compression_bomb_setting_br(self):
         _skip_if_no_br()
@@ -539,7 +534,7 @@ class TestHttpCompression:
 
     def _test_compression_bomb_spider_attr(self, compression_id):
         class DownloadMaxSizeSpider(Spider):
-            download_maxsize = 10_000_000
+            download_maxsize = 1_000_000
 
         crawler = get_crawler(DownloadMaxSizeSpider)
         spider = crawler._create_spider("scrapytest.org")
@@ -547,8 +542,9 @@ class TestHttpCompression:
         mw.open_spider(spider)
 
         response = self._getresponse(f"bomb-{compression_id}")
-        with pytest.raises(IgnoreRequest):
+        with pytest.raises(IgnoreRequest) as exc_info:
             mw.process_response(response.request, response)
+        assert exc_info.value.__cause__.decompressed_size < 1_100_000
 
     @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning")
     def test_compression_bomb_spider_attr_br(self):
@@ -577,9 +573,10 @@ class TestHttpCompression:
         mw.open_spider(spider)
 
         response = self._getresponse(f"bomb-{compression_id}")
-        response.meta["download_maxsize"] = 10_000_000
+        response.meta["download_maxsize"] = 1_000_000
-        with pytest.raises(IgnoreRequest):
+        with pytest.raises(IgnoreRequest) as exc_info:
             mw.process_response(response.request, response)
+        assert exc_info.value.__cause__.decompressed_size < 1_100_000
 
     def test_compression_bomb_request_meta_br(self):
         _skip_if_no_br()
@@ -728,3 +725,34 @@ class TestHttpCompression:
         _skip_if_no_zstd()
 
         self._test_download_warnsize_request_meta("zstd")
+
+    def _get_truncated_response(self, compression_id):
+        crawler = get_crawler(Spider)
+        spider = crawler._create_spider("scrapytest.org")
+        mw = HttpCompressionMiddleware.from_crawler(crawler)
+        mw.open_spider(spider)
+        response = self._getresponse(compression_id)
+        truncated_body = response.body[: len(response.body) // 2]
+        response = response.replace(body=truncated_body)
+        return mw.process_response(response.request, response)
+
+    def test_process_truncated_response_br(self):
+        _skip_if_no_br()
+        resp = self._get_truncated_response("br")
+        assert resp.body.startswith(b"<!DOCTYPE")
+
+    def test_process_truncated_response_zlibdeflate(self):
+        resp = self._get_truncated_response("zlibdeflate")
+        assert resp.body.startswith(b"<!DOCTYPE")
+
+    def test_process_truncated_response_gzip(self):
+        resp = self._get_truncated_response("gzip")
+        assert resp.body.startswith(b"<!DOCTYPE")
+
+    def test_process_truncated_response_zstd(self):
+        _skip_if_no_zstd()
+        for check_key in FORMAT:
+            if not check_key.startswith("zstd-"):
+                continue
+            resp = self._get_truncated_response(check_key)
+            assert len(resp.body) == 0

tests/test_proxy_connect.py

@@ -61,6 +61,7 @@ def _wrong_credentials(proxy_url):
     return urlunsplit(bad_auth_proxy)
 
 
+@pytest.mark.requires_mitmproxy
 class TestProxyConnect:
     @classmethod
     def setup_class(cls):
@@ -72,13 +73,7 @@ class TestProxyConnect:
         cls.mockserver.__exit__(None, None, None)
 
     def setup_method(self):
-        try:
-            import mitmproxy  # noqa: F401,PLC0415
-        except ImportError:
-            pytest.skip("mitmproxy is not installed")
-
         self._oldenv = os.environ.copy()
-
         self._proxy = MitmProxy()
         proxy_url = self._proxy.start()
         os.environ["https_proxy"] = proxy_url

tox.ini

@@ -25,9 +25,6 @@ deps =
 deps =
     {[test-requirements]deps}
     pytest >= 8.4.1  # https://github.com/pytest-dev/pytest/pull/13502
-
-    # mitmproxy does not support PyPy
-    mitmproxy; implementation_name != "pypy"
 passenv =
     S3_TEST_FILE_URI
     AWS_ACCESS_KEY_ID
@@ -112,9 +109,6 @@ deps =
     w3lib==1.17.0
     zope.interface==5.1.0
     {[test-requirements]deps}
-
-    # mitmproxy 8.0.0 requires upgrading some of the pinned dependencies
-    # above, hence we do not install it in pinned environments at the moment
 setenv =
     _SCRAPY_PINNED=true
 commands =
@@ -137,8 +131,7 @@ deps =
     Twisted[http2]
     boto3
     bpython  # optional for shell wrapper tests
-    brotli; implementation_name != "pypy"  # optional for HTTP compress downloader middleware tests
+    brotli >= 1.2.0  # optional for HTTP compress downloader middleware tests
-    brotlicffi; implementation_name == "pypy"  # optional for HTTP compress downloader middleware tests
     google-cloud-storage
     ipython
     robotexclusionrulesparser
@@ -152,9 +145,7 @@ deps =
     Pillow==8.3.2
     boto3==1.20.0
     bpython==0.7.1
-    brotli==0.5.2; implementation_name != "pypy"
+    brotli==1.2.0
-    brotlicffi==0.8.0; implementation_name == "pypy"
-    brotlipy
     google-cloud-storage==1.29.0
     ipython==7.1.0
     robotexclusionrulesparser==1.6.2
@@ -254,7 +245,7 @@ deps =
     {[testenv]deps}
     botocore>=1.13.45
 commands =
-    pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=botocore.junit.xml -o junit_family=legacy -m requires_botocore}
+    pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=botocore.junit.xml -o junit_family=legacy} -m requires_botocore
 
 [testenv:botocore-pinned]
 basepython = {[pinned]basepython}
@@ -264,4 +255,17 @@ deps =
 setenv =
     {[pinned]setenv}
 commands =
-    pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=botocore-pinned.junit.xml -o junit_family=legacy -m requires_botocore}
+    pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=botocore-pinned.junit.xml -o junit_family=legacy} -m requires_botocore
+
+
+# Run proxy tests that use mitmproxy in a separate env to avoid installing
+# numerous mitmproxy deps in other envs (even in extra-deps), as they can
+# conflict with other deps we want, or don't want, to have installed there.
+
+[testenv:mitmproxy]
+deps =
+    {[testenv]deps}
+    # mitmproxy does not support PyPy
+    mitmproxy; implementation_name != "pypy"
+commands =
+    pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=botocore.junit.xml -o junit_family=legacy} -m requires_mitmproxy