fix(assets): bound every link of the filter chain, and cover the backstop
The two findings of the 18.09 PM checkpoint of `0f308c1`. Red tests landed first in `3b587ea`; this is what turns them green. BLOCKER -- `_check_inflated` read `filters[0]`, measured that one link and returned, which is not a bound: a PDF decodes a stream through a LIST of filters. Measured in paired subprocesses from two pinned trees, idle machine: [/FlateDecode] 400 MB 408 516 B 59 232 256 -> 62 017 536 B [/FlateDecode x2] 400 MB 1 636 B 886 554 624 -> 52 367 360 B [/FlateDecode x3] 400 MB 1 070 B 889 393 152 -> 61 390 848 B [/FlateDecode x2] 1,2 GB 2 927 B 2 567 204 864 -> 60 403 712 B 542 000x the file at two links, and the picture WAS refused at the end -- by `check_payload` after `get_data()`, once the memory was spent. The single-link row is the control and does not move. It also left the 16 corpus objects behind an `[/ASCII85Decode /FlateDecode]` chain unmeasured, since `filters[0]` is not `FlateDecode` there. `_check_stream_cost` walks every link. THREE CLASSES and no fourth (`extract.bounded_pdf_filters`, pinned by a test): `FlateDecode` MEASURED, a link with another expanding link behind it inflated under the same bound and handed on; `ASCII85Decode`/`ASCIIHexDecode` bounded by their own input because they SHRINK; `DCTDecode`/`JPXDecode`/`JBIG2Decode` PASS THROUGH. Everything else -- `LZWDecode`, `RunLengthDecode`, `CCITTFaxDecode`, `/Crypt`, anything written later -- is refused UNREAD with a new code `asset_pdf_unbounded`, decided before the FIRST link is decoded so a document cannot make the run pay for the links in front of the one we cannot bound. An encrypted stream is deciphered and then measured, where `stream.decipher is not None` used to return unmeasured; 0 of 5 142 objects here are in an encrypted document, which is why nothing caught it. NOT ONE PICTURE CHANGES HANDS, AND IT IS MEASURED BY NAME. Every PDF on this machine -- 78 documents, K2 in both trinn1 and trinn2, the shipped fixtures and R761 -- run through `_pdf_images` page by page from both pinned trees: images carried 9 356 -> 9 356 documents losing one 0 of 78 documents gaining one 0 of 78 asset_pdf_unsupported 322 -> 314 asset_pdf_unbounded 0 -> 8 The 8 are the 4 `CCITTFaxDecode` stencil masks (`/ImageMask true`, `/BitsPerComponent 1`), counted twice because trinn1 and trinn2 hold the same document. They were refused before and are refused now, one step earlier and under a code that says why. MAJOR -- `check_payload(len(data))` after `get_data()` is the counted refusal four documentation surfaces point at, and deleting exactly that line passed all 2 132 tests. It is reachable through a stream pdfminer has ALREADY decoded (`decode()` sets `rawdata` to `None`), which is now the ONLY case outside the bound and has a test. Eight mutations, one line each, every one DEAD, with the unmutated tree run first as the control: first-link-only, loop dropped, inequality reversed, encrypted skipped, backstop deleted, unknown filter passed through, intermediate link not carried forward, whole check removed. `tools/okf_accounting_gate.py` gains one line, the new code in `REJECTION_CODES` -- what a rejection code requires and nothing more. Gate unchanged: exit 1, GATE RED rows 2, 3, 6. Version stays 0.10.1, untagged. Suite after `git add` against a clean tree: `uv run pytest -q` -> 2152 passed, 1 skipped (226 s). ruff, ruff format --check, mypy --strict clean. Report: docs/2026-09-18-filterkjeden-og-backstoppen.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
3b587ea567
commit
0c3c4904ee
11 changed files with 564 additions and 38 deletions
|
|
@ -53,6 +53,7 @@ import re
|
|||
import struct
|
||||
import unicodedata
|
||||
import zlib
|
||||
from collections.abc import Iterator
|
||||
from dataclasses import dataclass
|
||||
|
||||
from .errors import ExtractionError
|
||||
|
|
@ -220,6 +221,31 @@ def inflated_size(raw: bytes, *, name: str, limit: int | None = None) -> int:
|
|||
reader behind it reports that in its own vocabulary, so a `zlib.error` ends
|
||||
the measurement at whatever was produced up to it.
|
||||
"""
|
||||
return sum(len(chunk) for chunk in _inflate(raw, name=name, limit=limit))
|
||||
|
||||
|
||||
def inflate_bounded(raw: bytes, *, name: str, limit: int | None = None) -> bytes:
|
||||
"""The same measurement, KEEPING the output rather than discarding it.
|
||||
|
||||
One chain link is not the end of a chain: a PDF may decode a stream through
|
||||
`/Filter [/FlateDecode /FlateDecode]`, and measuring the first link says
|
||||
nothing about the second, which is where the cost is. To bound the second
|
||||
link the first one's bytes have to exist, so this inflates under the same
|
||||
running bound and hands the result on. It refuses at the same point
|
||||
`inflated_size` does, so what is held is never more than the bound -- which
|
||||
is what separates carrying an intermediate stage from paying for a bomb.
|
||||
|
||||
Measured 2026-09-18: 400 MB of zeros deflated twice is 795 bytes of stream,
|
||||
and the first link of that chain inflates to 407 685 bytes. The whole
|
||||
expansion lives in the LAST link, which is the one nothing measured.
|
||||
"""
|
||||
return b"".join(_inflate(raw, name=name, limit=limit))
|
||||
|
||||
|
||||
def _inflate(raw: bytes, *, name: str, limit: int | None) -> Iterator[bytes]:
|
||||
"""Inflate `raw` a chunk at a time, refusing the moment the running total
|
||||
crosses the bound. The two callers differ only in whether they keep what
|
||||
comes out."""
|
||||
bound = MAX_IMAGE_BYTES if limit is None else limit
|
||||
decompressor = zlib.decompressobj()
|
||||
total = 0
|
||||
|
|
@ -236,12 +262,12 @@ def inflated_size(raw: bytes, *, name: str, limit: int | None = None) -> int:
|
|||
"container declares is a claim and this is the cost",
|
||||
code="asset_too_large",
|
||||
)
|
||||
yield produced
|
||||
pending = decompressor.unconsumed_tail
|
||||
if decompressor.eof or not pending:
|
||||
break
|
||||
except zlib.error:
|
||||
return total
|
||||
return total
|
||||
return
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
|
|
|
|||
|
|
@ -149,6 +149,15 @@ class ExtractionError(IngestError):
|
|||
statistic about the first untrue. Refused before the stream is read: a
|
||||
negative dimension multiplies to a negative pixel count, under which
|
||||
every bound reads as satisfied
|
||||
- `asset_pdf_unbounded` — the image is reached through a PDF stream filter
|
||||
whose output this package cannot measure before producing it
|
||||
(`LZWDecode`, `RunLengthDecode`, `CCITTFaxDecode`, `/Crypt`, anything
|
||||
unknown), or through an encrypted stream it cannot decipher. DISTINCT
|
||||
from `asset_too_large`, which says a measurement was taken and came out
|
||||
over the bound: this one says no measurement was possible, so the picture
|
||||
is refused UNREAD rather than decoded to find out what it costs. Measured
|
||||
2026-09-18: bounding only the first link of a filter chain let 1 636
|
||||
bytes of PDF cost 886 554 624 bytes of peak RSS
|
||||
"""
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ from .assets import (
|
|||
check_payload,
|
||||
check_size,
|
||||
encode_png,
|
||||
inflate_bounded,
|
||||
inflated_size,
|
||||
read_image,
|
||||
render_block,
|
||||
|
|
@ -1366,8 +1367,124 @@ def _pdf_alpha(attrs: dict[str, object], width: int, height: int) -> bytes | Non
|
|||
return alpha if len(alpha) >= width * height else False
|
||||
|
||||
|
||||
def _check_inflated(stream: object, name: str) -> None:
|
||||
"""Refuse a PDF image stream whose DECOMPRESSED size is over the bound.
|
||||
def bounded_pdf_filters() -> frozenset[str]:
|
||||
"""The PDF stream filters an image may be reached through, by NAME.
|
||||
|
||||
THREE CLASSES, and what separates them is whether a bound can be put on the
|
||||
output before the decoding is paid for.
|
||||
|
||||
* `FlateDecode` is MEASURED: inflated a chunk at a time with the output
|
||||
discarded, refused the moment the running total crosses the bound.
|
||||
* `ASCII85Decode` and `ASCIIHexDecode` SHRINK by construction -- five
|
||||
characters to four bytes, two to one -- so their output is bounded by
|
||||
their input, which is already in memory as part of the file. They are
|
||||
decoded here so that a `FlateDecode` BEHIND one can be measured.
|
||||
* `DCTDecode`, `JPXDecode` and `JBIG2Decode` are pass-through in pdfminer:
|
||||
it hands the compressed image on for the image reader to sniff, so the
|
||||
size does not change.
|
||||
|
||||
EVERYTHING ELSE IS REFUSED with `asset_pdf_unbounded` before any of the
|
||||
stream is decoded -- `LZWDecode`, `RunLengthDecode`, `CCITTFaxDecode`,
|
||||
`/Crypt`, and any filter written after this one. They expand by an amount
|
||||
pdfminer will only reveal by producing the whole output, so there is no
|
||||
measuring them a chunk at a time, and decoding one to find out how big it
|
||||
is IS the failure this bound exists to stop. Refusing an unknown name
|
||||
rather than passing it through is the same decision `corpus.resolve_gate`
|
||||
takes for an unknown gate name: a fallback reproduces the defect with an
|
||||
extra step.
|
||||
|
||||
The cost is measured rather than assumed. Over the 5 142 image objects of
|
||||
the 78 PDFs on this machine (2026-09-18), the filter chains are 1 654
|
||||
`[/DCTDecode]`, 2 236 `[/FlateDecode]`, 596 `[/FlateDecode /DCTDecode]`,
|
||||
580 `[/FlateDecode /ASCII85Decode]`, 40 unfiltered, 16 `[/ASCII85Decode
|
||||
/FlateDecode]`, 16 `[/JPXDecode]` and 4 `[/CCITTFaxDecode]` -- so the
|
||||
refused class is those 4 objects, which are 1-bit stencil masks
|
||||
(`/ImageMask true`, `/BitsPerComponent 1`) and were already refused one
|
||||
step later by the encoder, twice over.
|
||||
"""
|
||||
return _BOUNDED_PDF_FILTERS
|
||||
|
||||
|
||||
#: The names in `bounded_pdf_filters`, as a constant the test suite pins. The
|
||||
#: docstring above is the published claim; this is what the code enforces, and
|
||||
#: `_pdf_filter_classes` is the same three classes as pdfminer literals.
|
||||
_BOUNDED_PDF_FILTERS = frozenset(
|
||||
{
|
||||
"FlateDecode",
|
||||
"ASCII85Decode",
|
||||
"ASCIIHexDecode",
|
||||
"DCTDecode",
|
||||
"JPXDecode",
|
||||
"JBIG2Decode",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _pdf_filter_classes() -> tuple[frozenset[object], frozenset[object], frozenset[object]]:
|
||||
"""The three classes as pdfminer literals: measured, shrinking, unchanged.
|
||||
|
||||
Read from pdfminer rather than written out here, because a filter has more
|
||||
than one spelling (`/Fl` is `/FlateDecode`) and a set of names written by
|
||||
hand would refuse the abbreviation a real document uses.
|
||||
"""
|
||||
from pdfminer.pdftypes import (
|
||||
LITERALS_ASCII85_DECODE,
|
||||
LITERALS_ASCIIHEX_DECODE,
|
||||
LITERALS_DCT_DECODE,
|
||||
LITERALS_FLATE_DECODE,
|
||||
LITERALS_JBIG2_DECODE,
|
||||
LITERALS_JPX_DECODE,
|
||||
)
|
||||
|
||||
return (
|
||||
frozenset(LITERALS_FLATE_DECODE),
|
||||
frozenset(LITERALS_ASCII85_DECODE) | frozenset(LITERALS_ASCIIHEX_DECODE),
|
||||
frozenset(LITERALS_DCT_DECODE)
|
||||
| frozenset(LITERALS_JPX_DECODE)
|
||||
| frozenset(LITERALS_JBIG2_DECODE),
|
||||
)
|
||||
|
||||
|
||||
def _pdf_stream_bytes(stream: object, name: str) -> bytes | None:
|
||||
"""The stream's raw bytes, DECIPHERED where the document is encrypted.
|
||||
|
||||
`None` means there are none left to measure: pdfminer's `decode()` sets
|
||||
`rawdata` to `None`, so a stream something else has already decoded was
|
||||
paid for before this package was asked anything. That is the one path the
|
||||
backstop after `get_data()` exists for.
|
||||
|
||||
Until this commit an encrypted stream RETURNED here without being measured,
|
||||
which made "the document declares encryption" a way past the bound.
|
||||
Deciphering does not change a stream's length, so this does what pdfminer's
|
||||
own `decode()` does -- decipher first, then read the filters -- and the
|
||||
bound applies to an encrypted document exactly as it does to any other.
|
||||
"""
|
||||
raw = stream.get_rawdata() if hasattr(stream, "get_rawdata") else None
|
||||
if raw is None:
|
||||
return None
|
||||
decipher = getattr(stream, "decipher", None)
|
||||
if decipher is None:
|
||||
return bytes(raw)
|
||||
objid = getattr(stream, "objid", None)
|
||||
genno = getattr(stream, "genno", None)
|
||||
if objid is None or genno is None:
|
||||
raise ExtractionError(
|
||||
f"the encrypted stream behind {name!r} carries no object number, so its "
|
||||
"bytes cannot be deciphered before they are decoded; refused rather "
|
||||
"than decoded to find out what it costs",
|
||||
code="asset_pdf_unbounded",
|
||||
)
|
||||
try:
|
||||
return bytes(decipher(objid, genno, raw, getattr(stream, "attrs", {})))
|
||||
except Exception as exc:
|
||||
raise ExtractionError(
|
||||
f"the encrypted stream behind {name!r} could not be deciphered: {exc}",
|
||||
code="asset_pdf_unbounded",
|
||||
) from exc
|
||||
|
||||
|
||||
def _check_stream_cost(stream: object, name: str) -> None:
|
||||
"""Refuse a PDF image stream whose DECODED size is over the bound.
|
||||
|
||||
`check_size` reads `/Width` and `/Height`, which an untrusted document
|
||||
writes and which say nothing about what `get_data()` will return: `/Length`
|
||||
|
|
@ -1377,27 +1494,91 @@ def _check_inflated(stream: object, name: str) -> None:
|
|||
and cost 892 MB of peak RSS -- the same failure mode the 0.10.0 review
|
||||
named, reached through the other number.
|
||||
|
||||
WHAT THIS BOUNDS, and what it does not. The measurement runs on the RAW
|
||||
stream, so it applies where `FlateDecode` is the first filter -- the
|
||||
overwhelming case, and the one every deflate bomb needs. It does not run
|
||||
when the stream is already decoded, when the document is encrypted (the raw
|
||||
bytes are ciphertext until pdfminer deciphers them), or when a chain puts
|
||||
another filter in front. Those are caught by `check_payload` AFTER
|
||||
`get_data()`, which makes them a counted refusal rather than a bounded one:
|
||||
the memory is spent and then the picture is dropped. Stated rather than
|
||||
implied, because the difference is exactly what the review found missing.
|
||||
"""
|
||||
from pdfminer.pdftypes import LITERALS_FLATE_DECODE
|
||||
THE CHAIN, NOT ITS FIRST LINK. 0.10.1 measured `filters[0]` and returned,
|
||||
which is not a bound: a PDF decodes a stream through a LIST of filters, and
|
||||
`/Filter [/FlateDecode /FlateDecode]` puts the whole expansion in the
|
||||
second one. Measured 2026-09-18 on `0f308c1`: 1 636 bytes of file became
|
||||
886 554 624 bytes of peak RSS, and three links did the same from 1 070
|
||||
bytes -- about 542 000x the file, and the picture WAS refused at the end,
|
||||
by `check_payload`, after the memory had been spent. It also left the 16
|
||||
corpus objects behind an `[/ASCII85Decode /FlateDecode]` chain unmeasured,
|
||||
because `filters[0]` is not `FlateDecode` there.
|
||||
|
||||
if getattr(stream, "decipher", None) is not None:
|
||||
return
|
||||
raw = stream.get_rawdata() if hasattr(stream, "get_rawdata") else None
|
||||
if not raw:
|
||||
So every link is walked, in order. An unknown or unmeasurable one is
|
||||
refused BEFORE anything is decoded (`bounded_pdf_filters` says which, and
|
||||
why). A `FlateDecode` that is the last expanding link is measured and its
|
||||
output discarded, which is the common case and costs exactly what 0.10.1
|
||||
cost. A `FlateDecode` with another expanding link behind it is inflated
|
||||
under the same bound and handed on, so the link behind it can be measured
|
||||
too -- what is held is never more than the bound.
|
||||
|
||||
WHAT THIS STILL DOES NOT BOUND, stated rather than implied: a stream
|
||||
something else has already decoded (`_pdf_stream_bytes` returns `None`),
|
||||
where the memory is spent before this package is asked. That one is caught
|
||||
by `check_payload` AFTER `get_data()`, which makes it a counted refusal
|
||||
rather than a bounded one.
|
||||
"""
|
||||
flate, shrinking, pass_through = _pdf_filter_classes()
|
||||
|
||||
data = _pdf_stream_bytes(stream, name)
|
||||
if data is None:
|
||||
return
|
||||
filters = stream.get_filters() if hasattr(stream, "get_filters") else []
|
||||
if not filters or filters[0][0] not in LITERALS_FLATE_DECODE:
|
||||
return
|
||||
inflated_size(raw, name=name)
|
||||
# THE WHOLE CHAIN IS READ BEFORE THE FIRST LINK IS DECODED. A filter this
|
||||
# package cannot bound must be refused without having paid for the links in
|
||||
# front of it, which is only possible if the refusal is decided up front.
|
||||
for literal, _params in filters:
|
||||
if literal not in flate and literal not in shrinking and literal not in pass_through:
|
||||
raise ExtractionError(
|
||||
f"the image {name!r} is decoded through {literal}, a filter whose "
|
||||
"output this package cannot measure before producing it; refused "
|
||||
"unread rather than decoded to find out what it costs",
|
||||
code="asset_pdf_unbounded",
|
||||
)
|
||||
for index, (literal, params) in enumerate(filters):
|
||||
if literal in flate:
|
||||
if not any(behind in flate for behind, _ in filters[index + 1 :]):
|
||||
inflated_size(data, name=name)
|
||||
return
|
||||
if _has_predictor(params):
|
||||
raise ExtractionError(
|
||||
f"the image {name!r} applies a predictor to a link that is not the "
|
||||
"last one, so the bytes this package would hand to the next filter "
|
||||
"are not the bytes pdfminer decodes; refused unread",
|
||||
code="asset_pdf_unbounded",
|
||||
)
|
||||
data = inflate_bounded(data, name=name)
|
||||
elif literal in shrinking:
|
||||
try:
|
||||
data = _shrink(literal, data)
|
||||
except Exception:
|
||||
# Not this function's problem: a stream that is not valid input
|
||||
# for its own filter is reported by the reader behind it, in
|
||||
# that reader's vocabulary. Nothing can expand from it here.
|
||||
return
|
||||
# A pass-through filter leaves the bytes exactly as they are.
|
||||
check_payload(len(data), name=name)
|
||||
|
||||
|
||||
def _has_predictor(params: object) -> bool:
|
||||
"""Whether a `DecodeParms` entry asks for a predictor other than `1`."""
|
||||
if not isinstance(params, dict) or "Predictor" not in params:
|
||||
return False
|
||||
from pdfminer.pdftypes import resolve1
|
||||
|
||||
predictor = resolve1(params["Predictor"])
|
||||
return isinstance(predictor, int) and predictor > 1
|
||||
|
||||
|
||||
def _shrink(literal: object, data: bytes) -> bytes:
|
||||
"""The two filters whose output is smaller than their input, decoded with
|
||||
pdfminer's own readers so both sides agree on what the bytes are."""
|
||||
from pdfminer.ascii85 import ascii85decode, asciihexdecode
|
||||
from pdfminer.pdftypes import LITERALS_ASCII85_DECODE
|
||||
|
||||
if literal in LITERALS_ASCII85_DECODE:
|
||||
return ascii85decode(data)
|
||||
return asciihexdecode(data)
|
||||
|
||||
|
||||
def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
||||
|
|
@ -1433,7 +1614,7 @@ def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
|||
# colour space is not resolved until further down and one channel is the
|
||||
# LOWEST estimate of the cost -- a bound that under-counts refuses nothing
|
||||
# it should carry, and the pixel count beside it does not depend on it.
|
||||
# What that leaves unbounded is the stream, which `_check_inflated` reads
|
||||
# What that leaves unbounded is the stream, which `_check_stream_cost` reads
|
||||
# for what it actually costs rather than for what it claims.
|
||||
declared = dict(getattr(stream, "attrs", {}))
|
||||
declared_width = resolve1(declared.get("Width"))
|
||||
|
|
@ -1444,7 +1625,7 @@ def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
|||
name=name,
|
||||
channels=1,
|
||||
)
|
||||
_check_inflated(stream, name)
|
||||
_check_stream_cost(stream, name)
|
||||
try:
|
||||
data = stream.get_data() # type: ignore[attr-defined]
|
||||
except Exception as exc:
|
||||
|
|
@ -1453,7 +1634,7 @@ def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
|||
code="asset_pdf_unsupported",
|
||||
) from exc
|
||||
# THE BACKSTOP, and it is a weaker guarantee than the one above it: this
|
||||
# one COUNTS a stream that `_check_inflated` could not bound in advance
|
||||
# one COUNTS a stream that `_check_stream_cost` could not bound in advance
|
||||
# (see its docstring for which those are), after the memory has been spent.
|
||||
check_payload(len(data), name=name)
|
||||
if data and sniff(data) is not None:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue