fix(tradein/photos): защита от pixel-flood DoS в image_sanitizer
All checks were successful
CI / changes (pull_request) Successful in 7s
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 51s
All checks were successful
CI / changes (pull_request) Successful in 7s
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 51s
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.
This commit is contained in:
parent
0f4ed2f6e9
commit
1c9f0e1161
2 changed files with 157 additions and 3 deletions
|
|
@ -24,6 +24,24 @@ logger = logging.getLogger(__name__)
|
||||||
# small enough to bound storage & decode cost.
|
# small enough to bound storage & decode cost.
|
||||||
_MAX_DIMENSION = 2400
|
_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,
|
# JPEG quality. 85 is industry standard for photos — visually lossless,
|
||||||
# ~70% size reduction vs original phone JPEGs.
|
# ~70% size reduction vs original phone JPEGs.
|
||||||
_JPEG_QUALITY = 85
|
_JPEG_QUALITY = 85
|
||||||
|
|
@ -46,10 +64,28 @@ def sanitize_image(content: bytes) -> tuple[bytes, str]:
|
||||||
(sanitized_bytes, "image/jpeg") — both ready for storage/serving.
|
(sanitized_bytes, "image/jpeg") — both ready for storage/serving.
|
||||||
|
|
||||||
Raises:
|
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:
|
try:
|
||||||
with Image.open(io.BytesIO(content)) as img:
|
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
|
img.load() # force full decode; surfaces UnidentifiedImageError early
|
||||||
# Bake EXIF Orientation into pixels BEFORE stripping EXIF, else phone
|
# Bake EXIF Orientation into pixels BEFORE stripping EXIF, else phone
|
||||||
# portrait photos (Orientation=6/8) end up rotated 90/270° once the tag
|
# 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.
|
# save() does NOT carry over .info / EXIF unless we pass exif=...; we don't.
|
||||||
img.save(buf, format="JPEG", quality=_JPEG_QUALITY, optimize=True)
|
img.save(buf, format="JPEG", quality=_JPEG_QUALITY, optimize=True)
|
||||||
return buf.getvalue(), SANITIZED_CONTENT_TYPE
|
return buf.getvalue(), SANITIZED_CONTENT_TYPE
|
||||||
|
except ImageSanitizationError:
|
||||||
|
raise # already a handled, user-facing rejection (byte/pixel cap)
|
||||||
except UnidentifiedImageError as e:
|
except UnidentifiedImageError as e:
|
||||||
raise ImageSanitizationError("not a recognized image format") from 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:
|
except Exception as e:
|
||||||
# PIL can raise broad errors (DecompressionBombError, OSError on truncated, ...);
|
# PIL can raise broad errors (OSError on truncated, DecompressionBombWarning
|
||||||
# normalize to one user-facing error type.
|
# promoted to error, ...); normalize to one user-facing error type.
|
||||||
logger.warning("image sanitization failed: %s", e)
|
logger.warning("image sanitization failed: %s", e)
|
||||||
raise ImageSanitizationError(f"image decode failed: {e}") from e
|
raise ImageSanitizationError(f"image decode failed: {e}") from e
|
||||||
|
|
|
||||||
112
tradein-mvp/backend/tests/services/test_image_sanitizer.py
Normal file
112
tradein-mvp/backend/tests/services/test_image_sanitizer.py
Normal file
|
|
@ -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")
|
||||||
Loading…
Add table
Reference in a new issue