From 85cef5113e5f9ef5ec8ded5daf823fd9fbe0ce40 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Wed, 26 Aug 2026 16:59:17 +0200 Subject: [PATCH] content: an inline image that spells an entry twice must not be read twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- content.go | 50 ++++++++++--- fuzz_test.go | 164 +++++++++++++++++++++++++++++++++++++++++ inlinecollide_test.go | 166 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 371 insertions(+), 9 deletions(-) create mode 100644 fuzz_test.go create mode 100644 inlinecollide_test.go diff --git a/content.go b/content.go index 91c4f44..fbfea6f 100644 --- a/content.go +++ b/content.go @@ -48,24 +48,56 @@ var inlineColourSpaces = map[Name]Name{ // Expanded returns the image's dictionary with the abbreviated keys and colour // space names written out, so it can be read like any image XObject. +// +// A dictionary may carry both spellings of the same entry — /BPC 8 next to +// /BitsPerComponent 4 — and one of them has to win. Which one used to depend +// on the order Go happened to walk the map in, and Go deliberately walks it in +// a different order every time: the same file, read twice by the same +// program, gave two different pictures. Worse, the expanded dictionary is what +// decides where the image's data ends, so an inline image in issue14256.pdf +// made the whole rest of the content stream tokenise differently — 58 +// operations on one run and 118 on the next, with no error either time. +// +// 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 it gives way. What matters more than the +// choice is that it is a choice, made the same way every time. func (im *InlineImage) Expanded() Dict { out := Dict{} + // The long and unknown keys first, so that an abbreviation written out + // below lands on top of the long form rather than beside it. for k, v := range im.Dict { - if long, ok := inlineKeys[k]; ok { - k = long + if _, abbreviated := inlineKeys[k]; abbreviated { + continue } - if k == "ColorSpace" { - if n, ok := ToName(v); ok { - if long, ok := inlineColourSpaces[n]; ok { - v = long - } - } + out[k] = expandColourSpace(k, v) + } + for k, v := range im.Dict { + long, abbreviated := inlineKeys[k] + if !abbreviated { + continue } - out[k] = v + out[long] = expandColourSpace(long, v) } return out } +// expandColourSpace writes out an abbreviated colour space name, which is the +// one value an inline image abbreviates as well as its key. +func expandColourSpace(k Name, v Object) Object { + if k != "ColorSpace" { + return v + } + n, ok := ToName(v) + if !ok { + return v + } + if long, ok := inlineColourSpaces[n]; ok { + return long + } + return v +} + // A ContentScanner walks a content stream one operation at a time. A stream // with rubbish in it yields the operations around the rubbish rather than // nothing at all, which is what a renderer needs; [ContentScanner.Err] reports diff --git a/fuzz_test.go b/fuzz_test.go new file mode 100644 index 0000000..79506c3 --- /dev/null +++ b/fuzz_test.go @@ -0,0 +1,164 @@ +package reader_test + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/go-pdfkit/reader" +) + +// seedDir is where an adversarial corpus can be put: point PDF_SEEDS at +// mozilla's pdf.js test suite, every file of which is there because it broke a +// reader once, and the fuzzer starts from a far better population than +// anything a generator would invent. Without it the committed seeds and the +// crashers under testdata still run — the corpus makes the search better, it +// is not what makes the test valid. +var seedDir = os.Getenv("PDF_SEEDS") + +// addSeeds feeds the fuzzer files from the corpus, capped: a seed the fuzzer +// cannot mutate quickly is a seed it will not explore. +func addSeeds(f *testing.F, max int, cap int) { + f.Add([]byte("%PDF-1.7\n1 0 obj<>endobj\ntrailer<>")) + f.Add([]byte("%PDF-1.4\nstartxref\n0\n%%EOF")) + if seedDir == "" { + return + } + ents, err := os.ReadDir(seedDir) + if err != nil { + return + } + type ent struct { + path string + size int64 + } + var list []ent + for _, e := range ents { + if e.IsDir() || filepath.Ext(e.Name()) != ".pdf" { + continue + } + info, err := e.Info() + if err != nil { + continue + } + if info.Size() > int64(cap) { + continue + } + list = append(list, ent{filepath.Join(seedDir, e.Name()), info.Size()}) + } + n := 0 + for _, e := range list { + b, err := os.ReadFile(e.path) + if err != nil { + continue + } + f.Add(b) + n++ + if n >= max { + break + } + } +} + +// budget fails the run if one input takes longer than a reader has any right +// to take. The fuzzer's own -timeout kills the whole process after ten +// minutes, which tells you nothing about which input was at fault; this names +// it. What is being hunted is a small file that costs a large amount of time, +// so the check has to be per input. +func budget(t *testing.T, limit time.Duration, size int, what string, f func()) { + t.Helper() + start := time.Now() + f() + if d := time.Since(start); d > limit { + t.Fatalf("%s: %d bytes took %s, over the %s budget", what, size, d, limit) + } +} + +func FuzzOpen(f *testing.F) { + addSeeds(f, 400, 40*1024) + f.Fuzz(func(t *testing.T, b []byte) { + budget(t, 2*time.Second, len(b), "Open", func() { + d, err := reader.Open(b) + if err != nil { + return + } + n := d.PageCount() + if n > 4 { + n = 4 + } + for i := 1; i <= n; i++ { + if _, err := d.Page(i); err != nil { + continue + } + if _, err := d.PageContent(i); err != nil { + continue + } + _, _ = d.PageOperations(i) + } + }) + }) +} + +func FuzzParseObject(f *testing.F) { + for _, s := range []string{ + "<< /A 1 /B [1 2 3] >>", "(hello \\( world)", "", "12 0 R", + "[[[[[[[[[[]]]]]]]]]]", "<>>>>>>>", "3.14", "true", + "/Name#20with#20spaces", "null", "-.0000000000000000001", + } { + f.Add([]byte(s)) + } + f.Fuzz(func(t *testing.T, b []byte) { + budget(t, time.Second, len(b), "ParseObject", func() { + o, _, err := reader.ParseObject(b) + if err == nil { + _ = reader.FormatObject(o) + } + }) + }) +} + +func FuzzOperations(f *testing.F) { + for _, s := range []string{ + "BT /F1 12 Tf (hi) Tj ET", "q 1 0 0 1 0 0 cm Q", + "BI /W 2 /H 2 /BPC 8 /CS /G ID \x00\x01\x02\x03 EI", + "[ (a) -250 (b) ] TJ", "1 2 3 4 5 6 c", + } { + f.Add([]byte(s)) + } + f.Fuzz(func(t *testing.T, b []byte) { + budget(t, time.Second, len(b), "Operations", func() { + _, _ = reader.Operations(b) + }) + }) +} + +// FuzzDecode drives the filter chain directly. The first byte picks the +// filter, so that one corpus explores all of them rather than one each. +func FuzzDecode(f *testing.F) { + names := []reader.Name{"FlateDecode", "LZWDecode", "ASCII85Decode", "ASCIIHexDecode", "RunLengthDecode"} + for i := range names { + f.Add(byte(i), byte(0), []byte("x\x9c\x03\x00\x00\x00\x00\x01")) + f.Add(byte(i), byte(2), []byte("~>")) + } + f.Fuzz(func(t *testing.T, which byte, colors byte, data []byte) { + name := names[int(which)%len(names)] + d := reader.Dict{"Filter": name} + if colors&1 != 0 { + // A predictor is where a decoder starts trusting numbers the file + // chose: rows, columns, colours and bits per component all come + // from the document, and their product is a buffer size. + d["DecodeParms"] = reader.Dict{ + "Predictor": reader.Integer(2 + int(colors)%14), + "Colors": reader.Integer(1 + int(colors>>1)%8), + "Columns": reader.Integer(1 + int(colors>>2)%64), + "BitsPerComponent": reader.Integer([]int{1, 2, 4, 8, 16}[int(colors>>3)%5]), + } + } + budget(t, 2*time.Second, len(data), "Decode", func() { + _, _, _ = reader.Decode(d, data, func(reader.Ref) (reader.Object, error) { + return nil, nil + }) + }) + }) +} diff --git a/inlinecollide_test.go b/inlinecollide_test.go new file mode 100644 index 0000000..e0d2049 --- /dev/null +++ b/inlinecollide_test.go @@ -0,0 +1,166 @@ +package reader_test + +import ( + "bytes" + "fmt" + "testing" + + "github.com/go-pdfkit/reader" +) + +// attempts is how many times a check that depends on map order is repeated. +// Go randomises the order on every walk, so one run of a two-key collision +// picks the wrong entry about half the time; two hundred runs make a defect +// that survives certain to show. +const attempts = 200 + +// TestInlineImageExpandedIsDeterministic pins down which entry wins when an +// inline image dictionary carries both spellings of the same one. +// +// issue14256.pdf in mozilla's pdf.js corpus holds images written like this — +// /BPC 8 beside /BitsPerComponent 4 — and the answer used to be whichever one +// Go's randomised map iteration reached last. The same file read twice by the +// same program gave two different pictures, and nothing reported anything. +func TestInlineImageExpandedIsDeterministic(t *testing.T) { + cases := []struct { + name string + dict reader.Dict + key reader.Name + want string + other string + }{ + { + name: "BPC beats BitsPerComponent", + dict: reader.Dict{"BPC": reader.Integer(8), "BitsPerComponent": reader.Integer(4)}, + key: "BitsPerComponent", want: "8", other: "4", + }, + { + name: "W beats Width", + dict: reader.Dict{"W": reader.Integer(20), "Width": reader.Integer(10)}, + key: "Width", want: "20", other: "10", + }, + { + name: "H beats Height", + dict: reader.Dict{"H": reader.Integer(10), "Height": reader.Integer(40)}, + key: "Height", want: "10", other: "40", + }, + { + name: "F beats Filter", + dict: reader.Dict{"F": reader.Name("AHx"), "Filter": reader.Name("A85")}, + key: "Filter", want: "/AHx", other: "/A85", + }, + { + name: "CS beats ColorSpace, and is written out", + dict: reader.Dict{"CS": reader.Name("RGB"), "ColorSpace": reader.Name("3chanRGB")}, + key: "ColorSpace", want: "/DeviceRGB", other: "/3chanRGB", + }, + { + name: "D beats Decode", + dict: reader.Dict{"D": reader.Array{reader.Integer(0), reader.Integer(1)}, + "Decode": reader.Array{reader.Integer(1), reader.Integer(0)}}, + key: "Decode", want: "[0 1]", other: "[1 0]", + }, + { + name: "I beats Interpolate", + dict: reader.Dict{"I": reader.Bool(false), "Interpolate": reader.Bool(true)}, + key: "Interpolate", want: "false", other: "true", + }, + { + name: "L beats Length", + dict: reader.Dict{"L": reader.Integer(1240), "Length": reader.Integer(99)}, + key: "Length", want: "1240", other: "99", + }, + { + name: "IM beats ImageMask", + dict: reader.Dict{"IM": reader.Bool(true), "ImageMask": reader.Bool(false)}, + key: "ImageMask", want: "true", other: "false", + }, + { + name: "DP beats DecodeParms", + dict: reader.Dict{"DP": reader.Dict{"K": reader.Integer(1)}, + "DecodeParms": reader.Dict{"K": reader.Integer(2)}}, + key: "DecodeParms", want: "<>", other: "<>", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + im := &reader.InlineImage{Dict: c.dict} + for i := 0; i < attempts; i++ { + got := string(reader.FormatObject(im.Expanded().Get(c.key))) + if got == c.other { + t.Fatalf("on attempt %d /%s came out as %s: the long spelling won, "+ + "so which entry is used depends on map order", i, c.key, got) + } + if got != c.want { + t.Fatalf("on attempt %d /%s came out as %s, want %s", i, c.key, got, c.want) + } + } + }) + } +} + +// TestInlineImageExpandedKeepsWhatItDoesNotKnow checks the two passes did not +// drop the entries neither spelling covers. +func TestInlineImageExpandedKeepsWhatItDoesNotKnow(t *testing.T) { + im := &reader.InlineImage{Dict: reader.Dict{ + "W": reader.Integer(3), "SMask": reader.Ref{Num: 7}, "Custom": reader.Name("x"), + }} + for i := 0; i < attempts; i++ { + e := im.Expanded() + if got := string(reader.FormatObject(e.Get("Width"))); got != "3" { + t.Fatalf("Width is %s", got) + } + if got := string(reader.FormatObject(e.Get("SMask"))); got != "7 0 R" { + t.Fatalf("SMask is %s", got) + } + if got := string(reader.FormatObject(e.Get("Custom"))); got != "/x" { + t.Fatalf("Custom is %s", got) + } + if len(e) != 3 { + t.Fatalf("expanded dictionary has %d entries, want 3", len(e)) + } + } +} + +// TestOperationsIsAFunctionOfItsBytes is the defect where it bites: the +// expanded dictionary says how long an inline image's data is, so an entry +// chosen at random moves the end of the image — and every operation after it +// belongs to a different stream. The count used to swing between 58 and 118 +// on the same bytes. +func TestOperationsIsAFunctionOfItsBytes(t *testing.T) { + var content bytes.Buffer + content.WriteString("q 1 0 0 1 0 0 cm\n") + // Twenty samples of two-by-two RGB at eight bits: 12 bytes, written as + // hex, is what /BPC 8 gives. /BitsPerComponent 4 would say six. + // Both spellings of the filter, naming different filters. Which one the + // expanded dictionary carried decided whether the candidate stretch of + // bytes decoded at all, and so where the image ended. + content.WriteString("BI /W 2 /H 2 /CS /RGB /BPC 8 /F [/AHx] /Filter [/A85] ID\n") + content.WriteString("00112233445566778899aabb>\nEI\n") + content.WriteString("Q\n") + for i := 0; i < 6; i++ { + content.WriteString(fmt.Sprintf("%d %d m %d %d l S\n", i, i, i+1, i+1)) + } + data := content.Bytes() + + first, err := reader.Operations(append([]byte(nil), data...)) + if err != nil { + t.Fatalf("first pass: %v", err) + } + for i := 0; i < attempts; i++ { + ops, err := reader.Operations(append([]byte(nil), data...)) + if err != nil { + t.Fatalf("attempt %d: %v", i, err) + } + if len(ops) != len(first) { + t.Fatalf("attempt %d found %d operations where the first pass found %d: "+ + "the same bytes tokenised two different ways", i, len(ops), len(first)) + } + for j := range ops { + if ops[j].Operator != first[j].Operator { + t.Fatalf("attempt %d operation %d is %q where the first pass had %q", + i, j, ops[j].Operator, first[j].Operator) + } + } + } +}