fix(assets): a remote reference is inert and a declared size is bounded
Two MAJOR findings of the independent v0.10.0 review, both with the shipped defaults, both new in 0.10.0. Repros rebuilt as tests first. - A remote <img src>/xlink:href became a LIVE markdown image link in the persisted concept, with the address and query string chosen by whoever wrote the document. Extraction opens no socket; a consumer rendering the bundle does. Now inert text with the address in a code span, pinned by a property over the readers rather than by one string. The tier asymmetry (user-upload refuses, trusted-source persisted) went to the guard repo with the repro. - Nothing bounded a declared image size: 9.6 KB of PDF declaring 3000x3000 grayscale zeros took 83 MB peak RSS, linear in pixels. MAX_IMAGE_PIXELS (40 000 000) and MAX_IMAGE_BYTES (256 MiB) are read off the corpora (largest measured 18.6 MP on K2, 1.4 MP on R761) and checked on what the container declares, before any decompression; over them is asset_too_large, counted. The same bound closes the inline data: URI, which the review flagged and did not measure. Also fixed, added by PM to this order: an inline PDF image was named from id() of a Python object, so two concept files of the reference corpus differed between builds. It is now named from its position. R761 unchanged: 50 carried of 50 found, assets diff -rq clean. Report: docs/2026-09-17-bildestien-0-10-1.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
864570b320
commit
230d1cbccd
8 changed files with 580 additions and 3 deletions
|
|
@ -117,6 +117,58 @@ IMAGE_POINTER = re.compile(
|
|||
)
|
||||
|
||||
|
||||
#: THE SIZE A PICTURE MAY DECLARE. Read off the corpora rather than chosen:
|
||||
#: over the 4 828 image objects of the 43-document reference corpus the largest
|
||||
#: is 4 515 x 4 128 (18.6 MP, a landscape drawing), and over R761's 109
|
||||
#: delivered pictures it is 2 072 x 656 (1.4 MP). This sits an order of
|
||||
#: magnitude above both, so nothing anyone has measured is refused -- and a
|
||||
#: declared size beyond it is a COUNTED refusal instead of a killed build.
|
||||
#:
|
||||
#: The bound exists because a few kilobytes can declare an enormous picture: a
|
||||
#: 9.6 KB PDF declaring 3 000 x 3 000 grayscale zeros took 83 MB of peak RSS
|
||||
#: and a 63 KB one declaring 8 000 x 8 000 took 276 MB, linear in the pixel
|
||||
#: count, so 50 000 x 50 000 is several GB and takes the whole batch with it.
|
||||
#: Found by an independent review of 0.10.0 before it was pushed.
|
||||
MAX_IMAGE_PIXELS = 40_000_000
|
||||
|
||||
#: The same bound in bytes, for a payload whose pixels are not yet known (an
|
||||
#: inline `data:` URI) and for raw samples with several channels.
|
||||
MAX_IMAGE_BYTES = 256 * 1024 * 1024
|
||||
|
||||
|
||||
def check_size(
|
||||
width: int | None, height: int | None, *, name: str, channels: int = 1, bits: int = 8
|
||||
) -> None:
|
||||
"""Refuse a declared size beyond the bound, BEFORE anything is decoded.
|
||||
|
||||
Read off what the container DECLARES, which is the only number available
|
||||
before the cost is paid: a check after the decompression has already paid
|
||||
for the bomb it was meant to stop.
|
||||
"""
|
||||
if not width or not height:
|
||||
return
|
||||
pixels = width * height
|
||||
expected = pixels * channels * (bits // 8 or 1)
|
||||
if pixels > MAX_IMAGE_PIXELS or expected > MAX_IMAGE_BYTES:
|
||||
raise ExtractionError(
|
||||
f"the image {name!r} declares {width}x{height} = {pixels} pixels "
|
||||
f"({expected} bytes of samples), over this package's bound of "
|
||||
f"{MAX_IMAGE_PIXELS} pixels and {MAX_IMAGE_BYTES} bytes; refused "
|
||||
"unread so one picture cannot take the run with it",
|
||||
code="asset_too_large",
|
||||
)
|
||||
|
||||
|
||||
def check_payload(size: int, *, name: str) -> None:
|
||||
"""The same bound for an encoded payload of `size` bytes."""
|
||||
if size > MAX_IMAGE_BYTES:
|
||||
raise ExtractionError(
|
||||
f"the image {name!r} carries {size} encoded bytes, over this package's "
|
||||
f"bound of {MAX_IMAGE_BYTES}; refused unread",
|
||||
code="asset_too_large",
|
||||
)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ExtractedImage:
|
||||
"""One image a document carries, as this package will write it.
|
||||
|
|
@ -368,6 +420,10 @@ def encode_png(
|
|||
) -> bytes:
|
||||
"""8-bit samples as a PNG, using nothing but `zlib`.
|
||||
|
||||
Refuses a size over :data:`MAX_IMAGE_PIXELS` on its own rather than
|
||||
trusting the caller to have checked: this function is what allocates
|
||||
`width * height * channels` bytes twice over.
|
||||
|
||||
A PDF image is usually not a file: `FlateDecode` hands back raw samples
|
||||
with the colour model in the dictionary beside them, so carrying one at all
|
||||
means encoding it. Doing that with the stdlib rather than with a renderer
|
||||
|
|
@ -379,6 +435,7 @@ def encode_png(
|
|||
`channels` is 1 (grey, or indexed when `palette` is given) or 3 (RGB).
|
||||
`alpha` is one byte per pixel, from a PDF `SMask`; absent means opaque.
|
||||
"""
|
||||
check_size(width, height, name=f"{width}x{height}", channels=channels)
|
||||
if channels not in (1, 3):
|
||||
raise ExtractionError(
|
||||
f"PNG encoding supports 1 or 3 channels, not {channels}",
|
||||
|
|
@ -477,6 +534,15 @@ def render_missing(
|
|||
"""
|
||||
shown = _inline(name or "image")
|
||||
detail = f"Image: {shown} (not carried: {_inline(reason)})"
|
||||
if href and not re.search(r"[\s()\[\]]", href):
|
||||
return f"\n{detail}"
|
||||
if href:
|
||||
# INERT, never ``. A remote reference is an address the
|
||||
# document's author chose, and 0.10.0 wrote it as a live markdown
|
||||
# image: a consumer that renders the bundle, or an agent that fetches
|
||||
# what it renders, turns "this bundle was opened" into a beacon to
|
||||
# them -- and a server-side consumer into an SSRF. This package opens
|
||||
# no socket, which is not the same as the pointer being harmless.
|
||||
# The address is still STATED, in a code span, because a reader has to
|
||||
# know what stood there to judge whether the picture mattered.
|
||||
address = _inline(href).replace("`", "'")
|
||||
return f"{detail} address: `{address}`"
|
||||
return detail
|
||||
|
|
|
|||
|
|
@ -45,6 +45,8 @@ from xml.etree.ElementTree import Element
|
|||
from .assets import (
|
||||
AssetRejection,
|
||||
ExtractedImage,
|
||||
check_payload,
|
||||
check_size,
|
||||
encode_png,
|
||||
read_image,
|
||||
render_block,
|
||||
|
|
@ -473,6 +475,16 @@ class _AssetCollector:
|
|||
def _data_uri(self, match: re.Match[str], *, label: str | None) -> str:
|
||||
payload = match.group("payload")
|
||||
name = f"data-uri{Path(match.group('media').split('/')[-1] or 'bin').suffix or ''}"
|
||||
# Before decoding: base64 expands by 3/4, and a percent-encoded payload
|
||||
# by at most 1. An inline picture is small by nature, so a payload over
|
||||
# the bound is refused unread -- the same bomb class as a PDF image
|
||||
# declaring an enormous size. A refusal is a ROW, like every other.
|
||||
try:
|
||||
check_payload(
|
||||
len(payload) * 3 // 4 if match.group("base64") else len(payload), name=name
|
||||
)
|
||||
except ExtractionError as exc:
|
||||
return self.reject(name, code=exc.code, reason=str(exc), label=label)
|
||||
try:
|
||||
raw = (
|
||||
base64.b64decode(payload, validate=True)
|
||||
|
|
@ -1380,6 +1392,18 @@ def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
|||
"""
|
||||
from pdfminer.pdftypes import resolve1
|
||||
|
||||
# THE DECLARED SIZE IS READ FIRST, and the stream is not touched until it
|
||||
# is within the bound: `get_data()` decompresses, so a check after it has
|
||||
# already paid for a picture of compressed zeros.
|
||||
declared = dict(getattr(stream, "attrs", {}))
|
||||
check_size(
|
||||
resolve1(declared.get("Width")) if isinstance(resolve1(declared.get("Width")), int) else 0,
|
||||
resolve1(declared.get("Height"))
|
||||
if isinstance(resolve1(declared.get("Height")), int)
|
||||
else 0,
|
||||
name=name,
|
||||
channels=1,
|
||||
)
|
||||
try:
|
||||
data = stream.get_data() # type: ignore[attr-defined]
|
||||
except Exception as exc:
|
||||
|
|
@ -1454,7 +1478,14 @@ def _pdf_images(page: object) -> tuple[tuple[ExtractedImage, ...], tuple[AssetRe
|
|||
# resource name local to one page's dictionary, so it is not an
|
||||
# identifier -- the page number in front of it is what makes the string
|
||||
# readable, and the content-addressed digest is what makes it unique.
|
||||
label = str(drawn.get("name") or index).lstrip("/")
|
||||
# A name pdfminer derived from `id()` is not a name. An inline image
|
||||
# (`BI ... EI`) has no resource name, and pdfminer fills the field with
|
||||
# the address of a Python object, so 0.10.0 wrote a pointer line that
|
||||
# changed between two runs of one build -- two concept files of the
|
||||
# reference corpus differed, which breaks the bit-exact rebuild.
|
||||
# Measured 2026-09-17.
|
||||
raw = str(drawn.get("name") or "").lstrip("/")
|
||||
label = raw if raw and not raw.isdigit() else f"inline-{index}"
|
||||
name = f"page-{number}-{label}"
|
||||
stream = drawn.get("stream")
|
||||
if stream is None:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue