fix(security): bound quadratic array-stream decoding in is_pdf_too_complex (SEC-146) - #10
Conversation
…mplex (SEC-146) Source PR: Unstructured-IO#4437 Source head: c8b495b
⛔ Shipwright · BlockedRecommendation: do not merge PR #10 · Tier
Findings (9)
Fireworks usage: 20,354 input · 1,658 output · 22,012 total tokens · $0.0056 · 23s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
| # streams on the page are still inspected and charged. | ||
| try: | ||
| obj = item.get_object() if isinstance(item, IndirectObject) else item | ||
| if not hasattr(obj, "get_data"): |
There was a problem hiding this comment.
Shipwright · CRITICAL
The per-page byte cap is checked as 'len(accumulated) + len(chunk) > max_raw_stream_bytes' before extending, but 'total_raw_bytes' is incremented before this check.
Impact: The per-page byte cap is checked as 'len(accumulated) + len(chunk) > max_raw_stream_bytes' before extending, but 'total_raw_bytes' is incremented before this check. A stream that trips the per-page cap returns True immediately, so the document-wide accounting is irrelevant on that path. However, the standalone branch checks 'len(chunk) > max_raw_stream_bytes' after incrementing 'total_raw_bytes', and the array…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| if hasattr(obj, "get_data"): | ||
| raw_data = obj.get_data() | ||
| if hasattr(contents, "get_object"): | ||
| contents = contents.get_object() |
There was a problem hiding this comment.
Shipwright · CRITICAL
The document-wide entry budget is charged as 'total_array_entries += len(contents)' before iterating, but the per-page entry cap check 'if len(contents) > max_content_stream_array_
Impact: The document-wide entry budget is charged as 'total_array_entries += len(contents)' before iterating, but the per-page entry cap check 'if len(contents) > max_content_stream_array_entries' returns True before the document budget is charged. This is fine for the per-page cap, but the document budget is only charged for arrays that pass the per-page cap. A document with many pages each containing an array just unde…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| # streams on the page are still inspected and charged. | ||
| try: | ||
| obj = item.get_object() if isinstance(item, IndirectObject) else item | ||
| if not hasattr(obj, "get_data"): |
There was a problem hiding this comment.
Shipwright · CRITICAL
The per-page byte cap is enforced only after 'obj.get_data()' has fully decoded the stream into memory.
Impact: The per-page byte cap is enforced only after 'obj.get_data()' has fully decoded the stream into memory. A single crafted stream that decompresses to 900 MB will cause a 900 MB allocation before the 'len(accumulated) + len(chunk) > max_raw_stream_bytes' check can reject it. The cap bounds the accumulated buffer but not the peak memory of a single stream decode, so the CVE-2026-33123 memory-exhaustion vector is onl…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| f"entries, exceeding the limit of " | ||
| f"{max_content_stream_array_entries}. " | ||
| "Flagging PDF as too complex for text extraction." | ||
| ) |
There was a problem hiding this comment.
Shipwright · HIGH
The array branch and standalone branch duplicate the byte-budget and cap-check logic with subtle differences: the array branch checks 'len(accumulated) + len(chunk) > max_raw_strea
Impact: The array branch and standalone branch duplicate the byte-budget and cap-check logic with subtle differences: the array branch checks 'len(accumulated) + len(chunk) > max_raw_stream_bytes' before extending, while the standalone branch checks 'len(chunk) > max_raw_stream_bytes' after assigning. The array branch charges 'total_raw_bytes' before the per-page cap check, while the standalone branch also charges bef…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| f"Content-stream array entries exceed {max_total_array_entries} " | ||
| f"by page {page_index + 1}. " | ||
| "Flagging PDF as too complex for text extraction." | ||
| ) |
There was a problem hiding this comment.
Shipwright · HIGH
The 'except Exception: continue' inside the array stream loop skips a stream that raises any non-'LimitReachedError' exception, but the test 'test_is_pdf_too_complex_unreadable_str
Impact: The 'except Exception: continue' inside the array stream loop skips a stream that raises any non-'LimitReachedError' exception, but the test 'test_is_pdf_too_complex_unreadable_stream_does_not_skip_rest_of_page' only covers 'NotImplementedError'. A stream that raises 'MemoryError' or 'RecursionError' during 'get_data()' is silently skipped, and if all streams on the page raise, the page is treated as not complex and…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| if hasattr(obj, "get_data"): | ||
| raw_data = obj.get_data() | ||
| if hasattr(contents, "get_object"): | ||
| contents = contents.get_object() |
There was a problem hiding this comment.
Shipwright · HIGH
The document-wide entry budget 'max_total_array_entries' is charged as 'total_array_entries += len(contents)' before the per-page entry cap check, but the per-page cap check return
Impact: The document-wide entry budget 'max_total_array_entries' is charged as 'total_array_entries += len(contents)' before the per-page entry cap check, but the per-page cap check returns True before the document budget is charged. This means a single page with an array over the per-page cap returns True without charging the document budget, which is fail-closed and acceptable. However, the document budget is only charged…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
Summary
Fixes SEC-146.
is_pdf_too_complex()inunstructured/partition/pdf.pyre-implemented pypdf's array-based/Contentsdecoding using the same quadraticraw_data += obj.get_data()accumulation that pypdf patched under CVE-2026-33123 / GHSA-qpxp-75px-xjcp ("Inefficient decoding of array-based streams"). A crafted PDF whose page/Contentsis an array of many small stream objects could force excessive CPU/memory. This function runs on every partitioned PDF, so it sits on the untrusted-input path (pdf.py:309).Per the ticket, this keeps the intentional lightweight raw-bytes approach (added in Unstructured-IO#4268 to cheaply detect vector-heavy CAD/engineering PDFs) — it does not switch to pypdf's expensive
ContentStream. It just makes the accumulation efficient and bounded.Changes
bytearraywith.extend()instead of rebinding abytesobject (amortized O(1) vs O(n²)).max_raw_stream_bytes(50 MB) — decoded bytes per page, checked before each stream is copied so an oversized stream is never accumulated or regex-scanned.max_content_stream_array_entries(10,000, pypdf's number) — bounds an array of many empty/tiny streams that a byte cap alone misses./Contentsarray → tiny file, unbounded scan). Charged incrementally per stream so a mid-page decode error can't discard the accounting. Set far above any plausible real document and logged atwarning:max_total_stream_bytes(1 GB) — decoded bytes per document.max_total_array_entries(1,000,000) — decoded entries per document (zero-byte entries never advance the byte total but still cost a decode)./Contentsbefore the array check —DictionaryObject.get(unlike__getitem__) does not resolve references, so an indirect array of streams was silently skipping the array branch entirely (both the heuristic and the caps). 87 of 1,105 corpus pages reach their content array this way.finditerinstead offindall, so counting no longer allocates a match list proportional to stream size.pypdfto>=6.9.1(from>=6.6.2) so the library's own code path is patched too; lock resolves 6.10.0.Audit (AC4)
Grepped
unstructured/partition/pdf.pyandpdf_image/for other+=-on-stream-bytes loops — none found; this was the only instance. A separate O(n²) string-accumulation pattern inunstructured/partition/html/transformations.py(element-merge loop) is out of SEC-146's scope (not on the PDF path) and is tracked in its own ticket.Testing
Rewrote the regression tests around real
PdfWriter/PdfReaderfixtures (FlateDecode-compressed streams so the file stays small while decoded output is huge — the actual attack shape). Coverage: direct + indirect graphics-heavy arrays, a many-small-streams array (~2 MB file → ~900 MB decoded) that runs for minutes / OOMs on the pre-fix code and returns in ~0.05 s here, the entry cap, the pre-copy byte cap, the cross-page byte and entry budgets, and budget survival across a mid-page decode error. The key tests were confirmed to fail on the pre-fix code.test_unstructured/partition/pdf_image/test_pdf.py: 178 passed, 1 skippedruff check+ruff format --check: cleanAcceptance criteria
is_pdf_too_complexaccumulates viabytearray, notbytes +=+=-on-stream-bytes copies; findings fixed or ticketedpypdfdependency confirmed ≥ 6.9.1🤖 Generated with Claude Code
Source merge-base:
8c4592a136b8abfa2e0ead78a45c3bfcf29479f8Source head:
c8b495b09429bcfe5de6ae429bbf3b947f58bd4e