From 1c9f0e116106264c8bda1e811db99429ba11a88c Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sun, 12 Jul 2026 22:12:12 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/photos):=20=D0=B7=D0=B0=D1=89=D0=B8?= =?UTF-8?q?=D1=82=D0=B0=20=D0=BE=D1=82=20pixel-flood=20DoS=20=D0=B2=20imag?= =?UTF-8?q?e=5Fsanitizer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sanitize_image декодировал изображение целиком (img.load) ДО ресайза, а Pillow MAX_IMAGE_PIXELS оставался дефолтным (~89 MP). Хорошо сжимаемый ≤10 MB аплоад с заявленными ~89-178 MP разворачивался в raw RGB буфер на сотни MB до thumbnail — OOM-kill backend'а (768 MiB, #2214), усиление ×12 слотов на estimate. Defense in depth: - _MAX_PIXELS = 40 MP: проверка img.size (из хедера, без декода) ДО img.load; превышение → ImageSanitizationError без загрузки полного буфера. - Image.MAX_IMAGE_PIXELS понижен до 40 MP — Pillow сам бросает DecompressionBombError на любом декод-пути в обход явной проверки. - Явный except Image.DecompressionBombError + re-raise ImageSanitizationError (byte/pixel cap) до широкого except — грациозный reject, не 500. - _MAX_BYTES backstop (endpoint уже режет 10 MB → 413, но сервис standalone). Контракт caller'а не меняется: reject → ImageSanitizationError → HTTP 400. Тест tests/services/test_image_sanitizer.py: happy-path, resize, pixel-flood без декода (load не вызывается), byte-backstop, граница cap, garbage bytes. --- .../backend/app/services/image_sanitizer.py | 48 +++++++- .../tests/services/test_image_sanitizer.py | 112 ++++++++++++++++++ 2 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 tradein-mvp/backend/tests/services/test_image_sanitizer.py diff --git a/tradein-mvp/backend/app/services/image_sanitizer.py b/tradein-mvp/backend/app/services/image_sanitizer.py index 24816332..73748361 100644 --- a/tradein-mvp/backend/app/services/image_sanitizer.py +++ b/tradein-mvp/backend/app/services/image_sanitizer.py @@ -24,6 +24,24 @@ logger = logging.getLogger(__name__) # small enough to bound storage & decode cost. _MAX_DIMENSION = 2400 +# Hard cap on declared pixel count (width*height) enforced BEFORE any decode. +# A highly-compressible ≤10 MB upload can declare ~89-178 MP; once img.load() +# inflates it to raw RGB (3 B/px) that is a 260-530 MB buffer (+ transpose/convert +# copies) → OOM-kills the 768 MiB backend (#2214), amplified ×12 photo slots. +# 40 MP (~8000×5000) sits far above any real listing photo yet well below the +# ~89 MP default Pillow bomb threshold. img.size is read from the header on +# open() without decoding pixels, so this check is cheap and pre-decode. +_MAX_PIXELS = 40_000_000 + +# Backstop byte cap (the upload endpoint already enforces 10 MB → 413, but this +# service is standalone; keep it self-defending against oversized inputs). +_MAX_BYTES = 10 * 1024 * 1024 + +# Belt-and-braces: lower Pillow's own DecompressionBombError threshold from its +# ~89 MP default to our cap, so oversized declared dimensions raise even on any +# decode path that bypasses the explicit size guard below. +Image.MAX_IMAGE_PIXELS = _MAX_PIXELS + # JPEG quality. 85 is industry standard for photos — visually lossless, # ~70% size reduction vs original phone JPEGs. _JPEG_QUALITY = 85 @@ -46,10 +64,28 @@ def sanitize_image(content: bytes) -> tuple[bytes, str]: (sanitized_bytes, "image/jpeg") — both ready for storage/serving. Raises: - ImageSanitizationError: bytes are not a recognized image format. + ImageSanitizationError: bytes are not a recognized image format, or the + declared dimensions / byte size exceed the pixel-flood safety caps. """ + if len(content) > _MAX_BYTES: + raise ImageSanitizationError(f"image too large: {len(content)} bytes exceeds cap") try: with Image.open(io.BytesIO(content)) as img: + # #2214: reject pixel-flood bombs BEFORE img.load(). img.size comes + # from the header (no pixel decode); loading an oversized declared + # image would inflate a small file into a hundreds-of-MB RGB buffer. + width, height = img.size + if width * height > _MAX_PIXELS: + logger.warning( + "image rejected: %dx%d (%d px) exceeds %d px cap", + width, + height, + width * height, + _MAX_PIXELS, + ) + raise ImageSanitizationError( + f"image too large: {width}x{height} exceeds {_MAX_PIXELS} px cap" + ) img.load() # force full decode; surfaces UnidentifiedImageError early # Bake EXIF Orientation into pixels BEFORE stripping EXIF, else phone # portrait photos (Orientation=6/8) end up rotated 90/270° once the tag @@ -65,10 +101,16 @@ def sanitize_image(content: bytes) -> tuple[bytes, str]: # save() does NOT carry over .info / EXIF unless we pass exif=...; we don't. img.save(buf, format="JPEG", quality=_JPEG_QUALITY, optimize=True) return buf.getvalue(), SANITIZED_CONTENT_TYPE + except ImageSanitizationError: + raise # already a handled, user-facing rejection (byte/pixel cap) except UnidentifiedImageError as e: raise ImageSanitizationError("not a recognized image format") from e + except Image.DecompressionBombError as e: + # Any decode path that outran the size guard above (e.g. >2× cap) lands here. + logger.warning("decompression bomb rejected: %s", e) + raise ImageSanitizationError("image dimensions exceed safe limits") from e except Exception as e: - # PIL can raise broad errors (DecompressionBombError, OSError on truncated, ...); - # normalize to one user-facing error type. + # PIL can raise broad errors (OSError on truncated, DecompressionBombWarning + # promoted to error, ...); normalize to one user-facing error type. logger.warning("image sanitization failed: %s", e) raise ImageSanitizationError(f"image decode failed: {e}") from e diff --git a/tradein-mvp/backend/tests/services/test_image_sanitizer.py b/tradein-mvp/backend/tests/services/test_image_sanitizer.py new file mode 100644 index 00000000..e313a8b5 --- /dev/null +++ b/tradein-mvp/backend/tests/services/test_image_sanitizer.py @@ -0,0 +1,112 @@ +"""Unit tests for `app.services.image_sanitizer.sanitize_image`. + +Coverage: +- happy path: a small RGBA PNG re-encodes to a clean JPEG (RGB, EXIF stripped) +- resize path: an image above _MAX_DIMENSION is thumbnailed down, still decodes +- pixel-flood DoS: an image declaring > _MAX_PIXELS is rejected via img.size + BEFORE img.load() is ever called (no full decode → no OOM) — #2214 +- byte-size backstop: content over _MAX_BYTES rejected before Image.open +- Pillow's own bomb threshold (MAX_IMAGE_PIXELS) is lowered to our cap +- garbage bytes still raise ImageSanitizationError (regression guard) + +The oversized-dimension case is asserted via monkeypatch rather than a real +100 MP buffer: allocating the bomb is exactly the OOM we are defending against, +so the test proves img.load() is NOT reached. +""" + +from __future__ import annotations + +import io +import os +from unittest.mock import MagicMock + +import pytest + +# DATABASE_URL required by config before any app import (см. sibling service tests). +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from PIL import Image + +from app.services import image_sanitizer +from app.services.image_sanitizer import ( + SANITIZED_CONTENT_TYPE, + ImageSanitizationError, + sanitize_image, +) + + +def _png_bytes(width: int, height: int, mode: str = "RGBA") -> bytes: + buf = io.BytesIO() + Image.new(mode, (width, height), color=(120, 60, 30, 255)[: len(mode)]).save(buf, format="PNG") + return buf.getvalue() + + +def test_small_image_passes_and_becomes_jpeg() -> None: + out, ctype = sanitize_image(_png_bytes(100, 80)) + assert ctype == SANITIZED_CONTENT_TYPE + # Output is a valid JPEG, decoded back in RGB (transparency dropped). + reloaded = Image.open(io.BytesIO(out)) + assert reloaded.format == "JPEG" + assert reloaded.mode == "RGB" + assert reloaded.size == (100, 80) + + +def test_oversized_but_under_pixel_cap_is_resized() -> None: + # 3000x2000 = 6 MP (< 40 MP cap) but longest edge > _MAX_DIMENSION → thumbnailed. + out, _ = sanitize_image(_png_bytes(3000, 2000, mode="RGB")) + reloaded = Image.open(io.BytesIO(out)) + assert max(reloaded.size) == image_sanitizer._MAX_DIMENSION + assert reloaded.size == (2400, 1600) + + +def test_pixel_flood_rejected_without_decode(monkeypatch) -> None: + """Image declaring > cap pixels is rejected via img.size, never img.load().""" + fake_img = MagicMock() + fake_img.size = (10_000, 10_000) # 100 MP >> 40 MP cap + fake_img.__enter__.return_value = fake_img + fake_img.__exit__.return_value = False + monkeypatch.setattr(image_sanitizer.Image, "open", lambda *a, **k: fake_img) + + with pytest.raises(ImageSanitizationError) as excinfo: + sanitize_image(b"\x89PNG\r\n\x1a\n-tiny-header") + + assert "exceeds" in str(excinfo.value) + # The whole point of the fix: no full decode happened → no hundreds-of-MB buffer. + fake_img.load.assert_not_called() + + +def test_pixel_cap_boundary_allows_at_cap(monkeypatch) -> None: + """Exactly _MAX_PIXELS is allowed through to load() (strict '>' comparison).""" + fake_img = MagicMock() + fake_img.size = (image_sanitizer._MAX_PIXELS, 1) # width*height == cap + fake_img.mode = "RGB" + fake_img.__enter__.return_value = fake_img + fake_img.__exit__.return_value = False + monkeypatch.setattr(image_sanitizer.Image, "open", lambda *a, **k: fake_img) + monkeypatch.setattr(image_sanitizer.ImageOps, "exif_transpose", lambda im: im) + + sanitize_image(b"whatever") + fake_img.load.assert_called_once() + + +def test_byte_size_backstop_rejects_before_open(monkeypatch) -> None: + called = {"open": False} + + def _guard(*_a, **_k): + called["open"] = True + raise AssertionError("Image.open must not be called for oversized bytes") + + monkeypatch.setattr(image_sanitizer.Image, "open", _guard) + with pytest.raises(ImageSanitizationError): + sanitize_image(b"\x00" * (image_sanitizer._MAX_BYTES + 1)) + assert called["open"] is False + + +def test_pillow_bomb_threshold_lowered_to_cap() -> None: + assert image_sanitizer._MAX_PIXELS == 40_000_000 + assert Image.MAX_IMAGE_PIXELS == image_sanitizer._MAX_PIXELS + + +def test_garbage_bytes_rejected() -> None: + with pytest.raises(ImageSanitizationError): + sanitize_image(b"definitely not an image") -- 2.45.3