linearize: check the parameters against the document, not only against themselves

README.md

@@ -134,6 +134,52 @@ types; and `/L` against the real file length.
 Not checked: the hint tables themselves (F.4) — a file can pass this and still
 send a reader to the wrong offset.
 
+⛔⛔ **`Linearized` on its own is a weaker test than it looks, and a writer
+developed against it would pass without linearizing anything.** Of Table F.1's
+claims it checks exactly one against the file — `/L` against the byte count —
+because it is deliberately cheap: a reader deciding whether to stream a file
+has fetched 1024 bytes and has no cross-reference table to resolve anything
+with. `/O`, `/E`, `/N` and `/T` are read for type and never consulted, so a
+file carrying a correct `/L` and four **invented integers** is accepted. That
+is precisely what a first linearizer emits.
+
+`CheckLinearized(b)` is the other question, for the other caller: not "may a
+reader trust the front of this file" but "is this file actually what the front
+says". It opens the document, so it costs what opening costs, and it is the
+judgement a writer needs on bytes it has just produced. It establishes that
+every parameter **points at what it claims to** — `/N` is the real page count,
+`/O` is the real first page *and* is a `/Page`, `/H` begins at a real stream
+object, `/T` reaches the main cross-reference (either shape: Table F.1 gives
+it two meanings and both are real), `/E` is inside the file.
+
+The four cases that gate it are files `Linearized` **accepts** and
+`CheckLinearized` refuses: the committed qpdf fixture with one integer
+replaced by one of **the same width**, so `/L` still matches and nothing at
+the front of the file is wrong — what is wrong is that it describes a
+different document. The replacement's width is asserted by the helper, because
+an edit one byte longer would be refused for the length instead and the deep
+check would never run.
+
+⛔⛔ **`/E` is NOT gated, and a writer must not read a pass here as evidence
+about it.** Table F.1 calls `/E` the offset of the end of the first page, and
+qpdf *computes* it — on the committed fixture it reports `computed = 901..902`
+and refuses a wrong value. This only bounds `/E` between 1 and the file
+length, so a writer that miscomputes it passes here and fails qpdf. Computing
+it needs the first-page object set, which is the same partition the hint
+tables describe, so it waits for that work rather than being approximated now.
+
+⛔ **The `/H` length is compared with the object exactly**, not within a
+window. A first version allowed the declared length to exceed the parsed
+extent by 0–2 bytes, reasoning that a writer might use CRLF; that accepts
+`/H [ 550 126 ]` and `/H [ 550 128 ]` for a 126-byte object, both of which
+qpdf refuses. The end-of-line is now **read** rather than guessed, and every
+spelling of one (LF, CR, CRLF, none) has its own test — a tolerance wide
+enough to cover every line ending is wide enough to hide a wrong length.
+
+Still not checked, by either: the hint tables (F.4). `qpdf --check` remains the
+foreign judge, and it is run across 1/2/5/25 pages × plain, object-streams and
+uncompressed-streams.
+
 The positives are **qpdf's** output, not fixtures written to agree with this
 reading of Annex F. On five real library scans linearized by qpdf and judged by
 `qpdf --check` alongside: **5 agree, 0 disagree**, including a 68-page, 13 MB

fuzz_test.go

@@ -1,6 +1,7 @@
 package reader_test
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -171,3 +172,75 @@ func FuzzDecode(f *testing.F) {
 		})
 	})
 }
+
+// FuzzCheckLinearized drives the deep linearization check.
+//
+// ⛔ IT IS A PARSER OF NUMBERS AN ATTACKER CHOSE. /H, /O, /E, /T and /N come
+// out of the first object of the file and are used as OFFSETS and LENGTHS into
+// it. The cheap [reader.Linearized] only type-checks them; this one slices the
+// file at them, which is where a bad bound stops being a wrong answer and
+// starts being a read past the end.
+//
+// The seeds are the two committed fixtures — byte substitutions in them keep
+// the length, so /L still matches and the mutator reaches the deep checks
+// rather than stopping at the front — plus fronts whose integers are absurd on
+// purpose.
+func FuzzCheckLinearized(f *testing.F) {
+	for _, name := range []string{"linearized.pdf", "linearized-objstm.pdf"} {
+		if b, err := os.ReadFile(filepath.Join("testdata", name)); err == nil {
+			f.Add(b)
+		}
+	}
+	// ⛔ MaxInt32 in both halves of /H. Written as an addition, the bound check
+	// overflows to a negative number on a 32-bit build and admits the file;
+	// this seed is the regression for that, and it is the shape a hostile file
+	// takes rather than one a generator would invent.
+	//
+	// ⛔⛔ THE SEED'S /L MUST BE THE SEED'S OWN LENGTH, which review found it
+	// was not: every one of them said `/L 1` for a file hundreds of bytes
+	// long, so [reader.Linearized] refused them on the length and the deep
+	// checks — the ones these seeds were written for — never ran at all. The
+	// placeholder is TEN DIGITS so that filling it in cannot change the
+	// length it is measuring.
+	for _, s := range []string{
+		"<< /Linearized 1 /L %010d /H [ 2147483647 2147483647 ] /O 1 /E 1 /N 1 /T 1 >>",
+		"<< /Linearized 1 /L %010d /H [ -1 -1 ] /O -1 /E -1 /N -1 /T -1 >>",
+		"<< /Linearized 1 /L %010d /H [ 0 0 ] /O 0 /E 0 /N 0 /T 0 >>",
+		"<< /Linearized 1 /L %010d /H [ 40 20 ] /O 1 /E 60 /N 1 /T 50 >>",
+	} {
+		body := "%PDF-1.7\n1 0 obj\n" + s + "\nendobj\n"
+		f.Add([]byte(fmt.Sprintf(body, len(fmt.Sprintf(body, 0)))))
+	}
+	f.Fuzz(func(t *testing.T, b []byte) {
+		budget(t, 2*time.Second, len(b), "CheckLinearized", func() {
+			lin, err := reader.CheckLinearized(b)
+			if err != nil {
+				return
+			}
+			// ⛔ A TAUTOLOGY WAS REMOVED FROM HERE. It asserted that anything
+			// CheckLinearized accepts, Linearized accepts too — which cannot
+			// fail, because CheckLinearized calls Linearized first and returns
+			// its error. Review named it; an invariant that cannot fail is a
+			// line that looks like a gate and is not one.
+			//
+			// ⛔ WHAT FOLLOWS IS NOT A RESTATEMENT OF THE CHECKS. The checks
+			// bound each value as they read it; this re-derives the same
+			// property at the API boundary, from len(b) alone, AFTER the
+			// function has committed to an answer. That difference is what
+			// catches an arithmetic fault rather than a missing branch: with
+			// the old `HintOffset+HintLength > len(b)` form, a length near
+			// MaxInt passes the check and fails here.
+			if lin.HintOffset < 0 || lin.HintOffset > len(b) ||
+				lin.HintLength < 0 || lin.HintLength > len(b)-lin.HintOffset {
+				t.Fatalf("/H [%d %d] accepted for a file of %d bytes",
+					lin.HintOffset, lin.HintLength, len(b))
+			}
+			if lin.MainXrefOffset < 0 || lin.MainXrefOffset > len(b) {
+				t.Fatalf("/T %d accepted for a file of %d bytes", lin.MainXrefOffset, len(b))
+			}
+			if lin.FirstPageEnd < 0 || lin.FirstPageEnd > len(b) {
+				t.Fatalf("/E %d accepted for a file of %d bytes", lin.FirstPageEnd, len(b))
+			}
+		})
+	})
+}