Skip to content

content: an inline image that spells an entry twice must not be read twice - #13

Open
tannevaled wants to merge 1 commit into
mainfrom
battle-fuzz
Open

content: an inline image that spells an entry twice must not be read twice#13
tannevaled wants to merge 1 commit into
mainfrom
battle-fuzz

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

Found by a robustness campaign: hashing what 21 311 corpus files say, then running it twice. Every other file gave the same answer both times.

The defect: reading the same file twice gave two different documents

InlineImage.Expanded copies an inline image's dictionary, writing the abbreviations out — /BPC becomes /BitsPerComponent, /W becomes /Width. A dictionary may carry both spellings of the same entry, and one of them has to win:

for k, v := range im.Dict {        // Go randomises this order, deliberately
	if long, ok := inlineKeys[k]; ok { k = long }
	out[k] = v                     // whichever came last wins
}

issue14256.pdf in mozilla's pdf.js corpus is exactly that file — it is in their suite for this reason. Its inline images are written like:

<</BPC 8 /BitsPerComponent 4 /CS /RGB /F [/AHx] /H 10 /Height 40 /L 1240 /W 20 /Width 10>>
<</BPC 8 /CS /RGB /F [/AHx] /Filter [/A85] /H 10 /Length 1240 /W 20>>
<</BPC 8 /CS /RGB /ColorSpace /3chanRGB /F [/AHx] /H 10 /Length 1240 /W 20>>

Why it is worse than a wrong pixel

The expanded dictionary is what inlineImageEnd uses to decide whether a candidate stretch of bytes really is the whole of the image — it carries the filters the data has to run through. Get /Filter wrong and the candidate fails to decode, so the scanner keeps looking and takes a different EI. Every operation after that belongs to a different stream.

Measured on issue14256.pdf, reader.Operations over identical bytes:

2 distinct results in 20 runs of Operations on identical bytes
   n=58   runs [0 1 2 3 4 6 7 8 9 11 12 13 14 15 17 19]
   n=118  runs [5 10 16 18]

Neither was right. With the fix it is 119 operations, 40 runs out of 40.

No error was reported on any run. This is the silent-wrongness category: the page renders differently on different runs of the same program on the same file, and nothing anywhere says so.

The fix

The abbreviation wins. It is the spelling the specification defines for an inline image, so a producer that wrote /BPC 8 meant eight; the long form is the tolerated alternative and gives way. What matters more than the choice is that it is a choice, made the same way every time.

Two passes rather than a sort: the long and unknown keys first, then the abbreviations written out on top of them.

The regression test

TestInlineImageExpandedIsDeterministic covers all ten colliding pairs (BPC/W/H/F/CS/D/I/L/IM/DP), 200 attempts each — Go randomises map order per walk, so one run of a two-key collision picks wrong about half the time and 200 make a survivor certain to show. Against the parent commit all ten subtests fail:

--- FAIL: TestInlineImageExpandedIsDeterministic/BPC_beats_BitsPerComponent
    on attempt 0 /BitsPerComponent came out as 4: the long spelling won, so which entry is used depends on map order
--- FAIL: .../W_beats_Width      ... came out as 10
--- FAIL: .../H_beats_Height     ... came out as 40
--- FAIL: .../F_beats_Filter     ... came out as /A85
--- FAIL: .../CS_beats_ColorSpace ... came out as /3chanRGB
--- FAIL: .../D_beats_Decode, I_beats_Interpolate, L_beats_Length,
          IM_beats_ImageMask, DP_beats_DecodeParms

TestOperationsIsAFunctionOfItsBytes is the end-to-end one: a content stream with an inline image naming /F [/AHx] and /Filter [/A85], tokenised 200 times. It fails 5 runs out of 5 against the parent commit and passes 5 out of 5 here.

A note on how that test was arrived at: my first version of it used a /BPC vs /BitsPerComponent collision and passed against the unfixed code, because with no /L the fallback EI scan finds the same end either way. A test that cannot fail proves nothing, so it was rebuilt around the filter collision, which is the entry that actually moves the image's end, and then verified to bite.

Corpus-wide check

The text and image metadata of 21 311 files — every page's runs with their positions, sizes, fonts and flags, the assembled text, and every image's dimensions, placement, filter and content hash — hashed before and after. One file differed, and it was this one, differing from itself.

Fuzz targets

There were none in this repository. This adds FuzzOpen, FuzzParseObject, FuzzOperations and FuzzDecode (which drives Flate/LZW/ASCII85/ASCIIHex/RunLength and the predictors, with the first byte picking the filter so one corpus explores all of them). Each enforces a per-input time budget, because the highest-value defect in this space is a small file that costs a large amount of time, and go test -fuzz's own timeout kills the process without saying which input was at fault.

They ran for a combined 47 minutes and 73 million executions without finding a crasher:

target executions time result
FuzzOpen 4 397 939 15 min clean
FuzzDecode 27 682 243 10 min clean
FuzzParseObject 14 792 666 7 min clean
FuzzOperations ~26 M 15 min clean

Point PDF_SEEDS at a corpus to seed from it; without it the built-in seeds and anything under testdata still run. (The first version skipped the whole target when the corpus was absent — which would have skipped the committed crashers too. Fixed.)

What else was looked for and not found

  • 1 311 adversarial and form files through Open, every page, every stream decoded, every font, extract, forms, ops round-trip and render: no panics, no non-termination.
  • 31 000+ arXiv figure PDFs the same way: no panics in reader.

Gates

  • go test ./... green, 100.0% of statements
  • go vet clean, gofmt clean, CGO_ENABLED=0
  • Builds for linux amd64/arm64/riscv64/loong64/ppc64le/s390x, js/wasm, darwin/arm64, windows/amd64

🤖 Generated with Claude Code

…twice

Expanded copies an inline image's dictionary, writing the abbreviations out:
/BPC becomes /BitsPerComponent, /W becomes /Width. A dictionary may carry both
spellings of the same entry, and one of them has to win. Which one depended on
the order Go happened to walk the map in, and Go deliberately walks it in a
different order every time.

So the same file, read twice by the same program, gave two different answers.
The expanded dictionary is also what says whether a candidate stretch of bytes
really is the whole of the image — it carries the filters the data has to run
through — so an inline image whose /F and /Filter named different filters
moved the end of the image, and every operation after it belonged to a
different stream. issue14256.pdf in mozilla's pdf.js corpus tokenised as 58
operations on some runs and 118 on others, with no error either time. Neither
was right: it holds 119.

The abbreviation wins now. It is the spelling the specification defines for an
inline image, so a producer that wrote /BPC 8 meant eight, and the long form
gives way. What matters more than the choice is that it is a choice, made the
same way every time.

Two passes rather than a sort: the long and unknown keys first, then the
abbreviations written out on top of them.

Found by hashing what 21 311 corpus files say and running it twice. Every
other file gave the same answer both times; this one gave four different
answers in five runs.

This commit also adds the fuzz targets. There were none in this repository.
FuzzOpen, FuzzParseObject, FuzzOperations and FuzzDecode ran for a combined
forty-seven minutes and 73 million executions without finding a crasher.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant