diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d2b616c..c9381cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,6 +63,17 @@ jobs: fi echo "all statements covered (100.0%)" + # The seeds run above as ordinary subtests; this is the half that + # explores. A minute each found both of jpegInfo's divergences from + # image/jpeg within the first second, so a minute is not a formality. + # A new crasher is written under testdata/fuzz and fails this step. + - name: Fuzz the parsers + run: | + for target in FuzzJPEGInfo FuzzDrawImageSamples; do + echo "== $target ==" + go test -run XXX -fuzz "$target" -fuzztime 60s . + done + - name: Cross-compile (6 64-bit arches + wasm + darwin + windows) run: | for arch in amd64 arm64 riscv64 loong64 ppc64le s390x; do @@ -72,3 +83,22 @@ jobs: GOOS=js GOARCH=wasm go build ./... GOOS=darwin GOARCH=arm64 go build ./... GOOS=windows GOARCH=amd64 go build ./... + + vuln: + name: govulncheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-go@v7 + with: + go-version: "1.27.3" + check-latest: true + cache: true + + # This lane never had one. pdfkit parses fonts and image headers out of + # files, so a known vulnerability in anything it links is a defect in it. + - name: govulncheck + run: | + go install golang.org/x/vuln/cmd/govulncheck@latest + govulncheck ./... diff --git a/README.md b/README.md index d4a39fd..405f49c 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,11 @@ Go-idiomatic rather than a gem port. - **Images** — JPEG embedded directly (DCTDecode); PNG and any `image.Image` rasterised as XObjects (FlateDecode) with an `/SMask` for alpha, the colour samples written straight (not premultiplied), as a soft mask without - `/Matte` requires. + `/Matte` requires. The JPEG marker walk reads frame geometry the way libjpeg + and `image/jpeg` do — skipping fill bytes, `X'FF00'` stuffing and extraneous + data between segments — because `DrawJPEG` embeds the bytes untouched, and a + writer that refuses what every reader of its own output accepts is the one + in the wrong. **Pixel-identical bitmaps are shared**: each image is content-addressed by its *uncompressed* samples, so a repeat is embedded — and compressed — once per document and every placement, on any page, references the one XObject. @@ -178,6 +182,23 @@ smaller than the whole `CFF` table and re-parsed so each kept glyph still render intact. Tests are deterministic and network-free: they use a synthesised TrueType font, a synthesised CFF2 font and a bundled OFL OpenType/CFF font. +**Fuzzing.** `jpegInfo` is the one parser here that reads caller-supplied bytes +by hand — markers, segment lengths and offsets, every one a number out of the +file — so `FuzzJPEGInfo` drives it against `image/jpeg` as a differential +oracle, and `FuzzDrawImageSamples` drives the sample splitter's premultiply +arithmetic. The seed corpus is committed under `testdata/fuzz/`; CI runs the +seeds on every pull request and fuzzes each target for a minute. + +⛔ The oracle is not "the specification" but *what every consumer of our output +will have used to read the same bytes*. The property is therefore **not** that +`jpegInfo` accepts what `image/jpeg` accepts — it accepts more, deliberately, +since `DrawJPEG` embeds without decoding — but that the two never **agree to +disagree**: a file both read must have the same dimensions in both, or the PDF +carries a `/Width` the reader's own decoder contradicts. That found two real +divergences in the first second of fuzzing, both of them files +`jpegInfo` refused and every other decoder accepts. See +*Scope and limitations*. + ## Scope and limitations - Both outline flavours are **subsetted**: TrueType `glyf` fonts via diff --git a/fuzz_test.go b/fuzz_test.go new file mode 100644 index 0000000..c3b6a2c --- /dev/null +++ b/fuzz_test.go @@ -0,0 +1,180 @@ +// Copyright (c) 2026 the go-pdfkit/pdfkit authors. All rights reserved. +// Use of this source code is governed by a BSD-3-Clause license that can be +// found in the LICENSE file at the root of this repository. + +package pdfkit + +import ( + "bytes" + "image" + "image/jpeg" + "testing" +) + +// jpegInfo is the only parser in this package that reads bytes a caller +// supplied, and it reads them by hand: markers, segment lengths and offsets, +// every one of them a number out of the file. image/jpeg is the oracle it is +// measured against — not because it is the specification, but because it is +// what every consumer of the PDF we write will have used to look at the same +// bytes. +// +// ⛔ The property is NOT "jpegInfo accepts what image/jpeg accepts". jpegInfo +// stops at the first start-of-frame and never looks at the entropy-coded data, +// so it accepts files image/jpeg rejects, and that is by design: pdfkit +// embeds the bytes without decoding them. What must never happen is the two +// AGREEING TO DISAGREE — a file both read, with different dimensions — because +// then pdfkit writes a /Width into the PDF that the consumer's own decoder +// contradicts, and the picture comes out sheared with nothing to blame. +func FuzzJPEGInfo(f *testing.F) { + f.Add(jpegBytes()) + f.Add(grayJPEGBytes(f)) + f.Add(fillBytesJPEG(f)) + f.Add([]byte{0xFF, 0xD8}) + f.Add([]byte{0xFF, 0xD8, 0xFF, 0xC0, 0x00, 0x0B, 0x08, 0x00, 0x10, 0x00, 0x10, 0x01}) + f.Add([]byte("not a jpeg at all")) + // TEM, which stands alone (Table B.1), then a frame header. + f.Add([]byte{0xFF, 0xD8, 0xFF, 0x01, 0xFF, 0xC0, 0x00, 0x0B, 0x08, 0x00, 0x10, 0x00, 0x10, 0x01}) + // X'FF00': byte stuffing, not a marker. + f.Add([]byte{0xFF, 0xD8, 0xFF, 0x00, 0x00, 0x0B}) + // A marker with nothing behind it to hold its length. + f.Add([]byte{0xFF, 0xD8, 0xFF, 0xC0, 0x00}) + // A frame header that declares more than it carries. + f.Add([]byte{0xFF, 0xD8, 0xFF, 0xC0, 0x00, 0x07, 0x08, 0x00, 0x10, 0x00}) + // A frame header too short to hold a frame: present, well-formed, empty. + f.Add([]byte{0xFF, 0xD8, 0xFF, 0xC0, 0x00, 0x04, 0xAA, 0xBB}) + // Extraneous bytes between segments, which libjpeg warns about and skips. + f.Add([]byte{0xFF, 0xD8, 0x30, 0xFF, 0xC0, 0x00, 0x0B, 0x08, 0x00, 0x10, 0x00, 0x10, 0x01}) + + f.Fuzz(func(t *testing.T, data []byte) { + w, h, comps, err := jpegInfo(data) + + cfg, cfgErr := jpeg.DecodeConfig(bytes.NewReader(data)) + if cfgErr != nil { + return // image/jpeg could not read it; there is nothing to compare against + } + if err != nil { + t.Fatalf("image/jpeg read this as %dx%d and jpegInfo refused it (%v): a JPEG a "+ + "consumer can decode is one DrawJPEG should be able to embed", + cfg.Width, cfg.Height, err) + } + if w != cfg.Width || h != cfg.Height { + t.Fatalf("jpegInfo says %dx%d, image/jpeg says %dx%d: DrawJPEG would write a "+ + "/Width the reader's own decoder contradicts", + w, h, cfg.Width, cfg.Height) + } + if comps < 1 || comps > 4 { + t.Fatalf("jpegInfo reports %d components for a file image/jpeg decoded", comps) + } + }) +} + +// grayJPEGBytes is a one-component frame: the other colour space DrawJPEG +// names, and a different SOF component count for the parser to walk past. +func grayJPEGBytes(f *testing.F) []byte { + f.Helper() + var buf bytes.Buffer + g := image.NewGray(image.Rect(0, 0, 8, 8)) + for i := range g.Pix { + g.Pix[i] = uint8(i) + } + if err := jpeg.Encode(&buf, g, nil); err != nil { + f.Fatal(err) + } + return buf.Bytes() +} + +// fillBytesJPEG is a valid JPEG with a fill byte inserted before the marker +// that follows SOI. ⛔ A marker "may optionally be preceded by any number of +// fill bytes" (ITU-T T.81 §B.1.1.2), so this is a legal file every decoder +// accepts — and the shape a hand-written marker walk is most likely to lose +// its place in. It is a seed rather than a test so the fuzzer can build on it. +func fillBytesJPEG(f *testing.F) []byte { + f.Helper() + src := jpegBytes() + out := make([]byte, 0, len(src)+2) + out = append(out, src[0], src[1]) // SOI + out = append(out, 0xFF, 0xFF) // two fill bytes + out = append(out, src[2:]...) + return out +} + +// FuzzDrawImageSamples drives the sample splitter over images whose pixels +// come from the fuzzer. It allocates nothing a caller did not already hold -- +// the geometry is this test's, so the bytes are the only thing being varied -- +// and it exists because unpremultiply does integer arithmetic on numbers the +// image chose. +// +// ⛔ The property is one no bug in the arithmetic can satisfy by accident: a +// fully opaque pixel must come back EXACTLY as it went in. Half-transparent +// pixels are checked by round-trip bound instead, because dividing by alpha +// and multiplying back is lossy and an exact assertion there would be a test +// of rounding rather than of correctness. +func FuzzDrawImageSamples(f *testing.F) { + f.Add([]byte{0xFF, 0x00, 0x00, 0xFF}) + f.Add([]byte{0x80, 0x40, 0x20, 0x80}) + f.Add([]byte{0x00, 0x00, 0x00, 0x00}) + + f.Fuzz(func(t *testing.T, pix []byte) { + const w, h = 4, 4 + img := image.NewRGBA(image.Rect(0, 0, w, h)) + for i := range img.Pix { + if i < len(pix) { + img.Pix[i] = pix[i] + } + } + // Premultiplied storage: a channel above its alpha is a state no + // encoder produces and image/color does not define. Clamp rather than + // skip, so every input still exercises the walk. + for i := 0; i+3 < len(img.Pix); i += 4 { + a := img.Pix[i+3] + for c := range 3 { + if img.Pix[i+c] > a { + img.Pix[i+c] = a + } + } + } + + rgb, alpha, hasAlpha := samples(img) + if len(rgb) != w*h*3 || len(alpha) != w*h { + t.Fatalf("samples returned %d colour and %d alpha bytes for %d pixels", + len(rgb), len(alpha), w*h) + } + opaque := true + for i := 0; i+3 < len(img.Pix); i += 4 { + px := i / 4 + if a := img.Pix[i+3]; a != 0xFF { + opaque = false + continue + } + for c := range 3 { + if rgb[px*3+c] != img.Pix[i+c] { + t.Fatalf("pixel %d channel %d went in as %d and came out as %d, and it "+ + "was fully opaque: nothing should have been divided", + px, c, img.Pix[i+c], rgb[px*3+c]) + } + } + if alpha[px] != 0xFF { + t.Fatalf("pixel %d is opaque and its mask sample is %d", px, alpha[px]) + } + } + if opaque && hasAlpha { + t.Fatal("every pixel is opaque and samples asked for a soft mask anyway: " + + "the document pays for an /SMask that masks nothing") + } + // The straight samples must be a colour, not an overflow. Only + // reachable through unpremultiply, which is where the arithmetic is. + for px := range w * h { + a := img.Pix[px*4+3] + if a == 0 || a == 0xFF { + continue + } + for c := range 3 { + if got := rgb[px*3+c]; int(got) < int(img.Pix[px*4+c]) { + t.Fatalf("pixel %d channel %d: %d premultiplied became %d straight at "+ + "alpha %d, which is darker than it went in", + px, c, img.Pix[px*4+c], got, a) + } + } + } + }) +} diff --git a/image.go b/image.go index 108afde..4e48fd1 100644 --- a/image.go +++ b/image.go @@ -211,16 +211,48 @@ func jpegInfo(data []byte) (w, h, comps int, err error) { if len(data) < 2 || data[0] != 0xFF || data[1] != 0xD8 { return 0, 0, 0, fmt.Errorf("pdfkit: not a JPEG (bad SOI)") } + // ⛔ The three skips below are not leniency for its own sake. DrawJPEG + // embeds these bytes UNTOUCHED, so whoever opens the PDF decodes the same + // file with a libjpeg-shaped decoder — and a writer that refuses what every + // reader of its own output accepts is the one in the wrong. The rules are + // taken from Go's image/jpeg (reader.go:542-581), which states where each + // comes from; both divergences below were found by FuzzJPEGInfo, within a + // second of each other, against image/jpeg as the oracle. i := 2 - for i+4 <= len(data) { + for i+1 < len(data) { + // Extraneous non-marker bytes between segments. libjpeg's next_marker + // raises JWRN_EXTRANEOUS_DATA and skips to the next X'FF'; image/jpeg + // does the same and so do we. (Found: a file image/jpeg read as + // 12336x12336 was refused here as "expected marker".) if data[i] != 0xFF { - return 0, 0, 0, fmt.Errorf("pdfkit: corrupt JPEG (expected marker)") + i++ + continue } marker := data[i+1] - if marker == 0xD8 || marker == 0xD9 || (marker >= 0xD0 && marker <= 0xD7) { + // X'FF00' is byte stuffing inside entropy-coded data, not a marker — + // a marker's second byte is "not equal to 0 or X'FF'" (T.81 §B.1.1.2). + if marker == 0x00 { i += 2 continue } + // "Any marker may optionally be preceded by any number of fill bytes, + // which are bytes assigned code X'FF'" (§B.1.1.2). (Found: a legal + // file image/jpeg read as 4x4 was refused here as a bad segment + // length, because the walk took the second X'FF' for the marker and + // then read the real marker as a length.) + if marker == 0xFF { + i++ + continue + } + // The markers that stand alone and carry no segment: Table B.1's + // asterisked rows — SOI, EOI, the eight restarts, and TEM. + if marker == 0xD8 || marker == 0xD9 || marker == 0x01 || (marker >= 0xD0 && marker <= 0xD7) { + i += 2 + continue + } + if i+4 > len(data) { + return 0, 0, 0, fmt.Errorf("pdfkit: corrupt JPEG (truncated segment)") + } seg := int(data[i+2])<<8 | int(data[i+3]) if seg < 2 || i+2+seg > len(data) { return 0, 0, 0, fmt.Errorf("pdfkit: corrupt JPEG (bad segment length)") diff --git a/testdata/fuzz/FuzzJPEGInfo/1e609ca636cd5bfc b/testdata/fuzz/FuzzJPEGInfo/1e609ca636cd5bfc new file mode 100644 index 0000000..27007c7 --- /dev/null +++ b/testdata/fuzz/FuzzJPEGInfo/1e609ca636cd5bfc @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("\xff\xd80\xff\xc0\x00\x11\b0000\x030A\x001A\x012A\x03\xff\xda00")