[[MM-70248] Bound plain text extraction to prevent heap exhaustion - #85
Closed
devinbinnie wants to merge 5 commits into
Closed
[[MM-70248] Bound plain text extraction to prevent heap exhaustion#85devinbinnie wants to merge 5 commits into
devinbinnie wants to merge 5 commits into
Conversation
Limit the recursion depth in readObject() to prevent excessive resource consumption when parsing deeply nested PDF structures. Returns an error if parsing past the limit. Co-authored-by: JG Heithcock <jgheithcock@gmail.com>
* Change module name to github.com/mattermost/pdf * Fix remaining mention to original repo
* Add context to text extraction API Add context.Context to Interpret, Page.Content, Page.GetPlainText, Reader.GetPlainText, and Reader.GetStyledTexts. Context is checked at two levels: - Per page boundary in the Reader-level functions, allowing cheap cancellation between pages. - Per PDF operator inside Interpret, allowing fine-grained cancellation mid-stream even on single large pages. Page.Content's signature changes from returning Content to (Content, error) so cancellation errors can propagate. Callers that don't need cancellation can pass context.Background(); existing internal callers (readCmap, walkTextBlocks) are updated accordingly. * Add context cancellation tests Test pre-cancelled, background, and mid-extraction cancellation for both Reader.GetPlainText and Reader.GetStyledTexts. Uses a triggerCtx helper that closes its Done channel after a configurable number of polls, enabling deterministic mid-loop cancellation without timing dependencies. * Acknowledge Interpret error in walkTextBlocks Interpret now returns an error after gaining context support. walkTextBlocks uses context.Background() so the error can never be non-nil, but leaving the return value unhandled triggers errcheck. Use a blank identifier to make the intent explicit.
* MM-69725: Add opt-in tests for problematic PDF fixtures (#4) * Additional tests for problematic PDFs * MM-69725: Address fixture test review feedback Avoid counting retained plaintext in allocation probes, preserve depth-limit errors for assertions, and align the fixture tooling defaults and failures. * MM-69725: Adversarial test fixes (#5) * Propagate ctx through lexer reload and CMap parsing The buffer.reload() method is called for every chunk read from a stream, but previously had no way to respect context cancellation. A malicious PDF with a huge ToUnicode CMap or a giant literal string token could hold the goroutine indefinitely even after the caller cancelled. Changes: - Add ctx field to buffer; reload() checks it before each read so long tokens and stream decodes stay cancellable. - Thread ctx through Interpret → buffer so PostScript streams (CMap, content) honor cancellation at the lexer level. - Add internal Font.encoder(ctx) / getEncoder(ctx) / charmapEncoding(ctx) and readCmap(ctx, …) so ToUnicode CMap parsing uses the caller's ctx instead of context.Background(). - GetPlainText and Content now call encoder(ctx) so the full extraction path is cancellable end-to-end. * Cap Predictor Columns to prevent large up-front alloc applyFilter allocates a ~2×Columns-byte buffer before any content is read. A malicious PDF with a huge Columns value (e.g. 2^31-1) would cause a multi-GB allocation before cancellation or EOF. Add maxPredictorColumns = 1<<20 (1M columns, well above any real image scanline or xref stream) and panic with a clear error if the value is out of range. Also update adversarial_pdfs/README.md to reflect the "gap under test" framing and add a probe command for manual testing. * Cap literal string tokens and fix AcroForm alloc threshold String token cap: readLiteralString and readHexString accumulate the entire token into a growing slice with no upper bound. A malicious PDF can embed a huge (...) or <...> string — either raw on disk or compressed via FlateDecode — forcing an unbounded allocation before any cancel poll fires. Add maxStringBytes = 128 MiB and panic with a clear error if either accumulator exceeds it. AcroForm test threshold: The hardcoded 8 MiB limit in TestAdversarial_AcroFormStaysCheap was calibrated for the medium-scale fixture (~2 MB, 10 K fields). At large scale (50 K fields, ~9.4 MB), xref-table parsing allocates proportional to object count, so the threshold must scale with the fixture. Use len(data) + 2 MiB instead. * Fix nested_content depth at small scale depth=200 is below maxObjectDepth=1000, so the depth-limit test never triggered at small scale. The fixture is a few KB regardless of depth, so use 1200 (same as medium/large) across all scales. * Convert Python script into Go * Remove development tools and documents * Add CI step to run adversarial tests * Run CI on push to master, and all on PRs * Enforce string byte cap on every append path maxStringBytes was only checked in readLiteralString's default branch. Escaped characters, octal escapes, and nested-parenthesis bytes all append to tmp through other branches, so a payload built from those (e.g. repeated \n escapes) could still grow the token past the cap before ever tripping the check. Move the check to run once per loop iteration, after the switch, so every append path is bounded the same way. * Make CMap range loops poll ctx for cancellation Interpret only polls ctx between tokens, but readCmap's end{codespacerange,bf{char,range}} handlers each loop n times within a single token callback, where n comes straight from an attacker-controlled int64 on the stack. A malicious ToUnicode CMap can set n arbitrarily high, turning one callback into an unbounded busy loop that no cancellation can interrupt. Check ctx.Err() on each iteration and bail out via the existing ok=false path so these loops stay cooperative with a canceled context. * Trigger adversarial tests on real stream reads Fix the outdated mention to the Python script. The four cancel-cheaply tests used a fixed 200us context timeout that, measured against unrestricted runs taking 10-340ms, was expiring before GetPlainText ever reached the fixture's adversarial content. The tests were passing by canceling before parsing started, not by exercising cancellation mid-parse. Replace the fixed timeout with contentStreamTrigger, an io.ReaderAt wrapper that cancels only after it observes a run of consecutive, sequentially-offset reads: the signature of the lexer actually streaming through a stream's bytes, as opposed to the scattered small reads used to resolve the xref table, page tree, and fonts. Cancellation latency and allocations are then measured from that confirmed point forward, giving a deterministic signal instead of one that races an arbitrary deadline against unrelated setup cost. --------- Co-authored-by: JG Heithcock <jgheithcock@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GetPlainTextaccumulated all extracted text into an in-memory buffer with no upper bound. A small PDF whoseFlateDecodecontent stream expands into a very large volume of text could therefore drive the heap far beyond the size of the file.This PR caps total accumulated text at 1 MiB, which is the same value we truncate to on the server. Once the cap is reached, extraction stops appending and cancels the interpreter's context.
Ticket Link
https://mattermost.atlassian.net/browse/MM-70248