diff --git a/Makefile b/Makefile index 510883311..b46954eb5 100644 --- a/Makefile +++ b/Makefile @@ -2813,8 +2813,8 @@ tables-cpp-release: $(MAKE) tables-wire-fuzz N=500000 $(MAKE) tables-wire-fuzz SEED=2 N=500000 -.PHONY: tables-wire-fuzz-negative-control tables-wire-fuzz-length-negative-control tables-wire-fuzz-index-negative-control tables-wire-fuzz-arm-width-negative-control tables-wire-fuzz-arm-terminator-negative-control tables-wire-fuzz-oracle-negative-control tables-wire-fuzz-node-type-negative-control -tables-wire-fuzz-negative-control: tables-wire-fuzz-length-negative-control tables-wire-fuzz-index-negative-control tables-wire-fuzz-arm-width-negative-control tables-wire-fuzz-arm-terminator-negative-control tables-wire-fuzz-oracle-negative-control tables-wire-fuzz-node-type-negative-control +.PHONY: tables-wire-fuzz-negative-control tables-wire-fuzz-length-negative-control tables-wire-fuzz-index-negative-control tables-wire-fuzz-arm-width-negative-control tables-wire-fuzz-arm-terminator-negative-control tables-wire-fuzz-oracle-negative-control tables-wire-fuzz-node-type-negative-control tables-wire-fuzz-blob-node-negative-control +tables-wire-fuzz-negative-control: tables-wire-fuzz-length-negative-control tables-wire-fuzz-index-negative-control tables-wire-fuzz-arm-width-negative-control tables-wire-fuzz-arm-terminator-negative-control tables-wire-fuzz-oracle-negative-control tables-wire-fuzz-node-type-negative-control tables-wire-fuzz-blob-node-negative-control # the string read's `room( len )`: a length past the body is then read anyway, # so the sabotaged leg takes a string out of a neighbour's bytes and CLAMPS it @@ -3039,6 +3039,36 @@ tables-wire-fuzz-node-type-negative-control: build/conformance-harness build/wir @grep -m1 "FAILED" $(NODE_TYPE_NC)/log @echo "negative control: placing a node no pointer names turns the pinned vector RED" +# THE SAME RULE AT THE TWO RESERVED BLOB IDS (docs/SPEC-TABLES.md §2.5, §3.1, +# §3.3), and the vector message_blob_node_unpointed is the red it closed. §3.3's +# tail announces `bytes` and `string` whether or not the root names them, so a +# mutated record can claim a blob under a root that has no blob edge at all. The +# reference's NodeStorage answers -1 for one; the engine named both ids at +# every root, placed a blob, and turned the pointer that reached it into a +# kind_mismatch. The sabotage is that revert, and the run must go red ON THE +# VECTOR. +BLOB_NODE_NC := build/wire-fuzz-nc-blob-node +.PHONY: tables-wire-fuzz-blob-node-negative-control +tables-wire-fuzz-blob-node-negative-control: build/conformance-harness build/wire-fuzz-cpp + @rm -rf $(BLOB_NODE_NC) && mkdir -p $(BLOB_NODE_NC) + @sed -e 's|ir.PointerReachableBlobs(m.Unit, inst.Def)|true, true // NEGATIVE CONTROL: both reserved ids are nameable at every root|' \ + internal/tablewire/decodenodes.go > $(BLOB_NODE_NC)/decodenodes.go.txt + @cmp -s internal/tablewire/decodenodes.go $(BLOB_NODE_NC)/decodenodes.go.txt && \ + { echo "NEGATIVE CONTROL: the blob-node sabotage patched nothing"; exit 1; } || true + @printf '{"Replace":{"%s/internal/tablewire/decodenodes.go":"%s/$(BLOB_NODE_NC)/decodenodes.go.txt"}}\n' \ + "$(CURDIR)" "$(CURDIR)" > $(BLOB_NODE_NC)/overlay.json + go build -overlay $(BLOB_NODE_NC)/overlay.json -o $(BLOB_NODE_NC)/harness ./test/conformance/harness + @if $(BLOB_NODE_NC)/harness wire-fuzz --driver ./build/wire-fuzz-cpp --seed $(SEED) --n 0 \ + --failed $(BLOB_NODE_NC)/failed.bin > $(BLOB_NODE_NC)/log 2>&1; then \ + echo "NEGATIVE CONTROL FAILED: both reserved ids are nameable again and the wire fuzzer stayed green"; \ + cat $(BLOB_NODE_NC)/log; exit 1; \ + fi + @grep -q "message_blob_node_unpointed" $(BLOB_NODE_NC)/log || \ + { echo "NEGATIVE CONTROL FAILED: the wire fuzzer went red, but not on the pinned vector"; \ + cat $(BLOB_NODE_NC)/log; exit 1; } + @grep -m1 "FAILED" $(BLOB_NODE_NC)/log + @echo "negative control: naming a blob node no pointer reaches turns the pinned vector RED" + # The GENERATED half of the data: the JSON text of every instance and the read # report of every evolution case, both from the compiler's own engine. .PHONY: conformance-generate diff --git a/docs/SPEC-TABLES.md b/docs/SPEC-TABLES.md index 8d780d0d6..f171c8e5b 100644 --- a/docs/SPEC-TABLES.md +++ b/docs/SPEC-TABLES.md @@ -3609,12 +3609,17 @@ the payload opens with the count and then carries the records: - **A blob is numbered as every node is**: first visit, depth-first, in the slot's declaration order, and it has no descent — a blob reaches nothing. Two slots that name one blob name one index, and the record is written once. -- **A reader that has no blobs skips it by its length and counts it - `unknown`**, as it does any record whose type id it cannot name, and the - record keeps its index. A reader that names the id and meets a length past - the field it rides in refuses the whole table as **malformed**, as it - refuses any record that does; nothing reads a blob's bytes from outside its - record. A blob record under a `*T` slot, or a table record under a `*bytes` +- **A ROOT that reaches no blob of that shape skips it by its length and + counts it `unknown`**, as it does any record whose type id it cannot name, + and the record keeps its index. The reserved ids are named on the same terms + a table's name id is (§3.1): a blob node is a pointer's pointee, so a + `*string` record under a root that no `*string` edge sits below is a node + this reader cannot place, and it commands no storage. A file never carries + one, because a writer writes only the ids its own body used; a MESSAGE can, + because §3.3's tail announces every reserved id whether or not the root + names it. A reader that names the id and meets a length past the field it + rides in refuses the whole table as **malformed**, as it refuses any record + that does; nothing reads a blob's bytes from outside its record. A blob record under a `*T` slot, or a table record under a `*bytes` slot, is the type-id check below: **kind mismatch**, pointer null. A `*bytes` slot, a `*string` slot and a `*wstring` slot are three ids for the same reason `string(N)`, `bytes(N)` and `wstring(N)` are three kinds on this diff --git a/internal/tablewire/decodenodes.go b/internal/tablewire/decodenodes.go index c66c4dcda..081e292fe 100644 --- a/internal/tablewire/decodenodes.go +++ b/internal/tablewire/decodenodes.go @@ -119,18 +119,28 @@ func decodeVariable(m *tabletext.Model, inst *tabletext.Instance, data []byte, i } } + // AND THE TWO RESERVED BLOB IDS ARE ON THE SAME FOOTING (§2.5): a blob + // node is a pointer's pointee too, so a `*bytes` record under a root no + // `*bytes` edge sits below is a node this reader cannot name either. An id + // missing from this map falls through to the table lookup, which never + // holds it, and is counted unknown there. + blobKind := map[uint64]ir.FieldTypeKind{} + bytesEdge, stringEdge := ir.PointerReachableBlobs(m.Unit, inst.Def) + if bytesEdge { + blobKind[ir.BytesWireTypeId] = ir.TBytes + } + if stringEdge { + blobKind[ir.StringWireTypeId] = ir.TString + } + // PASS ONE: fill the numbering from the FRAMING, so that an index resolves // whichever way it points. It reads no body — a blob's record IS its // bytes, so its node is complete here, and the tolerant wire load copies // them as it copies every node (§2.5). st.nodes = make([]Node, len(records)) for i, rec := range records { - switch rec.TypeId { - case ir.BytesWireTypeId: - st.nodes[i] = Node{Blob: &tabletext.Blob{Data: append([]byte(nil), rec.Body...)}, Kind: ir.TBytes} - continue - case ir.StringWireTypeId: - st.nodes[i] = Node{Blob: &tabletext.Blob{Data: append([]byte(nil), rec.Body...)}, Kind: ir.TString} + if kind, ok := blobKind[rec.TypeId]; ok { + st.nodes[i] = Node{Blob: &tabletext.Blob{Data: append([]byte(nil), rec.Body...)}, Kind: kind} continue } sd := byTypeId[rec.TypeId] diff --git a/ir/table.go b/ir/table.go index 6fbcd602a..3cc22f670 100644 --- a/ir/table.go +++ b/ir/table.go @@ -321,6 +321,57 @@ func PointerReachable(u *Unit, root *Struct) map[string]bool { return named } +// PointerReachableBlobs is [PointerReachable]'s answer for the two RESERVED +// node type ids (docs/SPEC-TABLES.md §2.5): whether a `*bytes` edge and +// whether a `*string` edge sits anywhere below this root, over the same walk. +// +// A blob node is a pointer's pointee exactly as a table's node is, so the same +// rule decides it: a `*bytes` record under a root no `*bytes` pointer sits +// below is a node this reader cannot place, and it commands no region storage, +// its body is skipped and one `unknown` is counted (§3.1, §6.5). A file never +// carries one; the MESSAGE form can, because §3.3's unconditional tail +// announces both reserved ids whether or not the root names them. +func PointerReachableBlobs(u *Unit, root *Struct) (bytes bool, str bool) { + visited := map[string]bool{} + var descend func(st *Struct) + descend = func(st *Struct) { + if st == nil || visited[st.Name] { + return + } + visited[st.Name] = true + for _, f := range st.Fields { + if f.IsMap() { + descend(f.MapEntry) + continue + } + if f.Type.Blob() { + if f.Type.Kind == TString { + str = true + } else { + bytes = true + } + continue + } + if f.Type.Kind != TNamed { + continue + } + if un, isUnion := f.Type.Ref.(*Union); isUnion { + for _, v := range un.Variants { + if v.Ref != nil { + descend(v.Ref) + } + } + continue + } + if ref, ok := f.Type.Ref.(*Struct); ok { + descend(ref) + } + } + } + descend(root) + return bytes, str +} + // unionPointerTargets adds every table a POINTER ARM of un targets, through // nested union arms too (docs/SPEC-TABLES.md §2.6). func unionPointerTargets(un *Union, targets map[string]bool, seen map[*Union]bool) { diff --git a/testdata/wire/tables/fuzz-vectors/INDEX.txt b/testdata/wire/tables/fuzz-vectors/INDEX.txt index eef385b5d..af86ebb0e 100644 --- a/testdata/wire/tables/fuzz-vectors/INDEX.txt +++ b/testdata/wire/tables/fuzz-vectors/INDEX.txt @@ -65,3 +65,18 @@ pointer_self_cycle graphdemo Scene testdata/wire/tables/fuzz-vectors/pointer_ # own landing (#530). The fix is ir.PointerReachable, in the engine's node # table and in the fuzzer's own region arithmetic. message_node_type_unpointed graphdemo Scene testdata/wire/tables/fuzz-vectors/message_node_type_unpointed.bin message + +# A BLOB NODE UNDER A ROOT NO BLOB POINTER SITS BELOW (§2.5, §3.1, §6.5, §3.3), +# which is the node-type rule above at the two RESERVED ids. §3.3's tail +# announces `bytes` and `string` unconditionally, so a mutated record can claim +# `*string` in graphdemo, where no `*bytes` and no `*string` edge exists at all. +# A blob node is a pointer's pointee like any other, so the reference gives it +# no region storage, skips its body and counts one unknown — NodeStorage +# names a reserved id only when reachableBlobs found the edge. The engine named +# both ids at every root, placed a blob, and the `*TreeNode` slot that reached +# it then counted a kind_mismatch: two unknown against the reference's one, and +# one kind_mismatch against none. Found by the random pass at mutant 434519 of +# seed 24845619678, on Certify run 33925154771. The fix is +# ir.PointerReachableBlobs, gating the engine's two reserved ids on the same +# walk the emitter already gated its own on. +message_blob_node_unpointed graphdemo Scene testdata/wire/tables/fuzz-vectors/message_blob_node_unpointed.bin message diff --git a/testdata/wire/tables/fuzz-vectors/message_blob_node_unpointed.bin b/testdata/wire/tables/fuzz-vectors/message_blob_node_unpointed.bin new file mode 100644 index 000000000..e5f316622 Binary files /dev/null and b/testdata/wire/tables/fuzz-vectors/message_blob_node_unpointed.bin differ