From a6c79bdda20e05dbf7f8f3ee9543ceb7253bb069 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 14:39:13 +0000 Subject: [PATCH 01/21] Document reader resource limits for the pointer fan-out DoS Add a "Reader Resource Limits" section. A crafted record can make a reader use far more time and memory than its size suggests, so a reader should ensure that decoding one lookup cannot consume disproportionate resources. The section describes the risks (deep nesting, a huge decoded-value count, and repeated materialization of large string and bytes data) and recommends default limits (a maximum depth of 512 and 65,536 decoded values), while allowing an equivalent strategy such as safe memoization, schema-directed decoding, or a weighted work budget. The largest records MaxMind produces decode a few hundred values and hold about a kilobyte of such data, so the limits leave a wide margin. See GHSA-hj94-g986-h9r7. Implementing these limits across the readers surfaced five points the section now covers: - Schema-aware readers can apply tighter semantic limits to collections whose valid size is known to be small, alongside a general bound for unknown fields. - Under a logical value rule, map keys and values are separate occurrences. - A destination that allocates from a container's declared length must charge that length before allocating. Counting afterward is too late. - The bound should cover the whole externally requested operation. Bounding each navigation step does not bound their cumulative work. - Payload skipped without copying, validation, or allocation need not consume a materialization budget when the traversal to skip it is bounded separately. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Opus 5 (1M context) --- MaxMind-DB-spec.md | 119 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/MaxMind-DB-spec.md b/MaxMind-DB-spec.md index 37a854e1..5a5d3f60 100644 --- a/MaxMind-DB-spec.md +++ b/MaxMind-DB-spec.md @@ -310,6 +310,10 @@ A pointer to another part of the data section's address space. The pointer will point to the beginning of a field. It is illegal for a pointer to point to another pointer. +Because several pointers can share one target, following pointers naively can +decode far more data than a record's size suggests. See +[Reader Resource Limits](#reader-resource-limits). + Pointer values start from the beginning of the data section, _not_ the beginning of the file. Pointers in the metadata start from the beginning of the metadata section. @@ -537,6 +541,121 @@ are ignored. This means that we are limited to 4GB of address space for pointers, so the data section size for the database is limited to 4GB. +## Reader Resource Limits + +A crafted or corrupt data section can make a reader use far more time and memory +than the record's size suggests. A value can be a pointer, and several pointers +can share one target. A small data section can therefore describe a structure +that is huge, or effectively infinite, when fully expanded. + +A reader should ensure that no operation decoding attacker-controlled values +consumes disproportionate time or memory. This applies to a lookup, a path +selection, metadata decoding on open, and verification. + +A reader can apply the depth, value-count, and payload limits below. It can +instead use another strategy with an equivalent bound, such as: + +- Safe memoization of pointer targets. +- Schema-directed decoding into a destination that is finite and nonrecursive, + and that skips unknown values without following pointers. +- A weighted work budget. + +A reader that delegates decoding to caller-defined code should document where +that responsibility transfers. + +Decoding into a specific typed destination does not bound the work by itself. +Recursive types, attacker-sized collections, repeated recognized fields, and +repeated materialization can all still amplify it. A schema-aware reader can add +tighter semantic limits where it knows a collection's valid size is small. Those +do not replace a general bound for unknown fields, custom schemas, and +dynamically shaped output. + +### Maximum data structure depth + +A reader should limit how deeply it nests while decoding one record. The depth +increases by one each time the reader enters a map or an array, or follows a +pointer into one. If the depth exceeds 512, the reader should stop and reject +the record. + +This limit stops unbounded recursion, including a pointer cycle and a structure +nested deeper than any real database needs, and it is a portable default. +Iterative decoding avoids call-stack exhaustion, but it must still detect cycles +and bound nesting and traversal work. + +### Maximum decoded value count + +The depth limit does not bound the total amount of work. A record can nest +arrays or maps whose elements are all pointers to the level below, so each level +is decoded twice as often as the one above. Such a structure stays shallow, but +decoding the top value produces 2\*\*depth leaf decodes and Θ(2\*\*depth) total +decode operations. A record under one kilobyte can therefore take longer to +decode than any real workload allows. + +A reader can bound this by counting the values it decodes for a single record +and stopping if the count exceeds a fixed limit. One rule is to charge each +logical value occurrence, including one resolved from a memoized target. Readers +with structural or weighted bounds may account differently. Under a logical +value rule, map keys and values are separate occurrences. + +If a destination uses a container's declared length to allocate storage, +counting only after that allocation may be too late. A reader can reserve the +declared children against its value budget first, cap the allocation separately, +grow storage incrementally, or use another approach with an equivalent bound. + +The bound should cover the complete externally requested operation rather than +restart at internal phase boundaries. For example, path navigation and decoding +the selected value can share one budget, or be covered by another end-to-end +bound. Bounding each navigation step on its own does not bound their cumulative +work. + +A limit of 65,536 (2\*\*16) values is recommended. The largest records MaxMind +produces decode a few hundred values, so this leaves a wide margin. + +### Maximum expanded payload work + +The value count limits how many fields a record decodes, not how many bytes they +hold. A single string or bytes value can be up to 16,843,036 bytes. An array of +32,767 pointers to one such value stays under the value limit. It still expands +to more than 500 GiB, from a file barely larger than the value itself. + +A reader that copies, validates, or allocates these values should bound the +total string and bytes data it materializes for one lookup. The right method and +limit depend on the reader's language and API, so this specification does not +require a single limit. As guidance, the largest records MaxMind produces hold +about a kilobyte of such data, so a few megabytes is generous. + +A reader can borrow these bytes, returning values that point into the data +section. Borrowing does not remove the cost. It moves the cost to whatever +copies the values next, and every consumer that copies pays it again. +Memoization has the same limit. Both bound only the reader's own +materialization, so a consumer that copies into owning values should bound that +step too. + +Payload that a reader skips without copying, validating, or allocating need not +consume a materialization budget, as long as the traversal to skip it is bounded +separately. A reader selecting part of a record likewise need not expand an +unrequested pointer target. A reader whose API or validation rules require that +work should bound it. + +A reader can bound this in several ways: + +- Charge each value's size against a per-lookup budget wherever it is decoded, + including data stored inline in a container that a pointer targets. Stop when + the total exceeds a limit. Re-decoding a pointer target charges its data + again, which is what bounds the amplification. +- Fold the size into the value-count budget, charging each value by its size. + Charge containers by entries traversed, pointer expansion when the target is + not safely reused, map-key and selector work, and strings and bytes + materialized. A small fixed-width scalar can cost little, once the traversal + needed to reach it is bounded. +- Safely memoize decoded pointer targets, handling cycles and in-progress + targets, so a shared target is materialized once. + +When a reader's resource controls reject a decode, it should return an error +rather than a partial result. It may treat a value outside its documented +resource profile as invalid, and may make limits configurable for unusually +large valid records. + ## Reference Implementations ### Writer From f9493bea49208637dcbf28f5c75a9bd0db62f6b9 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 03:08:26 +0000 Subject: [PATCH 02/21] Add pointer fan-out DoS test databases Add two fixtures for the data-section pointer fan-out, GHSA-hj94-g986-h9r7. Both map every address to a data section of nested arrays, each holding two pointers to the node below. A reader that re-decodes a shared pointer target once per referencing path performs 2**40 decode operations from a few hundred bytes. A reader that memoizes resolved targets decodes it in linear time. MaxMind-DB-test-pointer-decoder-dos.mmdb is minimal, at 451 bytes with a single search-tree node, which reads more like a bare data section than a database. Third-party implementations more often test with database files, so MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb is a conventional IPv6 database whose search tree maps the entire address space to the fan-out record. Its tree is a spine down the all-zeros path, 96 nodes deep so an IPv4 lookup, which follows the ::0/96 prefix, reaches the record too. mmdbwriter cannot produce this shape, because its own deduplication fans out while computing the file, so the bytes are written directly through the raw-mmdb helpers. The metadata writer now takes the IP version so the second fixture can declare version 6. Tests verify that the fan-out is exponential in the nesting depth, that both committed fixtures match the generator, and that a real reader opens them and resolves a lookup without expanding the record. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Opus 5 (1M context) --- cmd/write-test-data/main.go | 5 + go.mod | 6 +- pkg/writer/pointerdos.go | 162 +++++++++++++ pkg/writer/pointerdos_test.go | 225 ++++++++++++++++++ pkg/writer/rawmmdb.go | 7 +- ...Mind-DB-test-pointer-decoder-dos-ipv6.mmdb | Bin 0 -> 1027 bytes .../MaxMind-DB-test-pointer-decoder-dos.mmdb | Bin 0 -> 451 bytes 7 files changed, 399 insertions(+), 6 deletions(-) create mode 100644 pkg/writer/pointerdos.go create mode 100644 pkg/writer/pointerdos_test.go create mode 100644 test-data/MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb create mode 100644 test-data/MaxMind-DB-test-pointer-decoder-dos.mmdb diff --git a/cmd/write-test-data/main.go b/cmd/write-test-data/main.go index 638deea0..d7a7479c 100644 --- a/cmd/write-test-data/main.go +++ b/cmd/write-test-data/main.go @@ -82,6 +82,11 @@ func main() { os.Exit(1) } + if err := w.WritePointerDecoderDoSTestDB(); err != nil { + fmt.Printf("writing pointer decoder DoS test databases: %+v\n", err) + os.Exit(1) + } + if err := w.WriteGeoIP2TestDB(); err != nil { fmt.Printf("writing GeoIP2 test databases: %+v\n", err) os.Exit(1) diff --git a/go.mod b/go.mod index ce3e46d2..885bb7bb 100644 --- a/go.mod +++ b/go.mod @@ -4,10 +4,8 @@ go 1.25.0 require ( github.com/maxmind/mmdbwriter v1.2.0 + github.com/oschwald/maxminddb-golang/v2 v2.1.1 go4.org/netipx v0.0.0-20260823151212-3075585bcbeb ) -require ( - github.com/oschwald/maxminddb-golang/v2 v2.1.1 // indirect - golang.org/x/sys v0.38.0 // indirect -) +require golang.org/x/sys v0.38.0 // indirect diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go new file mode 100644 index 00000000..86cf6089 --- /dev/null +++ b/pkg/writer/pointerdos.go @@ -0,0 +1,162 @@ +package writer + +import ( + "fmt" + "os" + "path/filepath" +) + +// pointerDoSDepth is the nesting depth of the fan-out structure. Each level +// adds a factor of two to the number of leaf decodes a reader without pointer +// memoization performs. The 2**40 leaf decodes, and Θ(2**40) total decode +// operations, make a sub-kilobyte file impossible to decode without the fix +// while staying trivial to decode with it. +const pointerDoSDepth = 40 + +const ( + // pointerDoSBuildEpoch is fixed so the generated files are reproducible. + pointerDoSBuildEpoch = 1_000_000_000 + + pointerDoSFixtureFilename = "MaxMind-DB-test-pointer-decoder-dos.mmdb" + pointerDoSIPv6FixtureFilename = "MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb" +) + +// writeDataPointer writes a data-section pointer with a one-byte payload, +// which addresses offsets 0..2047. The value is relative to the start of the +// data section, which is where the reader sets the pointer base. +func writeDataPointer(target int) []byte { + if target < 0 || target > 2047 { + panic(fmt.Sprintf( + "data pointer target %d is out of range for a one-byte payload (0..2047)", + target, + )) + } + return []byte{ + (1 << 5) | byte((target>>8)&0x7), + byte(target & 0xFF), + } +} + +// dataRecordValue24 converts a data-section offset to a 24-bit search-tree +// record value. A search-tree record value is biased by the node count and the +// data-section separator, so it is not the bare data-section offset that a +// data-section pointer uses. This adds that bias. +func dataRecordValue24(nodeCount uint32, dataOffset int) uint32 { + if dataOffset < 0 { + panic(fmt.Sprintf("negative data-section offset: %d", dataOffset)) + } + recordValue := uint64(nodeCount) + dataSeparatorSize + uint64(dataOffset) + const max24BitRecordValue = 1<<24 - 1 + if recordValue > max24BitRecordValue { + panic(fmt.Sprintf( + "data record value %d does not fit in a 24-bit search-tree record", + recordValue, + )) + } + return uint32(recordValue) +} + +// buildPointerFanOutData builds the data section: nested arrays, each holding +// two pointers to the node below. It is laid out leaf first, so the leaf is at +// offset 0 and each array points back to the level below it. A decoder that +// re-decodes a shared pointer target once per referencing path performs +// 2**depth leaf decodes and Θ(2**depth) total decode operations from these few +// hundred bytes (GHSA-hj94-g986-h9r7); a decoder that memoizes resolved targets +// decodes it in linear time. mmdbwriter cannot produce this shape because its +// own deduplication fans out while computing the file, so the bytes are written +// directly. top is the data-section offset of the outermost array. +func buildPointerFanOutData(depth int) (data []byte, top int) { + data = []byte{0xA0} // uint16 with value 0 + prev := 0 + for range depth { + offset := len(data) + data = append(data, 0x02, 0x04) // array (extended type 11), size 2 + data = append(data, writeDataPointer(prev)...) + data = append(data, writeDataPointer(prev)...) + prev = offset + } + return data, prev +} + +// buildPointerFanOutDB builds a minimal valid IPv4 MMDB whose single search-tree +// node resolves every supported lookup to the fan-out data section. See +// buildPointerFanOutData. +func buildPointerFanOutDB(depth int) []byte { + data, top := buildPointerFanOutData(depth) + + const nodeCount = 1 + // A data record value is the data-section offset plus the node count plus + // the 16-byte data section separator, which is how a reader recovers the + // offset: offset = recordValue - nodeCount - dataSeparatorSize. + recordValue := dataRecordValue24(nodeCount, top) + + buf := make([]byte, 1024+len(data)) + pos := 0 + pos += writeSearchTree(buf[pos:], recordValue) + pos += dataSeparatorSize + pos += copy(buf[pos:], data) + pos += writeMetadataBlock(buf[pos:], nodeCount, pointerDoSBuildEpoch) + return buf[:pos] +} + +// buildPointerFanOutAllSpaceDB builds a conventional IPv6 MMDB that maps the +// entire address space to the fan-out data record, so opening it, looking up +// any address, and decoding the result reproduces the denial of service. A +// lookup only finds the record; decoding it is what expands the fan-out. This +// is the form a third-party implementation is most likely to test, since +// implementations commonly test with database files rather than bare data +// sections. +// +// The search tree is a spine down the all-zeros path. At every node the +// one-branch resolves to the data record and the zero-branch descends to the +// next node, and the final node resolves both branches to the record. The spine +// is 96 nodes deep so that an IPv4 lookup, which follows the ::0/96 prefix, +// reaches the final node and the record too. +func buildPointerFanOutAllSpaceDB(depth int) []byte { + data, top := buildPointerFanOutData(depth) + + const ipv4PrefixBits = 96 + const nodeCount = ipv4PrefixBits + 1 // nodes 0..96 + badRecord := dataRecordValue24(nodeCount, top) + + const recordPairSize = 6 // two 24-bit records per node + buf := make([]byte, nodeCount*recordPairSize+dataSeparatorSize+len(data)+256) + pos := 0 + for i := range uint32(nodeCount) { + leftRecord := badRecord + if i < ipv4PrefixBits { + leftRecord = i + 1 // descend the all-zeros path + } + pos += writeSearchTreeRecords(buf[pos:], leftRecord, badRecord) + } + pos += dataSeparatorSize + pos += copy(buf[pos:], data) + pos += writeMetadataBlockWithKeyOrder( + buf[pos:], + nodeCount, + pointerDoSBuildEpoch, + 6, + metadataKeysStandard, + ) + return buf[:pos] +} + +// WritePointerDecoderDoSTestDB writes the databases that exercise the +// data-section pointer fan-out denial of service: a minimal single-node +// database and a conventional IPv6 database that maps all of the address space +// to the fan-out record. See buildPointerFanOutData. +func (w *Writer) WritePointerDecoderDoSTestDB() error { + files := map[string][]byte{ + pointerDoSFixtureFilename: buildPointerFanOutDB(pointerDoSDepth), + pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( + pointerDoSDepth, + ), + } + for name, db := range files { + path := filepath.Clean(filepath.Join(w.target, name)) + if err := os.WriteFile(path, db, 0o644); err != nil { + return fmt.Errorf("writing pointer fan-out database %s: %w", name, err) + } + } + return nil +} diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go new file mode 100644 index 00000000..293bc8cb --- /dev/null +++ b/pkg/writer/pointerdos_test.go @@ -0,0 +1,225 @@ +package writer + +import ( + "bytes" + "math" + "net/netip" + "os" + "path/filepath" + "testing" + + "github.com/oschwald/maxminddb-golang/v2" +) + +// countLeafDecodes walks the fan-out data section the way a reader without +// pointer memoization does, following every pointer and counting how many times +// the shared leaf is decoded. The leaf count is 2**depth and the total number +// of decode operations is Θ(2**depth), which makes the structure a denial of +// service. +func countLeafDecodes(data []byte, offset int) int { + if data[offset]>>5 == 5 { // uint16 leaf + return 1 + } + // Array of size two: a two-byte header followed by two one-byte-payload + // pointers. A pointer's value is ((byte0 & 0x7) << 8) | byte1. + left := int(data[offset+2]&0x7)<<8 | int(data[offset+3]) + right := int(data[offset+4]&0x7)<<8 | int(data[offset+5]) + return countLeafDecodes(data, left) + countLeafDecodes(data, right) +} + +func TestPointerFanOutDataIsExponential(t *testing.T) { + for depth := 1; depth <= 10; depth++ { + data, top := buildPointerFanOutData(depth) + got := countLeafDecodes(data, top) + want := 1 << depth + if got != want { + t.Errorf("depth %d: leaf decoded %d times, want %d", depth, got, want) + } + } +} + +func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { + cases := map[string][]byte{ + pointerDoSFixtureFilename: buildPointerFanOutDB(pointerDoSDepth), + pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( + pointerDoSDepth, + ), + } + for name, got := range cases { + //nolint:gosec // name comes from the fixed cases map, not user input. + want, err := os.ReadFile(filepath.Join("..", "..", "test-data", name)) + if err != nil { + t.Fatalf("reading %s: %v", name, err) + } + if !bytes.Equal(got, want) { + t.Errorf("%s differs from the committed file; regenerate with write-test-data", name) + } + } +} + +func TestPointerFanOutFixturesAreSemanticallyValid(t *testing.T) { + tests := []struct { + name string + ipVersion uint + nodeCount uint + addresses []string + }{ + { + name: pointerDoSFixtureFilename, + ipVersion: 4, + nodeCount: 1, + addresses: []string{"0.0.0.0", "203.0.113.9", "255.255.255.255"}, + }, + { + name: pointerDoSIPv6FixtureFilename, + ipVersion: 6, + nodeCount: 97, + addresses: []string{ + "0.0.0.0", + "203.0.113.9", + "255.255.255.255", + "::", + "2001:db8::1", + "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + path := filepath.Clean(filepath.Join("..", "..", "test-data", test.name)) + db, err := maxminddb.Open(path) + if err != nil { + t.Fatalf("opening fixture: %v", err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Errorf("closing fixture: %v", err) + } + }) + + if got := db.Metadata.BinaryFormatMajorVersion; got != 2 { + t.Errorf("binary format major version = %d, want 2", got) + } + if got := db.Metadata.BinaryFormatMinorVersion; got != 0 { + t.Errorf("binary format minor version = %d, want 0", got) + } + if got := db.Metadata.BuildEpoch; got != pointerDoSBuildEpoch { + t.Errorf("build epoch = %d, want %d", got, pointerDoSBuildEpoch) + } + if got := db.Metadata.DatabaseType; got != "Test" { + t.Errorf("database type = %q, want %q", got, "Test") + } + if got := db.Metadata.IPVersion; got != test.ipVersion { + t.Errorf("IP version = %d, want %d", got, test.ipVersion) + } + if got := db.Metadata.NodeCount; got != test.nodeCount { + t.Errorf("node count = %d, want %d", got, test.nodeCount) + } + if got := db.Metadata.RecordSize; got != 24 { + t.Errorf("record size = %d, want 24", got) + } + + // Lookups stop at the outer record. Do not call Decode here: expanding + // the fan-out is deliberately hostile in an unprotected reader. + var outerOffset uintptr + for i, address := range test.addresses { + result := db.Lookup(netip.MustParseAddr(address)) + if err := result.Err(); err != nil { + t.Errorf("looking up %s: %v", address, err) + continue + } + if !result.Found() { + t.Errorf("lookup for %s did not find a record", address) + continue + } + if i == 0 { + outerOffset = result.Offset() + } else if got := result.Offset(); got != outerOffset { + t.Errorf("lookup for %s returned offset %d, want %d", address, got, outerOffset) + } + } + + // The reader independently validates the metadata and search tree. Walk + // only one branch per level to validate the data topology in bounded time. + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading fixture: %v", err) + } + recordSizeQuarter := uint64(db.Metadata.RecordSize / 4) + if recordSizeQuarter == 0 || + uint64( + db.Metadata.NodeCount, + ) > uint64( + math.MaxInt-dataSeparatorSize, + )/recordSizeQuarter { + t.Fatalf( + "search tree dimensions overflow int: %d nodes at %d bits per record", + db.Metadata.NodeCount, + db.Metadata.RecordSize, + ) + } + searchTreeSize64 := uint64(db.Metadata.NodeCount) * recordSizeQuarter + //nolint:gosec // G115: searchTreeSize64 is checked against math.MaxInt above. + searchTreeSize := int(searchTreeSize64) + dataStart := searchTreeSize + dataSeparatorSize + if dataStart > len(raw) { + t.Fatalf("data section starts at %d, beyond file size %d", dataStart, len(raw)) + } + if uint64(outerOffset) > uint64(math.MaxInt) { + t.Fatalf("outer record offset %d overflows int", outerOffset) + } + outerOffsetInt := int(outerOffset) + if outerOffsetInt >= len(raw)-dataStart { + t.Fatalf("outer record offset %d is outside the data section", outerOffset) + } + verifyPointerFanOutRecord(t, raw[dataStart:], outerOffsetInt, pointerDoSDepth) + }) + } +} + +// verifyPointerFanOutRecord follows one of the two equal pointers at each +// level. Its work is linear in depth and never expands the hostile structure. +func verifyPointerFanOutRecord(t *testing.T, data []byte, offset, depth int) { + t.Helper() + for level := depth; level > 0; level-- { + if offset < 0 || offset+6 > len(data) { + t.Fatalf("level %d array at offset %d is outside the data section", level, offset) + } + if data[offset] != 0x02 || data[offset+1] != 0x04 { + t.Fatalf("level %d at offset %d is not a two-element array", level, offset) + } + left := smallDataPointer(t, data, offset+2) + right := smallDataPointer(t, data, offset+4) + if left != right { + t.Fatalf("level %d pointers differ: left %d, right %d", level, left, right) + } + if left >= offset { + t.Fatalf( + "level %d pointer target %d does not precede array offset %d", + level, + left, + offset, + ) + } + offset = left + } + if offset != 0 { + t.Fatalf("leaf offset = %d, want 0", offset) + } + if data[offset] != 0xA0 { + t.Fatalf("leaf control byte = %#x, want 0xa0", data[offset]) + } +} + +func smallDataPointer(t *testing.T, data []byte, offset int) int { + t.Helper() + if offset < 0 || offset+2 > len(data) { + t.Fatalf("pointer at offset %d is outside the data section", offset) + } + control := data[offset] + if control>>5 != 1 || control&0x18 != 0 { + t.Fatalf("value at offset %d is not a two-byte data pointer", offset) + } + return int(control&0x7)<<8 | int(data[offset+1]) +} diff --git a/pkg/writer/rawmmdb.go b/pkg/writer/rawmmdb.go index b91b34c2..8392e6cb 100644 --- a/pkg/writer/rawmmdb.go +++ b/pkg/writer/rawmmdb.go @@ -141,6 +141,7 @@ func writeMetadataBlockWithKeyOrder( buf []byte, nodeCount uint32, buildEpoch uint64, + ipVersion uint16, keys []string, ) int { pos := 0 @@ -156,7 +157,7 @@ func writeMetadataBlockWithKeyOrder( "build_epoch": func(b []byte) int { return writeUint64(b, buildEpoch) }, "database_type": func(b []byte) int { return writeString(b, "Test") }, "description": func(b []byte) int { return writeMap(b, 0) }, - "ip_version": func(b []byte) int { return writeUint16(b, 4) }, + "ip_version": func(b []byte) int { return writeUint16(b, ipVersion) }, "languages": writeEmptyArray, "node_count": func(b []byte) int { return writeUint32(b, nodeCount) }, "record_size": func(b []byte) int { return writeUint16(b, 24) }, @@ -177,7 +178,7 @@ func writeMetadataBlockWithKeyOrder( // writeMetadataBlock writes the metadata marker followed by a standard // metadata map with the given parameters. func writeMetadataBlock(buf []byte, nodeCount uint32, buildEpoch uint64) int { - return writeMetadataBlockWithKeyOrder(buf, nodeCount, buildEpoch, metadataKeysStandard) + return writeMetadataBlockWithKeyOrder(buf, nodeCount, buildEpoch, 4, metadataKeysStandard) } func buildSimpleDB(metadataWriter func([]byte, uint32, uint64) int) []byte { @@ -268,6 +269,7 @@ func writeMetadataBlockEmptyMapLast(buf []byte, nodeCount uint32, buildEpoch uin buf, nodeCount, buildEpoch, + 4, metadataKeysEmptyMapLast, ) } @@ -279,6 +281,7 @@ func writeMetadataBlockEmptyArrayLast(buf []byte, nodeCount uint32, buildEpoch u buf, nodeCount, buildEpoch, + 4, metadataKeysEmptyArrayLast, ) } diff --git a/test-data/MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb b/test-data/MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb new file mode 100644 index 0000000000000000000000000000000000000000..9181a06c5e9a93cff47dfa5f81c9b31ff3abd84f GIT binary patch literal 1027 zcmZ9|$xjn;7{>8mD*`Tc!v)-UUFudvaYIE!1(#AQF4V2)IJ9FOLuZto`IC0`c zVnSlzL_Bcf#EHa&#Do(kPI%$q$$!C@uLqYmZ$6hNdEVqTCSoG}#zb)q*5F#K#X79V zb+{fk;6{w$Cftl$a4T-Z2HcK2a3}7JYK+y*oK$zGG4)Uyow!o4Lk8V-oTsKh23}yZ{r=ji%IOk z6yC!$-p5{ifDiEzKE^)G;1hg`&+s|Ez%2ITOMHbEzFzhBnt$hWv_^~=H6jtUPU_T{ z#MB09P@AMlZI)&=E^##>3AI&P)i!BUJETMHk}fqVNwrsc)r@4+tYlSNY_%W-wJ1e3 zlu)foRUMKcby$Yg5gAcOWmFxLF?C$V)k&FDr({Z4@V`@kc_mpJX;d5$L_AZQFRnnaSOkfLd%X$BdZ zMV97}qj}_M0R>t_k(N-RWt3?J71}_9HqoRlw5Wun9dxJzhXx2}goyUhrvnV=5JNh` zh>kI)6HMp~GdjndF0i0WEa?g>y2hGru%TOQ=?*)($DST=php~Ewx4zRP`6#{!qm=3 zt8Kf|kBl4bJ2EsJd8d8jUipEo-KmKGk8SsNENbnZ-L(wmY4i3he){a?)0?PUGL&r@ zC?kwKb#<+Rux6>i^ldL>?R(YseoiDRUFmjuvZDf#sJhxxhN*jQ_{y?a%~z)OEhDfW I)l{5+1CA4TZvX%Q literal 0 HcmV?d00001 From 5986edb490d51e6081d490786c09bab1f76feeea Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 26 Aug 2026 03:25:13 +0000 Subject: [PATCH 03/21] Add payload amplification DoS test databases The pointer fan-out fixtures exercise the value-count and depth limits. Those bound how many values a record decodes, not how many bytes they hold. An array of pointers to one large shared scalar stays under the value limit while a reader that copies each target materializes the value once per pointer. Add three fixtures, each a data section holding one 65,535-byte value and an array of pointers that all target it: - payload-amplification-dos.mmdb, 8,192 pointers to a bytes value. About 512 MiB materialized from an 82 KiB file. - payload-amplification-dos-worst-case.mmdb, 65,535 pointers. The array plus its elements decode to 65,536 values, which meets the recommended value limit without exceeding it, so a reader whose only defense is the value count does not reject it. About 4 GiB from roughly 200 KiB. - payload-amplification-dos-string.mmdb, the same shape with a UTF-8 string value. Without it the string branch of a reader's payload bound goes untested, and that is the type most bindings copy into a native string. buildPayloadAmplificationData takes the pointer count and the scalar type code, so all three share one builder. A limit on the copied bytes stops these, as can safe reuse or structural rejection. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/writer/pointerdos.go | 112 ++++++++++++-- pkg/writer/pointerdos_test.go | 142 +++++++++++++++--- ...test-payload-amplification-dos-string.mmdb | Bin 0 -> 82136 bytes ...-payload-amplification-dos-worst-case.mmdb | Bin 0 -> 196822 bytes ...ind-DB-test-payload-amplification-dos.mmdb | Bin 0 -> 82136 bytes 5 files changed, 224 insertions(+), 30 deletions(-) create mode 100644 test-data/MaxMind-DB-test-payload-amplification-dos-string.mmdb create mode 100644 test-data/MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb create mode 100644 test-data/MaxMind-DB-test-payload-amplification-dos.mmdb diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index 86cf6089..accb2c66 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -7,18 +7,22 @@ import ( ) // pointerDoSDepth is the nesting depth of the fan-out structure. Each level -// adds a factor of two to the number of leaf decodes a reader without pointer -// memoization performs. The 2**40 leaf decodes, and Θ(2**40) total decode -// operations, make a sub-kilobyte file impossible to decode without the fix -// while staying trivial to decode with it. +// adds a factor of two to the number of leaf decodes an unprotected reader that +// re-decodes every target per referencing path performs. The 2**40 leaf +// decodes, and Θ(2**40) total decode operations, make a sub-kilobyte file +// impossible to decode that way. Memoization is one defense. A cumulative work +// budget or a schema-directed path also avoids the blow-up. const pointerDoSDepth = 40 const ( // pointerDoSBuildEpoch is fixed so the generated files are reproducible. pointerDoSBuildEpoch = 1_000_000_000 - pointerDoSFixtureFilename = "MaxMind-DB-test-pointer-decoder-dos.mmdb" - pointerDoSIPv6FixtureFilename = "MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb" + pointerDoSFixtureFilename = "MaxMind-DB-test-pointer-decoder-dos.mmdb" + pointerDoSIPv6FixtureFilename = "MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb" + payloadDoSFixtureFilename = "MaxMind-DB-test-payload-amplification-dos.mmdb" + worstCasePayloadFixtureFilename = "MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb" + stringPayloadFixtureFilename = "MaxMind-DB-test-payload-amplification-dos-string.mmdb" ) // writeDataPointer writes a data-section pointer with a one-byte payload, @@ -78,6 +82,79 @@ func buildPointerFanOutData(depth int) (data []byte, top int) { return data, prev } +const ( + // payloadScalarSize is the size of the single shared bytes value that the + // pointers target. Size code 30 covers 285..65820 bytes. + payloadScalarSize = 1<<16 - 1 + // payloadPointerCount pointers each re-materialize the shared value, so a + // reader that copies the target for every pointer allocates + // payloadPointerCount*payloadScalarSize bytes, about 512 MiB, from a file of + // a few tens of kilobytes. + payloadPointerCount = 8192 + // worstCasePointerCount is the largest fan-out a reader whose only defense is + // the value count still accepts. Under flat value accounting the array plus + // its elements decode to worstCasePointerCount+1 = 65,536 values, which meets + // the recommended limit without exceeding it, so that reader does not reject + // it. Copying each target materializes worstCasePointerCount*payloadScalarSize + // bytes, about 4 GiB, from a file of about 200 KiB. A limit on the copied + // bytes stops this, as can safe reuse or structural rejection. + worstCasePointerCount = 65535 +) + +// MMDB scalar type codes used by the payload fixtures: 2 is a UTF-8 string and +// 4 is bytes. +const ( + scalarTypeString byte = 2 + scalarTypeBytes byte = 4 +) + +// buildPayloadAmplificationData builds a data section holding one large scalar +// value (string or bytes, per scalarType) at offset 0 followed by an array of +// pointers that all target it. The value count and depth limits do not bound +// this: the array stays well under the value limit, yet a reader that copies +// each pointer's target materializes payloadPointerCount*payloadScalarSize +// bytes. top is the offset of the array. +func buildPayloadAmplificationData( + pointerCount int, + scalarType byte, +) (data []byte, top int) { + const sizeCode30 = 30 + encoded := payloadScalarSize - 285 + data = append( + data, + (scalarType<<5)|sizeCode30, + byte((encoded>>8)&0xFF), + byte(encoded&0xFF), + ) + data = append(data, make([]byte, payloadScalarSize)...) + + top = len(data) + arrEncoded := pointerCount - 285 + data = append(data, 0x1E, 0x04, byte((arrEncoded>>8)&0xFF), byte(arrEncoded&0xFF)) + for range pointerCount { + data = append(data, writeDataPointer(0)...) + } + return data, top +} + +// buildPayloadAmplificationDB builds a minimal valid IPv4 MMDB whose single +// search-tree node resolves every supported lookup to the payload +// amplification record. See buildPayloadAmplificationData. +func buildPayloadAmplificationDB(pointerCount int, scalarType byte) []byte { + data, top := buildPayloadAmplificationData(pointerCount, scalarType) + + const nodeCount = 1 + recordValue := dataRecordValue24(nodeCount, top) + + buf := make([]byte, 1024+len(data)) + pos := 0 + pos += writeSearchTree(buf[pos:], recordValue) + pos += dataSeparatorSize + pos += copy(buf[pos:], data) + pos += writeMetadataBlock(buf[pos:], nodeCount, pointerDoSBuildEpoch) + return buf[:pos] +} + // buildPointerFanOutDB builds a minimal valid IPv4 MMDB whose single search-tree // node resolves every supported lookup to the fan-out data section. See // buildPointerFanOutData. @@ -142,20 +219,35 @@ func buildPointerFanOutAllSpaceDB(depth int) []byte { } // WritePointerDecoderDoSTestDB writes the databases that exercise the -// data-section pointer fan-out denial of service: a minimal single-node -// database and a conventional IPv6 database that maps all of the address space -// to the fan-out record. See buildPointerFanOutData. +// data-section pointer denial of service. Two exercise the fan-out: a minimal +// single-node database and a conventional IPv6 database that maps all of the +// address space to the fan-out record. Three exercise payload amplification: a +// moderate and a worst-case fixture with a shared bytes value, and one with a +// shared string value, the type most bindings copy into a native string. See +// buildPointerFanOutData and buildPayloadAmplificationData. func (w *Writer) WritePointerDecoderDoSTestDB() error { files := map[string][]byte{ pointerDoSFixtureFilename: buildPointerFanOutDB(pointerDoSDepth), pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( pointerDoSDepth, ), + payloadDoSFixtureFilename: buildPayloadAmplificationDB( + payloadPointerCount, + scalarTypeBytes, + ), + worstCasePayloadFixtureFilename: buildPayloadAmplificationDB( + worstCasePointerCount, + scalarTypeBytes, + ), + stringPayloadFixtureFilename: buildPayloadAmplificationDB( + payloadPointerCount, + scalarTypeString, + ), } for name, db := range files { path := filepath.Clean(filepath.Join(w.target, name)) if err := os.WriteFile(path, db, 0o644); err != nil { - return fmt.Errorf("writing pointer fan-out database %s: %w", name, err) + return fmt.Errorf("writing pointer DoS database %s: %w", name, err) } } return nil diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index 293bc8cb..49249e68 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -44,6 +44,18 @@ func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( pointerDoSDepth, ), + payloadDoSFixtureFilename: buildPayloadAmplificationDB( + payloadPointerCount, + scalarTypeBytes, + ), + worstCasePayloadFixtureFilename: buildPayloadAmplificationDB( + worstCasePointerCount, + scalarTypeBytes, + ), + stringPayloadFixtureFilename: buildPayloadAmplificationDB( + payloadPointerCount, + scalarTypeString, + ), } for name, got := range cases { //nolint:gosec // name comes from the fixed cases map, not user input. @@ -146,26 +158,7 @@ func TestPointerFanOutFixturesAreSemanticallyValid(t *testing.T) { if err != nil { t.Fatalf("reading fixture: %v", err) } - recordSizeQuarter := uint64(db.Metadata.RecordSize / 4) - if recordSizeQuarter == 0 || - uint64( - db.Metadata.NodeCount, - ) > uint64( - math.MaxInt-dataSeparatorSize, - )/recordSizeQuarter { - t.Fatalf( - "search tree dimensions overflow int: %d nodes at %d bits per record", - db.Metadata.NodeCount, - db.Metadata.RecordSize, - ) - } - searchTreeSize64 := uint64(db.Metadata.NodeCount) * recordSizeQuarter - //nolint:gosec // G115: searchTreeSize64 is checked against math.MaxInt above. - searchTreeSize := int(searchTreeSize64) - dataStart := searchTreeSize + dataSeparatorSize - if dataStart > len(raw) { - t.Fatalf("data section starts at %d, beyond file size %d", dataStart, len(raw)) - } + dataStart := dataSectionStart(t, db, raw) if uint64(outerOffset) > uint64(math.MaxInt) { t.Fatalf("outer record offset %d overflows int", outerOffset) } @@ -223,3 +216,112 @@ func smallDataPointer(t *testing.T, data []byte, offset int) int { } return int(control&0x7)<<8 | int(data[offset+1]) } + +// dataSectionStart returns the offset in raw where the data section begins, +// after the search tree and its separator, validating the tree dimensions fit +// an int. +func dataSectionStart(t *testing.T, db *maxminddb.Reader, raw []byte) int { + t.Helper() + recordSizeQuarter := uint64(db.Metadata.RecordSize / 4) + if recordSizeQuarter == 0 || + uint64(db.Metadata.NodeCount) > uint64(math.MaxInt-dataSeparatorSize)/recordSizeQuarter { + t.Fatalf("search tree dimensions overflow int: %d nodes at %d bits per record", + db.Metadata.NodeCount, db.Metadata.RecordSize) + } + searchTreeSize64 := uint64(db.Metadata.NodeCount) * recordSizeQuarter + //nolint:gosec // G115: searchTreeSize64 is checked against math.MaxInt above. + dataStart := int(searchTreeSize64) + dataSeparatorSize + if dataStart > len(raw) { + t.Fatalf("data section starts at %d, beyond file size %d", dataStart, len(raw)) + } + return dataStart +} + +// TestPayloadAmplificationFixturesAreSemanticallyValid independently validates +// each committed payload fixture: a shared 65,535-byte scalar at data-section +// offset 0 and an outer array whose elements all point to it. It inspects the +// raw data section without expanding the array. +func TestPayloadAmplificationFixturesAreSemanticallyValid(t *testing.T) { + tests := []struct { + name string + scalarType byte // MMDB major type: 2 is string, 4 is bytes + pointerCount int + }{ + {payloadDoSFixtureFilename, scalarTypeBytes, payloadPointerCount}, + {worstCasePayloadFixtureFilename, scalarTypeBytes, worstCasePointerCount}, + {stringPayloadFixtureFilename, scalarTypeString, payloadPointerCount}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + path := filepath.Clean(filepath.Join("..", "..", "test-data", test.name)) + db, err := maxminddb.Open(path) + if err != nil { + t.Fatalf("opening fixture: %v", err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Errorf("closing fixture: %v", err) + } + }) + + if got := db.Metadata.IPVersion; got != 4 { + t.Errorf("IP version = %d, want 4", got) + } + + // The lookup finds the outer array. Do not Decode: expanding it is + // deliberately hostile in an unprotected reader. + result := db.Lookup(netip.MustParseAddr("1.1.1.1")) + if err := result.Err(); err != nil { + t.Fatalf("lookup: %v", err) + } + if !result.Found() { + t.Fatal("lookup did not find a record") + } + offset := result.Offset() + if uint64(offset) > uint64(math.MaxInt) { + t.Fatalf("record offset %d overflows int", offset) + } + //nolint:gosec // G115: offset is checked against math.MaxInt above. + outerOffset := int(offset) + + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading fixture: %v", err) + } + data := raw[dataSectionStart(t, db, raw):] + + // The shared scalar sits at offset 0. Size code 30 means its length is + // the next two bytes plus 285. + wantControl := (test.scalarType << 5) | 30 + if got := data[0]; got != wantControl { + t.Errorf("scalar control byte = %#x, want %#x", got, wantControl) + } + if got := (int(data[1])<<8 | int(data[2])) + 285; got != payloadScalarSize { + t.Errorf("scalar length = %d, want %d", got, payloadScalarSize) + } + + // The outer record is an extended array (0x1E, 0x04) with a size-30 + // two-byte element count. + if outerOffset+4 > len(data) { + t.Fatalf("array header at %d is outside the data section", outerOffset) + } + if data[outerOffset] != 0x1E || data[outerOffset+1] != 0x04 { + t.Fatalf("outer record at %d is not an extended array", outerOffset) + } + count := (int(data[outerOffset+2])<<8 | int(data[outerOffset+3])) + 285 + if count != test.pointerCount { + t.Errorf("array element count = %d, want %d", count, test.pointerCount) + } + + // Every element is a two-byte pointer to the shared value at offset 0. + elem := outerOffset + 4 + for i := range test.pointerCount { + if target := smallDataPointer(t, data, elem); target != 0 { + t.Fatalf("array element %d points to offset %d, want 0", i, target) + } + elem += 2 + } + }) + } +} diff --git a/test-data/MaxMind-DB-test-payload-amplification-dos-string.mmdb b/test-data/MaxMind-DB-test-payload-amplification-dos-string.mmdb new file mode 100644 index 0000000000000000000000000000000000000000..1f70fb728f4885e87981c9f0ca779455b54ff245 GIT binary patch literal 82136 zcmeI)F-ikb5CG60NnxMHDtH1fU~6MSIy=GTXZIUdlHIVo5#v!T(|Qdn!8?dnR^C92 zm68is_-3k^ff;xQm|ht4!XWI##pmlzM@fJH0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0{>HB z*dM+f21$Sb0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5*BwZOyk$0XiP%BncZ>&5%|w5(#Y&Mxa_5xZ;= zuj?keNljbU)h6uij?3!zIE<&Oa$aO#d*4qLj_;qsq=;Rd#x`Z$dYMknQrnG- z)aFgO?6zxPM&N-*xCr8v$NPdcAvNgcFFEy#G_bP(0UCk6YpT?th@mc zD-Boho2h30WG4A~Vb}}9a8S>??ZL)KfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009F3Q{ZH< z+nxqZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z0D&KYm-nwpe3+C~ahBJM&+BPf#b%w|)XgGx*&^Q6O?ID}wydj7=pV*q^*0XV>8hL; zSz6Zl?NLAM>-_00Op4gWX>3#0t(WQIDz)9XNNwJf%Wi-6V^l7GUkoniv6`*oEVW@U cs_G(TdA+K-*Zr~HxJh~46j@t7rcF5h20#@~MF0Q* literal 0 HcmV?d00001 diff --git a/test-data/MaxMind-DB-test-payload-amplification-dos.mmdb b/test-data/MaxMind-DB-test-payload-amplification-dos.mmdb new file mode 100644 index 0000000000000000000000000000000000000000..9d711949494efc9f175f0f3cecaef4e316fc0ed7 GIT binary patch literal 82136 zcmeI)Ax;BP5CG6$Qh~2QHJpG8U@^3*!yz`myPvY8-EDTagmM%FqLyo*NVo$5h2jQ; zq9DBhf;XdPCYedz0j3uQy)XznasTv~@_O-pJ}s-*th0-{S;Q_| z#H+f=u2a*Nb+rk5yW_I@Jr3jPs+<>DTGsjH{@(Xfh2y)YFezder?E|0w_c``v($Fu zBDHx_F1zj8mr=R=Inh6z$7;5Uv($$EsH%&U<@KuS9=FSS<0j>GQ)F#OL#3-6mt A;s5{u literal 0 HcmV?d00001 From 3056e15692596ca442cd859cec202444fbdfc917 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 26 Aug 2026 22:23:03 +0000 Subject: [PATCH 04/21] Add decoder resource limit boundary fixtures Name the two recommended limits, 65,536 decoded values and 2 MiB of copied payload, and add fixtures that sit exactly on each boundary so a reader can check both sides of it: - decoder-value-limit and -over, at 65,536 and 65,537 decoded values - decoder-payload-limit and -over, at 2,097,152 and 2,097,153 payload bytes - decoder-value-limit-pointer-heavy, 65,535 values reached through a depth 15 fan-out rather than a flat array - metadata-payload-limit, which passes the payload limit while the reader is still opening the file Add writeScalar and writeArrayHeader so the builders share one size encoder, and pull the repeated database layout into buildSingleRecordDB. This also fills the payload of the three existing amplification fixtures, which were left as NUL. A NUL-filled string measures length 0 through strlen, so a binding that copies C strings passed the string fixture without ever reaching the copy path it exists to exercise. Their bytes change. Their shape does not. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/writer/pointerdos.go | 244 +++++++++++++++--- pkg/writer/pointerdos_test.go | 136 +++++++++- ...nd-DB-test-decoder-payload-limit-over.mmdb | Bin 0 -> 65852 bytes ...MaxMind-DB-test-decoder-payload-limit.mmdb | Bin 0 -> 65851 bytes ...Mind-DB-test-decoder-value-limit-over.mmdb | Bin 0 -> 131287 bytes ...est-decoder-value-limit-pointer-heavy.mmdb | Bin 0 -> 301 bytes .../MaxMind-DB-test-decoder-value-limit.mmdb | Bin 0 -> 131285 bytes ...axMind-DB-test-metadata-payload-limit.mmdb | Bin 0 -> 65811 bytes ...test-payload-amplification-dos-string.mmdb | Bin 82136 -> 82136 bytes ...-payload-amplification-dos-worst-case.mmdb | Bin 196822 -> 196822 bytes ...ind-DB-test-payload-amplification-dos.mmdb | Bin 82136 -> 82136 bytes 11 files changed, 345 insertions(+), 35 deletions(-) create mode 100644 test-data/MaxMind-DB-test-decoder-payload-limit-over.mmdb create mode 100644 test-data/MaxMind-DB-test-decoder-payload-limit.mmdb create mode 100644 test-data/MaxMind-DB-test-decoder-value-limit-over.mmdb create mode 100644 test-data/MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb create mode 100644 test-data/MaxMind-DB-test-decoder-value-limit.mmdb create mode 100644 test-data/MaxMind-DB-test-metadata-payload-limit.mmdb diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index accb2c66..0eaeee86 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -18,11 +18,17 @@ const ( // pointerDoSBuildEpoch is fixed so the generated files are reproducible. pointerDoSBuildEpoch = 1_000_000_000 - pointerDoSFixtureFilename = "MaxMind-DB-test-pointer-decoder-dos.mmdb" - pointerDoSIPv6FixtureFilename = "MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb" - payloadDoSFixtureFilename = "MaxMind-DB-test-payload-amplification-dos.mmdb" - worstCasePayloadFixtureFilename = "MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb" - stringPayloadFixtureFilename = "MaxMind-DB-test-payload-amplification-dos-string.mmdb" + pointerDoSFixtureFilename = "MaxMind-DB-test-pointer-decoder-dos.mmdb" + pointerDoSIPv6FixtureFilename = "MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb" + pointerValueLimitFixtureFilename = "MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb" + payloadDoSFixtureFilename = "MaxMind-DB-test-payload-amplification-dos.mmdb" + worstCasePayloadFixtureFilename = "MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb" + stringPayloadFixtureFilename = "MaxMind-DB-test-payload-amplification-dos-string.mmdb" + valueLimitFixtureFilename = "MaxMind-DB-test-decoder-value-limit.mmdb" + valueLimitOverFixtureFilename = "MaxMind-DB-test-decoder-value-limit-over.mmdb" + payloadLimitFixtureFilename = "MaxMind-DB-test-decoder-payload-limit.mmdb" + payloadLimitOverFixtureFilename = "MaxMind-DB-test-decoder-payload-limit-over.mmdb" + metadataLimitFixtureFilename = "MaxMind-DB-test-metadata-payload-limit.mmdb" ) // writeDataPointer writes a data-section pointer with a one-byte payload, @@ -83,6 +89,9 @@ func buildPointerFanOutData(depth int) (data []byte, top int) { } const ( + recommendedValueLimit = 1 << 16 + recommendedPayloadLimit = 1 << 21 + // payloadScalarSize is the size of the single shared bytes value that the // pointers target. Size code 30 covers 285..65820 bytes. payloadScalarSize = 1<<16 - 1 @@ -108,6 +117,52 @@ const ( scalarTypeBytes byte = 4 ) +func writeScalar(buf []byte, size int, scalarType byte) int { + pos := 1 + switch { + case size <= 28: + buf[0] = (scalarType << 5) | byte(size&0x1F) + case size <= 284: + buf[0] = (scalarType << 5) | 29 + buf[1] = byte(size - 29) + pos++ + case size <= 65820: + buf[0] = (scalarType << 5) | 30 + encoded := size - 285 + buf[1] = byte((encoded >> 8) & 0xFF) + buf[2] = byte(encoded & 0xFF) + pos += 2 + default: + panic(fmt.Sprintf("scalar size %d is unsupported by this fixture writer", size)) + } + clear(buf[pos : pos+size]) + return pos + size +} + +func writeArrayHeader(buf []byte, size int) int { + switch { + case size <= 28: + buf[0] = byte(size & 0x1F) + buf[1] = 4 + return 2 + case size <= 284: + buf[0] = 29 + buf[1] = 4 + buf[2] = byte(size - 29) + return 3 + case size <= 65820: + buf[0] = 30 + buf[1] = 4 + encoded := size - 285 + buf[2] = byte((encoded >> 8) & 0xFF) + buf[3] = byte(encoded & 0xFF) + return 4 + default: + //nolint:gosec // size is a bounded fixture element count. + return writeLargeArray(buf, uint32(size)) + } +} + // buildPayloadAmplificationData builds a data section holding one large scalar // value (string or bytes, per scalarType) at offset 0 followed by an array of // pointers that all target it. The value count and depth limits do not bound @@ -118,23 +173,54 @@ func buildPayloadAmplificationData( pointerCount int, scalarType byte, ) (data []byte, top int) { - const sizeCode30 = 30 - encoded := payloadScalarSize - 285 - data = append( - data, - (scalarType<<5)|sizeCode30, - byte((encoded>>8)&0xFF), - byte(encoded&0xFF), - ) - data = append(data, make([]byte, payloadScalarSize)...) + data = make([]byte, payloadScalarSize+16+pointerCount*2) + pos := writeScalar(data, payloadScalarSize, scalarType) + // The payload must not be left as NUL. A NUL-filled string measures length 0 + // through strlen, so a binding that copies C strings would skip the copy path + // this fixture exists to exercise. + for i := pos - payloadScalarSize; i < pos; i++ { + data[i] = 'a' + } + + top = pos + pos += writeArrayHeader(data[pos:], pointerCount) + for range pointerCount { + pos += copy(data[pos:], writeDataPointer(0)) + } + return data[:pos], top +} - top = len(data) - arrEncoded := pointerCount - 285 - data = append(data, 0x1E, 0x04, byte((arrEncoded>>8)&0xFF), byte(arrEncoded&0xFF)) +// buildValueLimitData produces one array node followed by pointerCount scalar +// nodes. The scalar has no string/bytes payload, so only the decoded-value +// budget determines whether the record is accepted. +func buildValueLimitData(pointerCount int) (data []byte, top int) { + data = make([]byte, 16+pointerCount*2) + data[0] = 0xA0 // uint16 with value 0 + pos := 1 + top = pos + pos += writeArrayHeader(data[pos:], pointerCount) for range pointerCount { - data = append(data, writeDataPointer(0)...) + pos += copy(data[pos:], writeDataPointer(0)) } - return data, top + return data[:pos], top +} + +// buildPayloadLimitData produces 32 references to a 65,535-byte value and one +// reference to a 32- or 33-byte value. Those totals are exactly 2 MiB and one +// byte over 2 MiB respectively, while the decoded-value count remains tiny. +func buildPayloadLimitData(smallSize int) (data []byte, top int) { + const largePointerCount = 32 + data = make([]byte, payloadScalarSize+smallSize+128) + pos := writeScalar(data, smallSize, scalarTypeBytes) + largeOffset := pos + pos += writeScalar(data[pos:], payloadScalarSize, scalarTypeBytes) + top = pos + pos += writeArrayHeader(data[pos:], largePointerCount+1) + for range largePointerCount { + pos += copy(data[pos:], writeDataPointer(largeOffset)) + } + pos += copy(data[pos:], writeDataPointer(0)) + return data[:pos], top } // buildPayloadAmplificationDB builds a minimal valid IPv4 MMDB whose single @@ -142,8 +228,14 @@ func buildPayloadAmplificationData( // amplification record. See buildPayloadAmplificationData. func buildPayloadAmplificationDB(pointerCount int, scalarType byte) []byte { data, top := buildPayloadAmplificationData(pointerCount, scalarType) + return buildSingleRecordDB(data, top) +} +func buildSingleRecordDB(data []byte, top int) []byte { const nodeCount = 1 + // A data record value is the data-section offset plus the node count plus the + // 16-byte data section separator, which is how a reader recovers the offset: + // offset = recordValue - nodeCount - dataSeparatorSize. recordValue := dataRecordValue24(nodeCount, top) buf := make([]byte, 1024+len(data)) @@ -155,27 +247,86 @@ func buildPayloadAmplificationDB(pointerCount int, scalarType byte) []byte { return buf[:pos] } -// buildPointerFanOutDB builds a minimal valid IPv4 MMDB whose single search-tree -// node resolves every supported lookup to the fan-out data section. See -// buildPointerFanOutData. -func buildPointerFanOutDB(depth int) []byte { - data, top := buildPointerFanOutData(depth) +func buildValueLimitDB(pointerCount int) []byte { + data, top := buildValueLimitData(pointerCount) + return buildSingleRecordDB(data, top) +} - const nodeCount = 1 - // A data record value is the data-section offset plus the node count plus - // the 16-byte data section separator, which is how a reader recovers the - // offset: offset = recordValue - nodeCount - dataSeparatorSize. - recordValue := dataRecordValue24(nodeCount, top) +func buildPayloadLimitDB(smallSize int) []byte { + data, top := buildPayloadLimitData(smallSize) + return buildSingleRecordDB(data, top) +} - buf := make([]byte, 1024+len(data)) +func writeMetadataLimitBlock(buf []byte, nodeCount uint32, buildEpoch uint64) int { pos := 0 - pos += writeSearchTree(buf[pos:], recordValue) + copy(buf[pos:], metadataMarker) + pos += len(metadataMarker) + metadataStart := pos + pos += writeMap(buf[pos:], len(metadataKeysStandard)) + + var databaseTypeOffset int + for _, key := range metadataKeysStandard { + pos += writeMetaKey(buf[pos:], key) + switch key { + case "binary_format_major_version": + pos += writeUint16(buf[pos:], 2) + case "binary_format_minor_version": + pos += writeUint16(buf[pos:], 0) + case "build_epoch": + pos += writeUint64(buf[pos:], buildEpoch) + case "database_type": + databaseTypeOffset = pos - metadataStart + pos += writeScalar(buf[pos:], payloadScalarSize, scalarTypeString) + for i := pos - payloadScalarSize; i < pos; i++ { + buf[i] = 'T' + } + case "description": + pos += writeMap(buf[pos:], 0) + case "ip_version": + pos += writeUint16(buf[pos:], 4) + case "languages": + // The pointers below target the database_type string, so that key must + // already be written. Offset 0 is the metadata map's own control byte, + // which would silently make this a pointer cycle instead of a payload + // fixture. + if databaseTypeOffset == 0 { + panic("languages must be written after database_type") + } + const pointerCount = 33 + pos += writeArrayHeader(buf[pos:], pointerCount) + for range pointerCount { + pos += copy(buf[pos:], writeDataPointer(databaseTypeOffset)) + } + case "node_count": + pos += writeUint32(buf[pos:], nodeCount) + case "record_size": + pos += writeUint16(buf[pos:], 24) + default: + panic("unknown metadata key: " + key) + } + } + return pos +} + +func buildMetadataLimitDB() []byte { + const nodeCount = 1 + const recordValue = nodeCount + dataSeparatorSize + buf := make([]byte, 128*1024) + pos := writeSearchTree(buf, recordValue) pos += dataSeparatorSize - pos += copy(buf[pos:], data) - pos += writeMetadataBlock(buf[pos:], nodeCount, pointerDoSBuildEpoch) + pos += writeMap(buf[pos:], 0) + pos += writeMetadataLimitBlock(buf[pos:], nodeCount, pointerDoSBuildEpoch) return buf[:pos] } +// buildPointerFanOutDB builds a minimal valid IPv4 MMDB whose single search-tree +// node resolves every supported lookup to the fan-out data section. See +// buildPointerFanOutData. +func buildPointerFanOutDB(depth int) []byte { + data, top := buildPointerFanOutData(depth) + return buildSingleRecordDB(data, top) +} + // buildPointerFanOutAllSpaceDB builds a conventional IPv6 MMDB that maps the // entire address space to the fan-out data record, so opening it, looking up // any address, and decoding the result reproduces the denial of service. A @@ -218,12 +369,21 @@ func buildPointerFanOutAllSpaceDB(depth int) []byte { return buf[:pos] } +// pointerValueLimitDepth is the deepest fan-out whose flat value count a reader +// at the recommended limit still accepts. A binary fan-out of depth d decodes +// 2**(d+1) - 1 values, so depth 15 gives 65,535, one below the limit, and depth +// 16 would give 131,071. Depth 15 is therefore the only depth that lands on the +// boundary from below. +const pointerValueLimitDepth = 15 + // WritePointerDecoderDoSTestDB writes the databases that exercise the // data-section pointer denial of service. Two exercise the fan-out: a minimal // single-node database and a conventional IPv6 database that maps all of the // address space to the fan-out record. Three exercise payload amplification: a // moderate and a worst-case fixture with a shared bytes value, and one with a -// shared string value, the type most bindings copy into a native string. See +// shared string value, the type most bindings copy into a native string. It +// also writes exact boundary fixtures for both recommended limits and a +// metadata fixture that exceeds the payload limit while opening. See // buildPointerFanOutData and buildPayloadAmplificationData. func (w *Writer) WritePointerDecoderDoSTestDB() error { files := map[string][]byte{ @@ -231,6 +391,9 @@ func (w *Writer) WritePointerDecoderDoSTestDB() error { pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( pointerDoSDepth, ), + pointerValueLimitFixtureFilename: buildPointerFanOutDB( + pointerValueLimitDepth, + ), payloadDoSFixtureFilename: buildPayloadAmplificationDB( payloadPointerCount, scalarTypeBytes, @@ -243,6 +406,19 @@ func (w *Writer) WritePointerDecoderDoSTestDB() error { payloadPointerCount, scalarTypeString, ), + valueLimitFixtureFilename: buildValueLimitDB( + recommendedValueLimit - 1, + ), + valueLimitOverFixtureFilename: buildValueLimitDB( + recommendedValueLimit, + ), + payloadLimitFixtureFilename: buildPayloadLimitDB( + recommendedPayloadLimit - 32*payloadScalarSize, + ), + payloadLimitOverFixtureFilename: buildPayloadLimitDB( + recommendedPayloadLimit - 32*payloadScalarSize + 1, + ), + metadataLimitFixtureFilename: buildMetadataLimitDB(), } for name, db := range files { path := filepath.Clean(filepath.Join(w.target, name)) diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index 49249e68..5ea567b9 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -44,6 +44,9 @@ func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( pointerDoSDepth, ), + pointerValueLimitFixtureFilename: buildPointerFanOutDB( + pointerValueLimitDepth, + ), payloadDoSFixtureFilename: buildPayloadAmplificationDB( payloadPointerCount, scalarTypeBytes, @@ -56,6 +59,19 @@ func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { payloadPointerCount, scalarTypeString, ), + valueLimitFixtureFilename: buildValueLimitDB( + recommendedValueLimit - 1, + ), + valueLimitOverFixtureFilename: buildValueLimitDB( + recommendedValueLimit, + ), + payloadLimitFixtureFilename: buildPayloadLimitDB( + recommendedPayloadLimit - 32*payloadScalarSize, + ), + payloadLimitOverFixtureFilename: buildPayloadLimitDB( + recommendedPayloadLimit - 32*payloadScalarSize + 1, + ), + metadataLimitFixtureFilename: buildMetadataLimitDB(), } for name, got := range cases { //nolint:gosec // name comes from the fixed cases map, not user input. @@ -69,23 +85,89 @@ func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { } } +func TestDecoderLimitBoundaryData(t *testing.T) { + for _, pointerCount := range []int{ + recommendedValueLimit - 1, + recommendedValueLimit, + } { + data, top := buildValueLimitData(pointerCount) + if top != 1 { + t.Errorf("value boundary top offset = %d, want 1", top) + } + if data[0] != 0xA0 { + t.Errorf("value boundary scalar = %#x, want 0xa0", data[0]) + } + if len(data) != 1+4+pointerCount*2 { + t.Errorf("value boundary length = %d, want %d", len(data), 1+4+pointerCount*2) + } + for offset := top + 4; offset < len(data); offset += 2 { + if target := smallDataPointer(t, data, offset); target != 0 { + t.Fatalf("value boundary pointer at %d targets %d, want 0", offset, target) + } + } + } + + for _, smallSize := range []int{32, 33} { + data, top := buildPayloadLimitData(smallSize) + if top <= payloadScalarSize { + t.Errorf("payload boundary top offset = %d, want after large scalar", top) + } + if len(data) <= top { + t.Fatalf("payload boundary data ends before outer array at %d", top) + } + gotPayload := sumPointedPayload(t, data, top) + wantPayload := recommendedPayloadLimit + smallSize - 32 + if gotPayload != wantPayload { + t.Errorf("payload boundary total = %d, want %d", gotPayload, wantPayload) + } + } +} + +func TestMetadataLimitFixtureIsSemanticallyValid(t *testing.T) { + path := filepath.Clean(filepath.Join("..", "..", "test-data", metadataLimitFixtureFilename)) + db, err := maxminddb.Open(path) + if err != nil { + t.Fatalf("opening metadata limit fixture: %v", err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Errorf("closing metadata limit fixture: %v", err) + } + }) + + if got := len(db.Metadata.DatabaseType); got != payloadScalarSize { + t.Errorf("database type length = %d, want %d", got, payloadScalarSize) + } + if got := len(db.Metadata.Languages); got != 33 { + t.Fatalf("language count = %d, want 33", got) + } + for i, language := range db.Metadata.Languages { + if got := len(language); got != payloadScalarSize { + t.Errorf("language %d length = %d, want %d", i, got, payloadScalarSize) + } + } +} + func TestPointerFanOutFixturesAreSemanticallyValid(t *testing.T) { tests := []struct { name string ipVersion uint nodeCount uint + depth int addresses []string }{ { name: pointerDoSFixtureFilename, ipVersion: 4, nodeCount: 1, + depth: pointerDoSDepth, addresses: []string{"0.0.0.0", "203.0.113.9", "255.255.255.255"}, }, { name: pointerDoSIPv6FixtureFilename, ipVersion: 6, nodeCount: 97, + depth: pointerDoSDepth, addresses: []string{ "0.0.0.0", "203.0.113.9", @@ -95,6 +177,13 @@ func TestPointerFanOutFixturesAreSemanticallyValid(t *testing.T) { "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", }, }, + { + name: pointerValueLimitFixtureFilename, + ipVersion: 4, + nodeCount: 1, + depth: 15, + addresses: []string{"1.1.1.1"}, + }, } for _, test := range tests { @@ -166,7 +255,7 @@ func TestPointerFanOutFixturesAreSemanticallyValid(t *testing.T) { if outerOffsetInt >= len(raw)-dataStart { t.Fatalf("outer record offset %d is outside the data section", outerOffset) } - verifyPointerFanOutRecord(t, raw[dataStart:], outerOffsetInt, pointerDoSDepth) + verifyPointerFanOutRecord(t, raw[dataStart:], outerOffsetInt, test.depth) }) } } @@ -205,6 +294,51 @@ func verifyPointerFanOutRecord(t *testing.T, data []byte, offset, depth int) { } } +// scalarPayloadSize returns the declared payload length of the string or bytes +// scalar at offset, reading only its control bytes. +func scalarPayloadSize(t *testing.T, data []byte, offset int) int { + t.Helper() + if offset < 0 || offset+3 > len(data) { + t.Fatalf("scalar at offset %d is outside the data section", offset) + } + control := data[offset] + major := control >> 5 + if major != scalarTypeString && major != scalarTypeBytes { + t.Fatalf("value at offset %d has major type %d, want a string or bytes", offset, major) + } + size := int(control & 0x1F) + switch { + case size <= 28: + return size + case size == 29: + return int(data[offset+1]) + 29 + case size == 30: + return (int(data[offset+1])<<8 | int(data[offset+2])) + 285 + default: + t.Fatalf("scalar at offset %d uses size code %d, which this test cannot read", offset, size) + return 0 + } +} + +// sumPointedPayload adds up the payload that every element of the array at top +// references, which is what a reader charging each occurrence materializes. It +// reads the generated bytes rather than recomputing the builder's arithmetic, so +// a change to the encoding or the element count fails the assertion. +func sumPointedPayload(t *testing.T, data []byte, top int) int { + t.Helper() + if top+3 > len(data) || data[top] != 29 || data[top+1] != 4 { + t.Fatalf("value at offset %d is not a size-29 array header", top) + } + count := int(data[top+2]) + 29 + total := 0 + elem := top + 3 + for range count { + total += scalarPayloadSize(t, data, smallDataPointer(t, data, elem)) + elem += 2 + } + return total +} + func smallDataPointer(t *testing.T, data []byte, offset int) int { t.Helper() if offset < 0 || offset+2 > len(data) { diff --git a/test-data/MaxMind-DB-test-decoder-payload-limit-over.mmdb b/test-data/MaxMind-DB-test-decoder-payload-limit-over.mmdb new file mode 100644 index 0000000000000000000000000000000000000000..5d8d82b83e0cdfe672868a61be5b57b2b8faf3de GIT binary patch literal 65852 zcmeIvu}T9$5C-6lT;U_w2!g$38e1C^(%D%Yo4Ylhn7eRy5%p07Eo^-aE5UaVJ1gHn z)JjQ8OTV3J|A86!W)OyhFbpg2Hd_hSzjq(6>!E}I0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0tEh{z;>4H?EbOB!}I$jUQVjI+$);-+wrujV|$gKH0?b0`8=LAZGMs3u4?N0u(8^% z>fh}!o-V8NGEa-9INjX%KFR*gQ<#*okJH$tyuVtcgQL{-<1%$cTP^ypu`i=)@v|a3 lJdgEk8E2^r*{E*Hlo!pi?jOH~4aRLMnzqcl>N?$rtxqq~R1W|E literal 0 HcmV?d00001 diff --git a/test-data/MaxMind-DB-test-decoder-payload-limit.mmdb b/test-data/MaxMind-DB-test-decoder-payload-limit.mmdb new file mode 100644 index 0000000000000000000000000000000000000000..bb09c1589d01a2ddc7840d6d3835509af62bfa36 GIT binary patch literal 65851 zcmeIvF-ikb5C-5EP2mx21n~fNscdabNM~oUyzD;XirEdj8!;Y5&?2qZuoAq3*jaf4 zQ7a{umVR%l_Ycg#H@z_Eg+W+*x0`FhukYRa%dbp8fB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009F3QDCRv-`)FTh5M(sal9IrRk5Gfv)7YJS;c0Voz~4PcG)bR*G+brnzpQ~RoGZ> zm(|a97)=)CMUka>ou6%OeBI>W<}r+m*u_a~Q`Rl#>F_wU-Ka=y-jwt1bL{i5oPV$A mA6>+1x`@-%hW@asiuK(+P9xKjdc_bYU@KwNwsw6gSzJst#?Pr|Hsz- zj-hILyElqEuW!%t(+`(VZ%`Xb)KFxV4jo@zb4-=tsDtg2)PAgO_j95s_vCgx?KncQ YcHXekzIW+W%Cai3_Q9xVANdI9-?kW0!vFvP literal 0 HcmV?d00001 diff --git a/test-data/MaxMind-DB-test-decoder-value-limit.mmdb b/test-data/MaxMind-DB-test-decoder-value-limit.mmdb new file mode 100644 index 0000000000000000000000000000000000000000..84cde97751d53cd49879232123bdd0d352851ffa GIT binary patch literal 131285 zcmeIuJx;?w5C+gCN6o<#x528dYR5HQrC~G)Rk?$ v?6Y)B;HMO+Z@+r{&qggZS2<7W$ET|sOIsebk!<#eJ<@L z?EbBD`?C(?*)rcYRa%t#dT;mp)5*hgm^5*Sv)HF@}gR$uK&6>VRU*M+pA@~O8wzzbX5O$&f3zX YsxHfRcnTrxjJs5quBrNbpElv(6aG71)c^nh literal 0 HcmV?d00001 diff --git a/test-data/MaxMind-DB-test-payload-amplification-dos-string.mmdb b/test-data/MaxMind-DB-test-payload-amplification-dos-string.mmdb index 1f70fb728f4885e87981c9f0ca779455b54ff245..0dee394d7d3c94bb3803a354acf777e32596390d 100644 GIT binary patch literal 82136 zcmeIuF-ikb6a~3QnHB z*dM+f21$Sb0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5*BwZOyk$0XiP%BncZ>&5%|w5(#Y&Mxa_5xZ;= zuj?keNljbU)h6uij?3!zIE<&Oa$aO#d*4qLj_;qsq=;Rd#x`Z$dYMknQrnG- z)aFgO?6zxPM&pQL#Zp zs6d(F+*e-dKIsg?co4?nATPUZ6aoYY5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAQ0opXtzBL zjsO7y1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009EO z0N-*xCr8v$NPdcAvNgcFFEy#G_bP(0UCk6YpT?th@mc zD-Boho2h30WG4A~Vb}}9a8S>??ZL)KfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009F3Q{ZH< z+nxqZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z0D&KYm-nwpe3+C~ahBJM&+BPf#b%w|)XgGx*&^Q6O?ID}wydj7=pV*q^*0XV>8hL; zSz6Zl?NLAM>-_00Op4gWX>3#0t(WQIDz)9XNNwJf%Wi-6V^l7GUkoniv6`*oEVW@U cs_G(TdA+K-*Zr~HxJh~46j@t7rcF5h20#@~MF0Q* diff --git a/test-data/MaxMind-DB-test-payload-amplification-dos.mmdb b/test-data/MaxMind-DB-test-payload-amplification-dos.mmdb index 9d711949494efc9f175f0f3cecaef4e316fc0ed7..94728552681999028729a624aa1185c20a4cca22 100644 GIT binary patch literal 82136 zcmeIuAx;BP6a~;3dIfx zb&zfVbMBKjxi2|`FdBqW*vsShdlUi$2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FilacsTwz z4vqi;0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF z5FkK+009C72oNAZfB*pk1PBoLZ-J-RuUWjCl~r+)*Q?LVd0EAFlU>#AD)!kb-qdY& zo7%3dt8F;gugmIh9j5bjxh%5O)cN({LD=Q=;U&z9*vEP7Qr2&pbas)tep;k1Z_B3N t&Av^_=J&<$d>O07IxbQdhLfr;QkK{2s(;>%4W@0%>$b?c@;+_D(GR8!Q7`}i literal 82136 zcmeI)Ax;BP5CG6$Qh~2QHJpG8U@^3*!yz`myPvY8-EDTagmM%FqLyo*NVo$5h2jQ; zq9DBhf;XdPCYedz0j3uQy)XznasTv~@_O-pJ}s-*th0-{S;Q_| z#H+f=u2a*Nb+rk5yW_I@Jr3jPs+<>DTGsjH{@(Xfh2y)YFezder?E|0w_c``v($Fu zBDHx_F1zj8mr=R=Inh6z$7;5Uv($$EsH%&U<@KuS9=FSS<0j>GQ)F#OL#3-6mt A;s5{u From b20fa64077c5c43e1e4f5251b98bedcf92af727f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 29 Aug 2026 12:26:26 -0700 Subject: [PATCH 05/21] Add cumulative decode path budget fixture A reader that bounds each navigation step on its own can still do unbounded work across one path lookup. This fixture measures that. It holds a map whose first 511 keys each point to one shared 4 KiB string, followed by an inline "target" key holding a 4,091-byte value. Reading the 511 decoy keys and the target key costs 2,093,062 bytes. Decoding the selected value takes the total to 2,097,153, one byte above the recommended 2 MiB budget. A reader that shares one budget across navigation and the selected decode rejects the lookup. A reader that starts a fresh budget for the selected value does not. Add writeMapHeader for the extended size form the 512-entry map needs. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/writer/pointerdos.go | 69 +++++++++++++++++- pkg/writer/pointerdos_test.go | 69 +++++++++++++++++- ...ind-DB-test-decode-path-shared-budget.mmdb | Bin 0 -> 10457 bytes 3 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 test-data/MaxMind-DB-test-decode-path-shared-budget.mmdb diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index 0eaeee86..8d371248 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -29,6 +29,7 @@ const ( payloadLimitFixtureFilename = "MaxMind-DB-test-decoder-payload-limit.mmdb" payloadLimitOverFixtureFilename = "MaxMind-DB-test-decoder-payload-limit-over.mmdb" metadataLimitFixtureFilename = "MaxMind-DB-test-metadata-payload-limit.mmdb" + decodePathBudgetFixtureFilename = "MaxMind-DB-test-decode-path-shared-budget.mmdb" ) // writeDataPointer writes a data-section pointer with a one-byte payload, @@ -91,6 +92,9 @@ func buildPointerFanOutData(depth int) (data []byte, top int) { const ( recommendedValueLimit = 1 << 16 recommendedPayloadLimit = 1 << 21 + decodePathDecoyCount = 511 + decodePathSharedKeySize = 4096 + decodePathValueSize = 4091 // payloadScalarSize is the size of the single shared bytes value that the // pointers target. Size code 30 covers 285..65820 bytes. @@ -163,6 +167,26 @@ func writeArrayHeader(buf []byte, size int) int { } } +func writeMapHeader(buf []byte, size int) int { + switch { + case size <= 28: + buf[0] = (7 << 5) | byte(size&0x1F) + return 1 + case size <= 284: + buf[0] = (7 << 5) | 29 + buf[1] = byte(size - 29) + return 2 + case size <= 65820: + buf[0] = (7 << 5) | 30 + encoded := size - 285 + buf[1] = byte((encoded >> 8) & 0xFF) + buf[2] = byte(encoded & 0xFF) + return 3 + default: + panic(fmt.Sprintf("map size %d is unsupported by this fixture writer", size)) + } +} + // buildPayloadAmplificationData builds a data section holding one large scalar // value (string or bytes, per scalarType) at offset 0 followed by an array of // pointers that all target it. The value count and depth limits do not bound @@ -223,6 +247,38 @@ func buildPayloadLimitData(smallSize int) (data []byte, top int) { return data[:pos], top } +// buildDecodePathSharedBudgetData produces a map whose first 511 keys point to +// one shared 4 KiB string. Reading those keys and the final inline "target" key +// consumes 2,093,062 bytes. The selected 4,091-byte value then takes the total +// to 2,097,153 bytes, exactly one byte above the recommended 2 MiB budget. This +// verifies that path navigation and selected-value decoding share one budget. +func buildDecodePathSharedBudgetData() (data []byte, top int) { + data = make([]byte, 16*1024) + pos := writeScalar(data, decodePathSharedKeySize, scalarTypeString) + for i := pos - decodePathSharedKeySize; i < pos; i++ { + data[i] = 'k' + } + + top = pos + pos += writeMapHeader(data[pos:], decodePathDecoyCount+1) + for range decodePathDecoyCount { + pos += copy(data[pos:], writeDataPointer(0)) + data[pos] = 0 + data[pos+1] = 7 // extended type 14, size 0: false + pos += 2 + } + + const target = "target" + pos += writeScalar(data[pos:], len(target), scalarTypeString) + copy(data[pos-len(target):pos], target) + pos += writeScalar(data[pos:], decodePathValueSize, scalarTypeString) + for i := pos - decodePathValueSize; i < pos; i++ { + data[i] = 'v' + } + + return data[:pos], top +} + // buildPayloadAmplificationDB builds a minimal valid IPv4 MMDB whose single // search-tree node resolves every supported lookup to the payload // amplification record. See buildPayloadAmplificationData. @@ -257,6 +313,11 @@ func buildPayloadLimitDB(smallSize int) []byte { return buildSingleRecordDB(data, top) } +func buildDecodePathSharedBudgetDB() []byte { + data, top := buildDecodePathSharedBudgetData() + return buildSingleRecordDB(data, top) +} + func writeMetadataLimitBlock(buf []byte, nodeCount uint32, buildEpoch uint64) int { pos := 0 copy(buf[pos:], metadataMarker) @@ -383,8 +444,9 @@ const pointerValueLimitDepth = 15 // moderate and a worst-case fixture with a shared bytes value, and one with a // shared string value, the type most bindings copy into a native string. It // also writes exact boundary fixtures for both recommended limits and a -// metadata fixture that exceeds the payload limit while opening. See -// buildPointerFanOutData and buildPayloadAmplificationData. +// metadata fixture that exceeds the payload limit while opening. A small path +// fixture verifies that navigation and selected-value decoding share the same +// budget. See buildPointerFanOutData and buildPayloadAmplificationData. func (w *Writer) WritePointerDecoderDoSTestDB() error { files := map[string][]byte{ pointerDoSFixtureFilename: buildPointerFanOutDB(pointerDoSDepth), @@ -418,7 +480,8 @@ func (w *Writer) WritePointerDecoderDoSTestDB() error { payloadLimitOverFixtureFilename: buildPayloadLimitDB( recommendedPayloadLimit - 32*payloadScalarSize + 1, ), - metadataLimitFixtureFilename: buildMetadataLimitDB(), + metadataLimitFixtureFilename: buildMetadataLimitDB(), + decodePathBudgetFixtureFilename: buildDecodePathSharedBudgetDB(), } for name, db := range files { path := filepath.Clean(filepath.Join(w.target, name)) diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index 5ea567b9..8d67f910 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -71,7 +71,8 @@ func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { payloadLimitOverFixtureFilename: buildPayloadLimitDB( recommendedPayloadLimit - 32*payloadScalarSize + 1, ), - metadataLimitFixtureFilename: buildMetadataLimitDB(), + metadataLimitFixtureFilename: buildMetadataLimitDB(), + decodePathBudgetFixtureFilename: buildDecodePathSharedBudgetDB(), } for name, got := range cases { //nolint:gosec // name comes from the fixed cases map, not user input. @@ -123,6 +124,72 @@ func TestDecoderLimitBoundaryData(t *testing.T) { } } +func TestDecodePathSharedBudgetData(t *testing.T) { + data, top := buildDecodePathSharedBudgetData() + if top != 3+decodePathSharedKeySize { + t.Fatalf("map offset = %d, want %d", top, 3+decodePathSharedKeySize) + } + if got := len(data); got >= 16*1024 { + t.Errorf("fixture data size = %d, want less than 16 KiB", got) + } + + pos := top + if got, want := data[pos:pos+3], []byte{0xFE, 0x00, 0xE3}; !bytes.Equal(got, want) { + t.Fatalf("map header = %#v, want %#v", got, want) + } + pos += 3 + for i := range decodePathDecoyCount { + if target := smallDataPointer(t, data, pos); target != 0 { + t.Fatalf("decoy key %d points to %d, want 0", i, target) + } + pos += 2 + if got, want := data[pos:pos+2], []byte{0x00, 0x07}; !bytes.Equal(got, want) { + t.Fatalf("decoy value %d = %#v, want %#v", i, got, want) + } + pos += 2 + } + + wantTargetKey := []byte{0x46, 't', 'a', 'r', 'g', 'e', 't'} + if got := data[pos : pos+7]; !bytes.Equal(got, wantTargetKey) { + t.Fatalf("target key = %#v, want %#v", got, wantTargetKey) + } + pos += 7 + if got, want := data[pos:pos+3], []byte{0x5E, 0x0E, 0xDE}; !bytes.Equal(got, want) { + t.Fatalf("selected value header = %#v, want %#v", got, want) + } + + const targetKeySize = len("target") + navigated := decodePathDecoyCount*decodePathSharedKeySize + targetKeySize + if got := navigated + decodePathValueSize; got != recommendedPayloadLimit+1 { + t.Errorf("navigated and selected payload = %d, want %d", got, recommendedPayloadLimit+1) + } +} + +func TestDecodePathSharedBudgetFixtureIsSemanticallyValid(t *testing.T) { + path := filepath.Clean(filepath.Join("..", "..", "test-data", decodePathBudgetFixtureFilename)) + db, err := maxminddb.Open(path) + if err != nil { + t.Fatalf("opening decode path budget fixture: %v", err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Errorf("closing decode path budget fixture: %v", err) + } + }) + + result := db.Lookup(netip.MustParseAddr("1.1.1.1")) + if err := result.Err(); err != nil { + t.Fatalf("lookup: %v", err) + } + var value string + if err := result.DecodePath(&value, "target"); err != nil { + t.Fatalf("decoding target path: %v", err) + } + if got := len(value); got != decodePathValueSize { + t.Errorf("selected value length = %d, want %d", got, decodePathValueSize) + } +} + func TestMetadataLimitFixtureIsSemanticallyValid(t *testing.T) { path := filepath.Clean(filepath.Join("..", "..", "test-data", metadataLimitFixtureFilename)) db, err := maxminddb.Open(path) diff --git a/test-data/MaxMind-DB-test-decode-path-shared-budget.mmdb b/test-data/MaxMind-DB-test-decode-path-shared-budget.mmdb new file mode 100644 index 0000000000000000000000000000000000000000..6e85ac07917e0142059b40455ea64bc81555fd63 GIT binary patch literal 10457 zcmeIwF=_%q6b9g@2&R%07AZYJE?_HxxNK(^aCkfO+)U!mEHksB9>vl&*GLt}6@p-u zDmMt(F5(gN|9MkAcmp3l-43apv&;v_R|EnOfB*y_009U<00Izz00bZafr}UTrQ?XG zj|T&E03ARF&;fMddOGlsIFyp+gRd0=0SG_<0uX=z1Rwwb2tWV=5P-lX3%q^)fa zsI;kNGLx7xBT)xir}pe&YU}?mdXGzXWy7UJ>P?+DqO@;ZdT)<)vLMnt D7j~=3 literal 0 HcmV?d00001 From 64663303a66344fb704a6b2c5470cd90cfd3c845 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 31 Aug 2026 19:01:43 +0000 Subject: [PATCH 06/21] Share one fixture map between the writer and the test The writer and the byte-equality test each held the same 12-entry map literal. Adding a thirteenth fixture to the writer and forgetting the test would leave it with no regeneration coverage while the test still passed. Build the map once and have both read it. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/writer/pointerdos.go | 32 ++++++++++++++++++------------ pkg/writer/pointerdos_test.go | 37 +---------------------------------- 2 files changed, 20 insertions(+), 49 deletions(-) diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index 8d371248..4d9521d0 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -437,18 +437,11 @@ func buildPointerFanOutAllSpaceDB(depth int) []byte { // boundary from below. const pointerValueLimitDepth = 15 -// WritePointerDecoderDoSTestDB writes the databases that exercise the -// data-section pointer denial of service. Two exercise the fan-out: a minimal -// single-node database and a conventional IPv6 database that maps all of the -// address space to the fan-out record. Three exercise payload amplification: a -// moderate and a worst-case fixture with a shared bytes value, and one with a -// shared string value, the type most bindings copy into a native string. It -// also writes exact boundary fixtures for both recommended limits and a -// metadata fixture that exceeds the payload limit while opening. A small path -// fixture verifies that navigation and selected-value decoding share the same -// budget. See buildPointerFanOutData and buildPayloadAmplificationData. -func (w *Writer) WritePointerDecoderDoSTestDB() error { - files := map[string][]byte{ +// pointerDoSFixtures returns every fixture this file generates, keyed by +// filename. The writer and the byte-equality test share it so a new fixture +// cannot be added to one without the other. +func pointerDoSFixtures() map[string][]byte { + return map[string][]byte{ pointerDoSFixtureFilename: buildPointerFanOutDB(pointerDoSDepth), pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( pointerDoSDepth, @@ -483,7 +476,20 @@ func (w *Writer) WritePointerDecoderDoSTestDB() error { metadataLimitFixtureFilename: buildMetadataLimitDB(), decodePathBudgetFixtureFilename: buildDecodePathSharedBudgetDB(), } - for name, db := range files { +} + +// WritePointerDecoderDoSTestDB writes the databases that exercise the +// data-section pointer denial of service. Two exercise the fan-out: a minimal +// single-node database and a conventional IPv6 database that maps all of the +// address space to the fan-out record. Three exercise payload amplification: a +// moderate and a worst-case fixture with a shared bytes value, and one with a +// shared string value, the type most bindings copy into a native string. It +// also writes exact boundary fixtures for both recommended limits and a +// metadata fixture that exceeds the payload limit while opening. A small path +// fixture verifies that navigation and selected-value decoding share the same +// budget. See buildPointerFanOutData and buildPayloadAmplificationData. +func (w *Writer) WritePointerDecoderDoSTestDB() error { + for name, db := range pointerDoSFixtures() { path := filepath.Clean(filepath.Join(w.target, name)) if err := os.WriteFile(path, db, 0o644); err != nil { return fmt.Errorf("writing pointer DoS database %s: %w", name, err) diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index 8d67f910..293e7922 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -39,42 +39,7 @@ func TestPointerFanOutDataIsExponential(t *testing.T) { } func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { - cases := map[string][]byte{ - pointerDoSFixtureFilename: buildPointerFanOutDB(pointerDoSDepth), - pointerDoSIPv6FixtureFilename: buildPointerFanOutAllSpaceDB( - pointerDoSDepth, - ), - pointerValueLimitFixtureFilename: buildPointerFanOutDB( - pointerValueLimitDepth, - ), - payloadDoSFixtureFilename: buildPayloadAmplificationDB( - payloadPointerCount, - scalarTypeBytes, - ), - worstCasePayloadFixtureFilename: buildPayloadAmplificationDB( - worstCasePointerCount, - scalarTypeBytes, - ), - stringPayloadFixtureFilename: buildPayloadAmplificationDB( - payloadPointerCount, - scalarTypeString, - ), - valueLimitFixtureFilename: buildValueLimitDB( - recommendedValueLimit - 1, - ), - valueLimitOverFixtureFilename: buildValueLimitDB( - recommendedValueLimit, - ), - payloadLimitFixtureFilename: buildPayloadLimitDB( - recommendedPayloadLimit - 32*payloadScalarSize, - ), - payloadLimitOverFixtureFilename: buildPayloadLimitDB( - recommendedPayloadLimit - 32*payloadScalarSize + 1, - ), - metadataLimitFixtureFilename: buildMetadataLimitDB(), - decodePathBudgetFixtureFilename: buildDecodePathSharedBudgetDB(), - } - for name, got := range cases { + for name, got := range pointerDoSFixtures() { //nolint:gosec // name comes from the fixed cases map, not user input. want, err := os.ReadFile(filepath.Join("..", "..", "test-data", name)) if err != nil { From e7ab6b94fa113fd9903270fb5cbd193ad640d307 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 31 Aug 2026 20:24:03 +0000 Subject: [PATCH 07/21] Let the fixture tests accept a reader that enforces limits Two tests asserted that the pinned reader decodes fixtures this branch's own spec text says to reject. The decode path fixture costs one byte above the recommended budget, and the metadata fixture materializes 2,228,190 bytes while opening. libmaxminddb already refuses both. When maxminddb-golang adopts the limits this work motivates, a dependency bump turns these tests red and the failure reads like a reader regression. Both outcomes are compliant, because the spec makes the payload limit a recommendation rather than a requirement. Assert the disjunction instead: the reader returns the complete value, or it refuses. A partial value, a wrong length, or a panic still fails. The fixture bytes keep their strict checks. TestDecodePathSharedBudgetData reads them directly and TestPointerFanOutFixturesMatchCommitted compares them to the generator, and neither depends on reader behavior. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/writer/pointerdos_test.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index 293e7922..9b044abd 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -146,20 +146,32 @@ func TestDecodePathSharedBudgetFixtureIsSemanticallyValid(t *testing.T) { if err := result.Err(); err != nil { t.Fatalf("lookup: %v", err) } + // The fixture sits one byte above the payload budget the spec calls a + // recommendation, so both outcomes are compliant: the reader returns the + // whole value, or it refuses on its own limit. Assert that disjunction. A + // partial value, a wrong length, or a panic still fails. The bytes are + // checked by TestDecodePathSharedBudgetData and + // TestPointerFanOutFixturesMatchCommitted, which do not depend on the reader. var value string - if err := result.DecodePath(&value, "target"); err != nil { - t.Fatalf("decoding target path: %v", err) - } - if got := len(value); got != decodePathValueSize { - t.Errorf("selected value length = %d, want %d", got, decodePathValueSize) + err = result.DecodePath(&value, "target") + if err != nil { + t.Logf("reader refused the path decode, which its limits allow: %v", err) + } else if len(value) != decodePathValueSize { + t.Errorf("selected value length = %d, want %d", len(value), decodePathValueSize) } } func TestMetadataLimitFixtureIsSemanticallyValid(t *testing.T) { path := filepath.Clean(filepath.Join("..", "..", "test-data", metadataLimitFixtureFilename)) + // The metadata materializes 2,228,190 bytes while opening, above the payload + // budget the spec calls a recommendation. A reader that bounds metadata + // rejects the file here, which is the behavior this fixture exists to + // provoke, so treat that as a pass and check the sizes only when the reader + // accepts it. db, err := maxminddb.Open(path) if err != nil { - t.Fatalf("opening metadata limit fixture: %v", err) + t.Logf("reader refused to open the fixture, which its limits allow: %v", err) + return } t.Cleanup(func() { if err := db.Close(); err != nil { From 254545a14647a07ba7b790559351bf179779f51c Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 31 Aug 2026 20:10:07 +0000 Subject: [PATCH 08/21] Document the denial of service test data The DoS and boundary fixtures had no entry in the fixture README. Describe what each one attacks and what a reader at the recommended limits should do with it. State that the boundary values are recommendations. The spec recommends 65,536 decoded values but does not require a single payload limit, so the 2 MiB the payload files use is the value libmaxminddb and the MaxMind readers picked, not a portable requirement. Record that the payload pair assumes per-occurrence accounting and cannot separate accept from reject for a reader that memoizes pointer targets, which the spec lists as a valid defense. Warn against decoding these files without a memory limit. The worst case expands to about 4 GiB from 200 KB, which hangs a harness that walks the directory and decodes everything. Co-Authored-By: Claude Opus 5 (1M context) --- test-data/README.md | 56 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test-data/README.md b/test-data/README.md index 4465d5b4..c379b07c 100644 --- a/test-data/README.md +++ b/test-data/README.md @@ -32,6 +32,62 @@ broken, and exploited functionality simply not available in the go mmdbwriter: - GeoIP2-City-Test-Invalid-Node-Count.mmdb - maps-with-pointers.raw +## Denial of service test data + +Some files in this directory are hostile by design. Each one is a valid MaxMind +DB file. A reader without resource limits uses very large amounts of CPU or +memory to decode them. Use them to test the limits in the "Reader Resource +Limits" section of the spec. + +Do not decode these files without a memory limit on the process. The worst case +file is about 200 KB and expands to about 4 GiB. A test harness that walks this +whole directory and decodes every record will hang or run out of memory. + +| File | What a reader must survive | +| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | +| MaxMind-DB-test-pointer-decoder-dos.mmdb | A depth 40 pointer fan-out. An unprotected decoder performs 2\*\*40 leaf decodes from 451 bytes. | +| MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb | The same fan-out in a conventional IPv6 database that maps the whole address space to the record. | +| MaxMind-DB-test-payload-amplification-dos.mmdb | 8,192 pointers to one 65,535 byte bytes value. A reader that copies each target materializes about 512 MiB. | +| MaxMind-DB-test-payload-amplification-dos-string.mmdb | The same shape with a UTF-8 string value, the type most bindings copy into a native string. | +| MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb | 65,535 pointers to that value. The record decodes to 65,536 values, which meets the recommended value limit, and materializes about 4 GiB. | + +These files are the boundary cases. A reader at the recommended limits accepts +the first file in each pair and rejects the second. + +| File | Expected result | +| ------------------------------------------------------ | ------------------------------------------------------------------------------- | +| MaxMind-DB-test-decoder-value-limit.mmdb | Accept. 65,536 decoded values, exactly the limit. | +| MaxMind-DB-test-decoder-value-limit-over.mmdb | Reject. 65,537 decoded values. | +| MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb | Accept. 65,535 values reached through a depth 15 fan-out. | +| MaxMind-DB-test-decoder-payload-limit.mmdb | Accept. 2,097,152 payload bytes, exactly 2 MiB. | +| MaxMind-DB-test-decoder-payload-limit-over.mmdb | Reject. 2,097,153 payload bytes. | +| MaxMind-DB-test-metadata-payload-limit.mmdb | Reject at open. The metadata alone materializes 2,228,190 bytes. | +| MaxMind-DB-test-decode-path-shared-budget.mmdb | Reject a path lookup. Navigation plus the selected value costs 2,097,153 bytes. | + +### The boundary values are recommendations + +The spec recommends a limit of 65,536 decoded values. It does not require a +single payload limit, because the right method and limit depend on the reader's +language and API. + +The payload files use 2 MiB (2\*\*21 bytes) because libmaxminddb and the MaxMind +readers use that value. A reader that picks a different payload limit is still +compliant. For that reader, treat the two `-decoder-payload-limit` files as an +example of the attack shape, not as a pass or fail requirement. Adjust the +expected boundary to the limit the reader actually uses. + +### The payload files assume per-occurrence accounting + +The payload boundary files point 32 of their 33 elements at one shared value. +They separate accept from reject only for a reader that charges every decoded +occurrence. + +A reader that memoizes pointer targets materializes the shared value one time, +about 64 KB. That reader accepts both files. This is correct behavior for it. +The spec lists safe memoization as one way to bound the amplification. Test such +a reader with the amplification files above, which no accounting model can +expand cheaply. + ## Usage ``` From 0cf3b51de9bd95ed4ea1af95077a53bb42858418 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 31 Aug 2026 20:11:40 +0000 Subject: [PATCH 09/21] Encode sizes above 28 correctly in the raw writers writeMap and writeString both masked the size with 0x1f and returned a one-byte header, so any size above 28 silently produced a wrong control byte and no extended size bytes. Every current caller stays under that: the largest map has 9 entries and the longest string is binary_format_major_version at 27 bytes. So nothing was broken, but the next caller needing a larger value would have written a corrupt fixture with no error. writeMap now writes the extended size forms and panics above 65,820 rather than truncate. pointerdos.go carried the same encoder as writeMapHeader, so drop that copy and keep one map encoder in rawmmdb.go, which the file header names as the home for the raw encoding helpers. writeString delegates its control byte to writeScalar, which already encodes every size form this writer supports. Fixture bytes are unchanged. Add tests for every size form, since the fixtures reach only the one-byte and three-byte map forms and no caller reaches the extended string forms at all. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/writer/pointerdos.go | 22 +----------- pkg/writer/rawmmdb.go | 37 ++++++++++++++++----- pkg/writer/rawmmdb_test.go | 68 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 29 deletions(-) create mode 100644 pkg/writer/rawmmdb_test.go diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index 4d9521d0..0e06d852 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -167,26 +167,6 @@ func writeArrayHeader(buf []byte, size int) int { } } -func writeMapHeader(buf []byte, size int) int { - switch { - case size <= 28: - buf[0] = (7 << 5) | byte(size&0x1F) - return 1 - case size <= 284: - buf[0] = (7 << 5) | 29 - buf[1] = byte(size - 29) - return 2 - case size <= 65820: - buf[0] = (7 << 5) | 30 - encoded := size - 285 - buf[1] = byte((encoded >> 8) & 0xFF) - buf[2] = byte(encoded & 0xFF) - return 3 - default: - panic(fmt.Sprintf("map size %d is unsupported by this fixture writer", size)) - } -} - // buildPayloadAmplificationData builds a data section holding one large scalar // value (string or bytes, per scalarType) at offset 0 followed by an array of // pointers that all target it. The value count and depth limits do not bound @@ -260,7 +240,7 @@ func buildDecodePathSharedBudgetData() (data []byte, top int) { } top = pos - pos += writeMapHeader(data[pos:], decodePathDecoyCount+1) + pos += writeMap(data[pos:], decodePathDecoyCount+1) for range decodePathDecoyCount { pos += copy(data[pos:], writeDataPointer(0)) data[pos] = 0 diff --git a/pkg/writer/rawmmdb.go b/pkg/writer/rawmmdb.go index 8392e6cb..84937ccc 100644 --- a/pkg/writer/rawmmdb.go +++ b/pkg/writer/rawmmdb.go @@ -6,7 +6,10 @@ package writer // large-size (case-31) encoding and complete database builders for crafting // intentionally malformed MMDB files that cannot be created through mmdbwriter. -import "encoding/binary" +import ( + "encoding/binary" + "fmt" +) const ( metadataMarker = "\xab\xcd\xefMaxMind.com" @@ -49,17 +52,35 @@ var ( } ) -// writeMap writes a map control byte (type 7) for sizes <= 28. +// writeMap writes a map control byte (type 7). Sizes above 28 use the extended +// size forms, which take one or two more bytes. func writeMap(buf []byte, size int) int { - buf[0] = (7 << 5) | byte(size&0x1f) - return 1 + switch { + case size <= 28: + buf[0] = (7 << 5) | byte(size&0x1F) + return 1 + case size <= 284: + buf[0] = (7 << 5) | 29 + buf[1] = byte(size - 29) + return 2 + case size <= 65820: + buf[0] = (7 << 5) | 30 + encoded := size - 285 + buf[1] = byte((encoded >> 8) & 0xFF) + buf[2] = byte(encoded & 0xFF) + return 3 + default: + panic(fmt.Sprintf("map size %d is unsupported by this fixture writer", size)) + } } -// writeString writes a string value (type 2). +// writeString writes a string value (type 2). It delegates the control byte to +// writeScalar so strings longer than 28 bytes get the extended size forms +// instead of a truncated size. func writeString(buf []byte, s string) int { - buf[0] = (2 << 5) | byte(len(s)&0x1f) - copy(buf[1:], s) - return 1 + len(s) + pos := writeScalar(buf, len(s), scalarTypeString) + copy(buf[pos-len(s):pos], s) + return pos } // writeUint16 writes a uint16 value (type 5, 2 bytes). diff --git a/pkg/writer/rawmmdb_test.go b/pkg/writer/rawmmdb_test.go new file mode 100644 index 00000000..c7bc1bf4 --- /dev/null +++ b/pkg/writer/rawmmdb_test.go @@ -0,0 +1,68 @@ +package writer + +import ( + "bytes" + "strings" + "testing" +) + +// TestWriteMapControlByte covers all three size forms. The fixtures only +// exercise the one-byte and three-byte forms, so a wrong byte in the two-byte +// form would otherwise reach a generated file unnoticed. +func TestWriteMapControlByte(t *testing.T) { + tests := []struct { + size int + want []byte + }{ + {0, []byte{0xE0}}, + {28, []byte{0xFC}}, + {29, []byte{0xFD, 0x00}}, + {284, []byte{0xFD, 0xFF}}, + {285, []byte{0xFE, 0x00, 0x00}}, + {512, []byte{0xFE, 0x00, 0xE3}}, + {65820, []byte{0xFE, 0xFF, 0xFF}}, + } + for _, test := range tests { + buf := make([]byte, 8) + n := writeMap(buf, test.size) + if got := buf[:n]; !bytes.Equal(got, test.want) { + t.Errorf("writeMap(%d) = %#v, want %#v", test.size, got, test.want) + } + } +} + +// TestWriteStringControlByte covers the boundary where the size no longer fits +// the control byte. Every current caller passes a short metadata key, so nothing +// else exercises the extended forms. +func TestWriteStringControlByte(t *testing.T) { + tests := []struct { + size int + wantHeader []byte + }{ + {0, []byte{0x40}}, + {2, []byte{0x42}}, + {28, []byte{0x5C}}, + {29, []byte{0x5D, 0x00}}, + {284, []byte{0x5D, 0xFF}}, + {285, []byte{0x5E, 0x00, 0x00}}, + } + for _, test := range tests { + value := strings.Repeat("a", test.size) + buf := make([]byte, 1024) + n := writeString(buf, value) + want := append(append([]byte{}, test.wantHeader...), value...) + if got := buf[:n]; !bytes.Equal(got, want) { + t.Errorf("writeString(%d bytes) = %#v, want %#v", + test.size, got[:min(n, 4)], want[:min(len(want), 4)]) + } + } +} + +func TestWriteMapRejectsOversize(t *testing.T) { + defer func() { + if recover() == nil { + t.Error("writeMap(65821) did not panic") + } + }() + writeMap(make([]byte, 8), 65821) +} From 6c476b1e3e1d66fba6b3a7e63c31d0659b180139 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 31 Aug 2026 19:01:43 +0000 Subject: [PATCH 10/21] Ignore the root-level write-test-data binary The existing entry is anchored to cmd/write-test-data, but building the tool from the repo root, which is what the README example encourages, drops a 4 MB binary at the root where nothing ignores it. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 9d60e124..04a2f291 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,5 @@ docs/.hugo_build.lock docs/public/ *.swp /cmd/write-test-data/write-test-data +/write-test-data .lycheecache From aa8a9cc41bf2add7e028fab097a5935575edd7ec Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:08:34 +0000 Subject: [PATCH 11/21] Harden raw MMDB fixture encoders --- pkg/writer/pointerdos.go | 33 ++++++++++++++--- pkg/writer/rawmmdb.go | 52 +++++++++++++++++++++----- pkg/writer/rawmmdb_test.go | 76 ++++++++++++++++++++++++++++++++++---- 3 files changed, 139 insertions(+), 22 deletions(-) diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index 0e06d852..79151fc4 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -57,8 +57,7 @@ func dataRecordValue24(nodeCount uint32, dataOffset int) uint32 { panic(fmt.Sprintf("negative data-section offset: %d", dataOffset)) } recordValue := uint64(nodeCount) + dataSeparatorSize + uint64(dataOffset) - const max24BitRecordValue = 1<<24 - 1 - if recordValue > max24BitRecordValue { + if recordValue > maximum24BitSearchTreeValue { panic(fmt.Sprintf( "data record value %d does not fit in a 24-bit search-tree record", recordValue, @@ -122,6 +121,17 @@ const ( ) func writeScalar(buf []byte, size int, scalarType byte) int { + if size < 0 || size > maximumDataStructureSize { + panic(fmt.Sprintf( + "scalar size %d is outside the supported range 0..%d", + size, + maximumDataStructureSize, + )) + } + if scalarType != scalarTypeString && scalarType != scalarTypeBytes { + panic(fmt.Sprintf("unsupported scalar type %d", scalarType)) + } + pos := 1 switch { case size <= 28: @@ -130,20 +140,32 @@ func writeScalar(buf []byte, size int, scalarType byte) int { buf[0] = (scalarType << 5) | 29 buf[1] = byte(size - 29) pos++ - case size <= 65820: + case size <= maximumSizeCode30: buf[0] = (scalarType << 5) | 30 encoded := size - 285 buf[1] = byte((encoded >> 8) & 0xFF) buf[2] = byte(encoded & 0xFF) pos += 2 default: - panic(fmt.Sprintf("scalar size %d is unsupported by this fixture writer", size)) + buf[0] = (scalarType << 5) | 31 + encoded := size - minimumSizeCode31 + buf[1] = byte((encoded >> 16) & 0xFF) + buf[2] = byte((encoded >> 8) & 0xFF) + buf[3] = byte(encoded & 0xFF) + pos += 3 } clear(buf[pos : pos+size]) return pos + size } func writeArrayHeader(buf []byte, size int) int { + if size < 0 || size > maximumDataStructureSize { + panic(fmt.Sprintf( + "array size %d is outside the supported range 0..%d", + size, + maximumDataStructureSize, + )) + } switch { case size <= 28: buf[0] = byte(size & 0x1F) @@ -154,7 +176,7 @@ func writeArrayHeader(buf []byte, size int) int { buf[1] = 4 buf[2] = byte(size - 29) return 3 - case size <= 65820: + case size <= maximumSizeCode30: buf[0] = 30 buf[1] = 4 encoded := size - 285 @@ -162,7 +184,6 @@ func writeArrayHeader(buf []byte, size int) int { buf[3] = byte(encoded & 0xFF) return 4 default: - //nolint:gosec // size is a bounded fixture element count. return writeLargeArray(buf, uint32(size)) } } diff --git a/pkg/writer/rawmmdb.go b/pkg/writer/rawmmdb.go index 84937ccc..2fd01326 100644 --- a/pkg/writer/rawmmdb.go +++ b/pkg/writer/rawmmdb.go @@ -12,8 +12,12 @@ import ( ) const ( - metadataMarker = "\xab\xcd\xefMaxMind.com" - dataSeparatorSize = 16 + metadataMarker = "\xab\xcd\xefMaxMind.com" + dataSeparatorSize = 16 + maximumSizeCode30 = 65820 + minimumSizeCode31 = maximumSizeCode30 + 1 + maximumDataStructureSize = minimumSizeCode31 + (1 << 24) - 1 + maximum24BitSearchTreeValue = 1<<24 - 1 ) var ( @@ -53,8 +57,15 @@ var ( ) // writeMap writes a map control byte (type 7). Sizes above 28 use the extended -// size forms, which take one or two more bytes. +// size forms, which take one to three more bytes. func writeMap(buf []byte, size int) int { + if size < 0 || size > maximumDataStructureSize { + panic(fmt.Sprintf( + "map size %d is outside the supported range 0..%d", + size, + maximumDataStructureSize, + )) + } switch { case size <= 28: buf[0] = (7 << 5) | byte(size&0x1F) @@ -63,14 +74,14 @@ func writeMap(buf []byte, size int) int { buf[0] = (7 << 5) | 29 buf[1] = byte(size - 29) return 2 - case size <= 65820: + case size <= maximumSizeCode30: buf[0] = (7 << 5) | 30 encoded := size - 285 buf[1] = byte((encoded >> 8) & 0xFF) buf[2] = byte(encoded & 0xFF) return 3 default: - panic(fmt.Sprintf("map size %d is unsupported by this fixture writer", size)) + return writeLargeMap(buf, uint32(size)) } } @@ -113,7 +124,15 @@ func writeMetaKey(buf []byte, key string) int { // writeLargeArray writes an array control byte (extended type 11) with // case-31 size encoding for sizes > 65820. func writeLargeArray(buf []byte, size uint32) int { - adjusted := size - 65821 + if size < minimumSizeCode31 || size > maximumDataStructureSize { + panic(fmt.Sprintf( + "array size %d is outside the case-31 range %d..%d", + size, + minimumSizeCode31, + maximumDataStructureSize, + )) + } + adjusted := size - minimumSizeCode31 buf[0] = (0 << 5) | 31 // extended type, size = case 31 buf[1] = 4 // extended type: 7 + 4 = 11 (array) buf[2] = byte((adjusted >> 16) & 0xFF) @@ -125,7 +144,15 @@ func writeLargeArray(buf []byte, size uint32) int { // writeLargeMap writes a map control byte (type 7) with case-31 size // encoding for sizes > 65820. func writeLargeMap(buf []byte, size uint32) int { - adjusted := size - 65821 + if size < minimumSizeCode31 || size > maximumDataStructureSize { + panic(fmt.Sprintf( + "map size %d is outside the case-31 range %d..%d", + size, + minimumSizeCode31, + maximumDataStructureSize, + )) + } + adjusted := size - minimumSizeCode31 buf[0] = (7 << 5) | 31 // type 7 (map), size = case 31 buf[1] = byte((adjusted >> 16) & 0xFF) buf[2] = byte((adjusted >> 8) & 0xFF) @@ -146,9 +173,16 @@ func writeSearchTree(buf []byte, recordValue uint32) int { return writeSearchTreeRecords(buf, recordValue, recordValue) } -// writeSearchTreeRecords writes a 1-node search tree with 24-bit records -// where the left and right records can hold different values. +// writeSearchTreeRecords writes one search-tree node with 24-bit records where +// the left and right records can hold different values. func writeSearchTreeRecords(buf []byte, leftRecord, rightRecord uint32) int { + if leftRecord > maximum24BitSearchTreeValue || rightRecord > maximum24BitSearchTreeValue { + panic(fmt.Sprintf( + "search-tree record values %d and %d must fit in 24 bits", + leftRecord, + rightRecord, + )) + } buf[0] = byte((leftRecord >> 16) & 0xFF) buf[1] = byte((leftRecord >> 8) & 0xFF) buf[2] = byte(leftRecord & 0xFF) diff --git a/pkg/writer/rawmmdb_test.go b/pkg/writer/rawmmdb_test.go index c7bc1bf4..6ef7fc71 100644 --- a/pkg/writer/rawmmdb_test.go +++ b/pkg/writer/rawmmdb_test.go @@ -21,6 +21,8 @@ func TestWriteMapControlByte(t *testing.T) { {285, []byte{0xFE, 0x00, 0x00}}, {512, []byte{0xFE, 0x00, 0xE3}}, {65820, []byte{0xFE, 0xFF, 0xFF}}, + {65821, []byte{0xFF, 0x00, 0x00, 0x00}}, + {maximumDataStructureSize, []byte{0xFF, 0xFF, 0xFF, 0xFF}}, } for _, test := range tests { buf := make([]byte, 8) @@ -45,10 +47,12 @@ func TestWriteStringControlByte(t *testing.T) { {29, []byte{0x5D, 0x00}}, {284, []byte{0x5D, 0xFF}}, {285, []byte{0x5E, 0x00, 0x00}}, + {65820, []byte{0x5E, 0xFF, 0xFF}}, + {65821, []byte{0x5F, 0x00, 0x00, 0x00}}, } for _, test := range tests { value := strings.Repeat("a", test.size) - buf := make([]byte, 1024) + buf := make([]byte, test.size+8) n := writeString(buf, value) want := append(append([]byte{}, test.wantHeader...), value...) if got := buf[:n]; !bytes.Equal(got, want) { @@ -58,11 +62,69 @@ func TestWriteStringControlByte(t *testing.T) { } } -func TestWriteMapRejectsOversize(t *testing.T) { - defer func() { - if recover() == nil { - t.Error("writeMap(65821) did not panic") +func TestWriteArrayHeader(t *testing.T) { + tests := []struct { + size int + want []byte + }{ + {0, []byte{0x00, 0x04}}, + {28, []byte{0x1C, 0x04}}, + {29, []byte{0x1D, 0x04, 0x00}}, + {284, []byte{0x1D, 0x04, 0xFF}}, + {285, []byte{0x1E, 0x04, 0x00, 0x00}}, + {65820, []byte{0x1E, 0x04, 0xFF, 0xFF}}, + {65821, []byte{0x1F, 0x04, 0x00, 0x00, 0x00}}, + {maximumDataStructureSize, []byte{0x1F, 0x04, 0xFF, 0xFF, 0xFF}}, + } + for _, test := range tests { + buf := make([]byte, 8) + n := writeArrayHeader(buf, test.size) + if got := buf[:n]; !bytes.Equal(got, test.want) { + t.Errorf("writeArrayHeader(%d) = %#v, want %#v", test.size, got, test.want) } - }() - writeMap(make([]byte, 8), 65821) + } +} + +func TestWriteSearchTreeRecordsMaximum(t *testing.T) { + buf := make([]byte, 6) + n := writeSearchTreeRecords(buf, maximum24BitSearchTreeValue, maximum24BitSearchTreeValue) + want := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF} + if got := buf[:n]; !bytes.Equal(got, want) { + t.Errorf("writeSearchTreeRecords() = %#v, want %#v", got, want) + } +} + +func TestRawWritersRejectInvalidInput(t *testing.T) { + tests := []struct { + name string + call func() + }{ + {"negative map size", func() { writeMap(make([]byte, 8), -1) }}, + {"oversized map", func() { writeMap(make([]byte, 8), maximumDataStructureSize+1) }}, + {"undersized large map", func() { writeLargeMap(make([]byte, 8), maximumSizeCode30) }}, + {"negative array size", func() { writeArrayHeader(make([]byte, 8), -1) }}, + { + "oversized array", + func() { writeArrayHeader(make([]byte, 8), maximumDataStructureSize+1) }, + }, + {"undersized large array", func() { writeLargeArray(make([]byte, 8), maximumSizeCode30) }}, + {"negative scalar size", func() { writeScalar(make([]byte, 8), -1, scalarTypeBytes) }}, + {"oversized scalar", func() { + writeScalar(make([]byte, 8), maximumDataStructureSize+1, scalarTypeBytes) + }}, + {"invalid scalar type", func() { writeScalar(make([]byte, 8), 0, 8) }}, + {"oversized search-tree record", func() { + writeSearchTreeRecords(make([]byte, 8), maximum24BitSearchTreeValue+1, 0) + }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + defer func() { + if recover() == nil { + t.Error("call did not panic") + } + }() + test.call() + }) + } } From 7de1b2ae94ade9082e4ddaa58aa09fa68ad9e65e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:12:06 +0000 Subject: [PATCH 12/21] Strengthen resource limit fixture validation --- pkg/writer/pointerdos_test.go | 137 ++++++++++++++++++++++++++++------ 1 file changed, 115 insertions(+), 22 deletions(-) diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index 9b044abd..ea79e02b 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -6,6 +6,7 @@ import ( "net/netip" "os" "path/filepath" + "strings" "testing" "github.com/oschwald/maxminddb-golang/v2" @@ -52,29 +53,52 @@ func TestPointerFanOutFixturesMatchCommitted(t *testing.T) { } func TestDecoderLimitBoundaryData(t *testing.T) { - for _, pointerCount := range []int{ - recommendedValueLimit - 1, - recommendedValueLimit, - } { - data, top := buildValueLimitData(pointerCount) + valueTests := []struct { + pointerCount int + wantCount int + wantValues int + }{ + {recommendedValueLimit - 1, 65_535, 65_536}, + {recommendedValueLimit, 65_536, 65_537}, + } + for _, test := range valueTests { + data, top := buildValueLimitData(test.pointerCount) if top != 1 { t.Errorf("value boundary top offset = %d, want 1", top) } if data[0] != 0xA0 { t.Errorf("value boundary scalar = %#x, want 0xa0", data[0]) } - if len(data) != 1+4+pointerCount*2 { - t.Errorf("value boundary length = %d, want %d", len(data), 1+4+pointerCount*2) + if len(data) != 1+4+test.pointerCount*2 { + t.Errorf( + "value boundary length = %d, want %d", + len(data), + 1+4+test.pointerCount*2, + ) } - for offset := top + 4; offset < len(data); offset += 2 { + count, headerSize := arrayElementCount(t, data, top) + if count != test.wantCount { + t.Errorf("array element count = %d, want %d", count, test.wantCount) + } + if values := count + 1; values != test.wantValues { + t.Errorf("decoded value count = %d, want %d", values, test.wantValues) + } + for offset := top + headerSize; offset < len(data); offset += 2 { if target := smallDataPointer(t, data, offset); target != 0 { t.Fatalf("value boundary pointer at %d targets %d, want 0", offset, target) } } } - for _, smallSize := range []int{32, 33} { - data, top := buildPayloadLimitData(smallSize) + payloadTests := []struct { + smallSize int + wantPayload int + }{ + {32, 2_097_152}, + {33, 2_097_153}, + } + for _, test := range payloadTests { + data, top := buildPayloadLimitData(test.smallSize) if top <= payloadScalarSize { t.Errorf("payload boundary top offset = %d, want after large scalar", top) } @@ -82,13 +106,49 @@ func TestDecoderLimitBoundaryData(t *testing.T) { t.Fatalf("payload boundary data ends before outer array at %d", top) } gotPayload := sumPointedPayload(t, data, top) - wantPayload := recommendedPayloadLimit + smallSize - 32 - if gotPayload != wantPayload { - t.Errorf("payload boundary total = %d, want %d", gotPayload, wantPayload) + if gotPayload != test.wantPayload { + t.Errorf("payload boundary total = %d, want %d", gotPayload, test.wantPayload) } } } +func TestDecoderLimitBoundaryFixturesOpen(t *testing.T) { + tests := []struct { + name string + wantOffset uintptr + }{ + {valueLimitFixtureFilename, 1}, + {valueLimitOverFixtureFilename, 1}, + {payloadLimitFixtureFilename, 65_572}, + {payloadLimitOverFixtureFilename, 65_573}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + path := filepath.Clean(filepath.Join("..", "..", "test-data", test.name)) + db, err := maxminddb.Open(path) + if err != nil { + t.Fatalf("opening fixture: %v", err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Errorf("closing fixture: %v", err) + } + }) + + result := db.Lookup(netip.MustParseAddr("1.1.1.1")) + if err := result.Err(); err != nil { + t.Fatalf("lookup: %v", err) + } + if !result.Found() { + t.Fatal("lookup did not find a record") + } + if got := result.Offset(); got != test.wantOffset { + t.Errorf("record offset = %d, want %d", got, test.wantOffset) + } + }) + } +} + func TestDecodePathSharedBudgetData(t *testing.T) { data, top := buildDecodePathSharedBudgetData() if top != 3+decodePathSharedKeySize { @@ -155,6 +215,9 @@ func TestDecodePathSharedBudgetFixtureIsSemanticallyValid(t *testing.T) { var value string err = result.DecodePath(&value, "target") if err != nil { + if !isReaderResourceLimitError(err) { + t.Fatalf("decoding path: %v", err) + } t.Logf("reader refused the path decode, which its limits allow: %v", err) } else if len(value) != decodePathValueSize { t.Errorf("selected value length = %d, want %d", len(value), decodePathValueSize) @@ -170,6 +233,9 @@ func TestMetadataLimitFixtureIsSemanticallyValid(t *testing.T) { // accepts it. db, err := maxminddb.Open(path) if err != nil { + if !isReaderResourceLimitError(err) { + t.Fatalf("opening metadata limit fixture: %v", err) + } t.Logf("reader refused to open the fixture, which its limits allow: %v", err) return } @@ -192,6 +258,10 @@ func TestMetadataLimitFixtureIsSemanticallyValid(t *testing.T) { } } +func isReaderResourceLimitError(err error) bool { + return strings.Contains(err.Error(), "maximum decoded record size") +} + func TestPointerFanOutFixturesAreSemanticallyValid(t *testing.T) { tests := []struct { name string @@ -271,12 +341,10 @@ func TestPointerFanOutFixturesAreSemanticallyValid(t *testing.T) { for i, address := range test.addresses { result := db.Lookup(netip.MustParseAddr(address)) if err := result.Err(); err != nil { - t.Errorf("looking up %s: %v", address, err) - continue + t.Fatalf("looking up %s: %v", address, err) } if !result.Found() { - t.Errorf("lookup for %s did not find a record", address) - continue + t.Fatalf("lookup for %s did not find a record", address) } if i == 0 { outerOffset = result.Offset() @@ -370,12 +438,9 @@ func scalarPayloadSize(t *testing.T, data []byte, offset int) int { // a change to the encoding or the element count fails the assertion. func sumPointedPayload(t *testing.T, data []byte, top int) int { t.Helper() - if top+3 > len(data) || data[top] != 29 || data[top+1] != 4 { - t.Fatalf("value at offset %d is not a size-29 array header", top) - } - count := int(data[top+2]) + 29 + count, headerSize := arrayElementCount(t, data, top) total := 0 - elem := top + 3 + elem := top + headerSize for range count { total += scalarPayloadSize(t, data, smallDataPointer(t, data, elem)) elem += 2 @@ -383,6 +448,34 @@ func sumPointedPayload(t *testing.T, data []byte, top int) int { return total } +func arrayElementCount(t *testing.T, data []byte, offset int) (count, headerSize int) { + t.Helper() + if offset < 0 || offset+2 > len(data) || data[offset]>>5 != 0 || data[offset+1] != 4 { + t.Fatalf("value at offset %d is not an array", offset) + } + sizeCode := int(data[offset] & 0x1F) + switch { + case sizeCode <= 28: + return sizeCode, 2 + case sizeCode == 29: + if offset+3 > len(data) { + t.Fatalf("size-29 array header at offset %d is truncated", offset) + } + return int(data[offset+2]) + 29, 3 + case sizeCode == 30: + if offset+4 > len(data) { + t.Fatalf("size-30 array header at offset %d is truncated", offset) + } + return (int(data[offset+2])<<8 | int(data[offset+3])) + 285, 4 + default: + if offset+5 > len(data) { + t.Fatalf("size-31 array header at offset %d is truncated", offset) + } + return (int(data[offset+2])<<16 | int(data[offset+3])<<8 | int(data[offset+4])) + + minimumSizeCode31, 5 + } +} + func smallDataPointer(t *testing.T, data []byte, offset int) int { t.Helper() if offset < 0 || offset+2 > len(data) { From c24bef743c16d7fc28feb0b0ef01eae3455d9e63 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:13:16 +0000 Subject: [PATCH 13/21] Define decoded value accounting guidance --- MaxMind-DB-spec.md | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/MaxMind-DB-spec.md b/MaxMind-DB-spec.md index 5a5d3f60..e1b6b836 100644 --- a/MaxMind-DB-spec.md +++ b/MaxMind-DB-spec.md @@ -584,18 +584,25 @@ and bound nesting and traversal work. ### Maximum decoded value count -The depth limit does not bound the total amount of work. A record can nest -arrays or maps whose elements are all pointers to the level below, so each level -is decoded twice as often as the one above. Such a structure stays shallow, but -decoding the top value produces 2\*\*depth leaf decodes and Θ(2\*\*depth) total -decode operations. A record under one kilobyte can therefore take longer to -decode than any real workload allows. - -A reader can bound this by counting the values it decodes for a single record -and stopping if the count exceeds a fixed limit. One rule is to charge each -logical value occurrence, including one resolved from a memoized target. Readers -with structural or weighted bounds may account differently. Under a logical -value rule, map keys and values are separate occurrences. +The depth limit does not bound the total amount of work. Arrays and maps can +contain many pointers to the same target. Unless the reader safely reuses the +target, it decodes that target once for every pointer occurrence. In the binary +fan-out example used by the test data, each array contains two pointers to the +level below, so each lower level doubles the work. Decoding the top value takes +exponentially more operations than the depth suggests. A record under one +kilobyte can therefore take longer to decode than any real workload allows. + +A reader can bound this by counting the values it decodes and stopping if the +count exceeds a fixed limit. One flat accounting rule is: + +- Charge every logical value occurrence once. The root is one occurrence. An + array or map is one occurrence in addition to its children. Map keys and map + values are separate occurrences. +- Do not charge a pointer separately from its resolved value. Charge the value + at the position where the pointer occurs. Under this rule, a value resolved + from a memoized target is still charged once for each logical occurrence. + +Readers with structural or weighted bounds may account differently. If a destination uses a container's declared length to allocate storage, counting only after that allocation may be too late. A reader can reserve the @@ -615,8 +622,9 @@ produces decode a few hundred values, so this leaves a wide margin. The value count limits how many fields a record decodes, not how many bytes they hold. A single string or bytes value can be up to 16,843,036 bytes. An array of -32,767 pointers to one such value stays under the value limit. It still expands -to more than 500 GiB, from a file barely larger than the value itself. +65,535 pointers to one such value stays at the recommended value limit under the +flat rule above. It can still describe more than 1 TiB of repeated data in a +file barely larger than the value itself. A reader that copies, validates, or allocates these values should bound the total string and bytes data it materializes for one lookup. The right method and @@ -643,11 +651,11 @@ A reader can bound this in several ways: including data stored inline in a container that a pointer targets. Stop when the total exceeds a limit. Re-decoding a pointer target charges its data again, which is what bounds the amplification. -- Fold the size into the value-count budget, charging each value by its size. - Charge containers by entries traversed, pointer expansion when the target is - not safely reused, map-key and selector work, and strings and bytes - materialized. A small fixed-width scalar can cost little, once the traversal - needed to reach it is bounded. +- Fold these concerns into a unified weighted budget. It can account for + container entries traversed, pointer expansion when the target is not safely + reused, map-key and selector work, and strings and bytes materialized. A small + fixed-width scalar can cost little or nothing once the work needed to reach it + is bounded. - Safely memoize decoded pointer targets, handling cycles and in-progress targets, so a shared target is materialized once. From 8262fb52328f7bae431867ce75ba8c4e35ff5f58 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:14:17 +0000 Subject: [PATCH 14/21] Clarify decode and materialization boundaries --- MaxMind-DB-spec.md | 77 +++++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/MaxMind-DB-spec.md b/MaxMind-DB-spec.md index e1b6b836..6e008890 100644 --- a/MaxMind-DB-spec.md +++ b/MaxMind-DB-spec.md @@ -548,14 +548,22 @@ than the record's size suggests. A value can be a pointer, and several pointers can share one target. A small data section can therefore describe a structure that is huge, or effectively infinite, when fully expanded. -A reader should ensure that no operation decoding attacker-controlled values -consumes disproportionate time or memory. This applies to a lookup, a path -selection, metadata decoding on open, and verification. +A reader should apply resource controls to operations that decode +attacker-controlled values. The controls should bound the reader's time and +memory before it allocates, traverses, or copies an attacker-chosen amount of +data. The examples in this section are implementation guidance. They do not +define whether an encoding conforms to the file format. + +A caller-requested decode operation can include a lookup, path selection and +decoding of the selected value, or metadata decoding while opening a database. A +reader that validates a whole database can apply a bound to each data entry if +the work outside those entries is also bounded. A reader can apply the depth, value-count, and payload limits below. It can -instead use another strategy with an equivalent bound, such as: +instead combine them or use another strategy that bounds the same risks, such +as: -- Safe memoization of pointer targets. +- Safe memoization of pointer targets, including cycle handling. - Schema-directed decoding into a destination that is finite and nonrecursive, and that skips unknown values without following pointers. - A weighted work budget. @@ -563,19 +571,20 @@ instead use another strategy with an equivalent bound, such as: A reader that delegates decoding to caller-defined code should document where that responsibility transfers. -Decoding into a specific typed destination does not bound the work by itself. -Recursive types, attacker-sized collections, repeated recognized fields, and -repeated materialization can all still amplify it. A schema-aware reader can add -tighter semantic limits where it knows a collection's valid size is small. Those -do not replace a general bound for unknown fields, custom schemas, and -dynamically shaped output. +A finite, nonrecursive destination can provide an inherent bound when its +recognized fields have bounded shapes and unknown values are skipped without +following their pointers. A specific typed destination does not provide that +bound by itself. It can still contain recursive types, attacker-sized +collections, repeated recognized fields, or dynamically shaped values. A +schema-aware reader can apply tighter semantic limits where it knows a +collection's valid size is small. ### Maximum data structure depth -A reader should limit how deeply it nests while decoding one record. The depth -increases by one each time the reader enters a map or an array, or follows a -pointer into one. If the depth exceeds 512, the reader should stop and reject -the record. +A reader that uses a nesting counter should limit how deeply it decodes one +record. The depth increases by one each time the reader enters a map or an +array, or follows a pointer. If the depth exceeds 512, the reader should stop +and reject the record. This limit stops unbounded recursion, including a pointer cycle and a structure nested deeper than any real database needs, and it is a portable default. @@ -609,11 +618,10 @@ counting only after that allocation may be too late. A reader can reserve the declared children against its value budget first, cap the allocation separately, grow storage incrementally, or use another approach with an equivalent bound. -The bound should cover the complete externally requested operation rather than -restart at internal phase boundaries. For example, path navigation and decoding -the selected value can share one budget, or be covered by another end-to-end -bound. Bounding each navigation step on its own does not bound their cumulative -work. +The bound should cover the whole operation the caller requested. It should not +restart between internal phases. For example, path navigation and decoding the +selected value can share one budget or use another end-to-end control. Bounding +each navigation step on its own does not bound their cumulative work. A limit of 65,536 (2\*\*16) values is recommended. The largest records MaxMind produces decode a few hundred values, so this leaves a wide margin. @@ -627,17 +635,16 @@ flat rule above. It can still describe more than 1 TiB of repeated data in a file barely larger than the value itself. A reader that copies, validates, or allocates these values should bound the -total string and bytes data it materializes for one lookup. The right method and -limit depend on the reader's language and API, so this specification does not -require a single limit. As guidance, the largest records MaxMind produces hold -about a kilobyte of such data, so a few megabytes is generous. - -A reader can borrow these bytes, returning values that point into the data -section. Borrowing does not remove the cost. It moves the cost to whatever -copies the values next, and every consumer that copies pays it again. -Memoization has the same limit. Both bound only the reader's own -materialization, so a consumer that copies into owning values should bound that -step too. +total string and bytes data it materializes for one caller-requested decode +operation. The right method and limit depend on the reader's language and API, +so this specification does not require a single limit. As guidance, the largest +records MaxMind produces hold about a kilobyte of such data, so a few megabytes +is generous. + +Borrowing bytes from the data section or safely memoizing pointer targets can +bound the reader's own materialization. The decoded result may still contain +many logical references to the same value. A binding, serializer, or conversion +to owning values that copies each occurrence should bound that downstream work. Payload that a reader skips without copying, validating, or allocating need not consume a materialization budget, as long as the traversal to skip it is bounded @@ -647,10 +654,10 @@ work should bound it. A reader can bound this in several ways: -- Charge each value's size against a per-lookup budget wherever it is decoded, - including data stored inline in a container that a pointer targets. Stop when - the total exceeds a limit. Re-decoding a pointer target charges its data - again, which is what bounds the amplification. +- Charge each value's size against a per-operation budget wherever it is + decoded, including data stored inline in a container that a pointer targets. + Stop when the total exceeds a limit. Re-decoding a pointer target charges its + data again, which is what bounds the amplification. - Fold these concerns into a unified weighted budget. It can account for container entries traversed, pointer expansion when the target is not safely reused, map-key and selector work, and strings and bytes materialized. A small From bc1fa6cfbffb197b50f92f8b4a19766ca2558c54 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:44:29 +0000 Subject: [PATCH 15/21] Remove unpublished advisory reference --- pkg/writer/pointerdos.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index 79151fc4..2626c09a 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -71,8 +71,8 @@ func dataRecordValue24(nodeCount uint32, dataOffset int) uint32 { // offset 0 and each array points back to the level below it. A decoder that // re-decodes a shared pointer target once per referencing path performs // 2**depth leaf decodes and Θ(2**depth) total decode operations from these few -// hundred bytes (GHSA-hj94-g986-h9r7); a decoder that memoizes resolved targets -// decodes it in linear time. mmdbwriter cannot produce this shape because its +// hundred bytes; a decoder that memoizes resolved targets decodes it in linear +// time. mmdbwriter cannot produce this shape because its // own deduplication fans out while computing the file, so the bytes are written // directly. top is the data-section offset of the outermost array. func buildPointerFanOutData(depth int) (data []byte, top int) { From 4ef895a37f913337d866cd74d26118d6059e56f0 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:44:34 +0000 Subject: [PATCH 16/21] Warn about hostile MMDB test fixtures --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index d5d20bdf..43616d01 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,12 @@ subnets (IPv4 or IPv6). This repository contains the spec for that format as well as test databases. +Some structurally well-formed databases under `test-data/` are deliberately +hostile and can exhaust an unprotected reader. Do not fully decode every fixture +without resource controls. See the +[denial-of-service test data](test-data/README.md#denial-of-service-test-data) +documentation. + # Generating Test Data The `write-test-data` command generates the MMDB test files under `test-data/` From 150f2afdb4c947173cb818d9c6fc90d1d5b62f2b Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:44:34 +0000 Subject: [PATCH 17/21] Document resource limit fixture policies --- test-data/README.md | 79 ++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/test-data/README.md b/test-data/README.md index c379b07c..29883507 100644 --- a/test-data/README.md +++ b/test-data/README.md @@ -34,31 +34,37 @@ broken, and exploited functionality simply not available in the go mmdbwriter: ## Denial of service test data -Some files in this directory are hostile by design. Each one is a valid MaxMind -DB file. A reader without resource limits uses very large amounts of CPU or -memory to decode them. Use them to test the limits in the "Reader Resource -Limits" section of the spec. - -Do not decode these files without a memory limit on the process. The worst case -file is about 200 KB and expands to about 4 GiB. A test harness that walks this -whole directory and decodes every record will hang or run out of memory. - -| File | What a reader must survive | -| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | -| MaxMind-DB-test-pointer-decoder-dos.mmdb | A depth 40 pointer fan-out. An unprotected decoder performs 2\*\*40 leaf decodes from 451 bytes. | -| MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb | The same fan-out in a conventional IPv6 database that maps the whole address space to the record. | -| MaxMind-DB-test-payload-amplification-dos.mmdb | 8,192 pointers to one 65,535 byte bytes value. A reader that copies each target materializes about 512 MiB. | -| MaxMind-DB-test-payload-amplification-dos-string.mmdb | The same shape with a UTF-8 string value, the type most bindings copy into a native string. | -| MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb | 65,535 pointers to that value. The record decodes to 65,536 values, which meets the recommended value limit, and materializes about 4 GiB. | - -These files are the boundary cases. A reader at the recommended limits accepts -the first file in each pair and rejects the second. +Some files in this directory are hostile by design. Each one is structurally +well-formed, but a reader's resource policy may still refuse to open or decode +it. A reader without resource controls can use far more CPU or memory than the +file size suggests. Use these files to test the guidance in the +[Reader Resource Limits](../MaxMind-DB-spec.md#reader-resource-limits) section +of the specification. + +These files do not define an exact boundary for the recommended nesting depth. +Use a reader-level test to verify the exact boundary that implementation uses. + +Do not decode these files without time and memory limits on the process. The +worst-case file is about 192 KiB and describes about 4 GiB of repeated data. A +test harness that walks this whole directory and decodes every data entry will +hang or run out of memory. + +| File | Behavior under test | +| --------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | +| MaxMind-DB-test-pointer-decoder-dos.mmdb | A depth-40 pointer fan-out. An unprotected decoder performs 2\*\*40 leaf decodes from 451 bytes. | +| MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb | The same fan-out in a conventional IPv6 database that maps the whole address space to the data entry. | +| MaxMind-DB-test-payload-amplification-dos.mmdb | 8,192 pointers to one 65,535-byte value of type `bytes`. A reader that copies each target materializes about 512 MiB. | +| MaxMind-DB-test-payload-amplification-dos-string.mmdb | The same shape with a UTF-8 string value, the type most bindings copy into a native string. | +| MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb | 65,535 pointers to that value. The data entry decodes to 65,536 values, meets the recommended value limit, and materializes about 4 GiB. | + +The next table lists example boundary fixtures. Its results assume the flat +value accounting rule and the 2 MiB payload limit described below. | File | Expected result | | ------------------------------------------------------ | ------------------------------------------------------------------------------- | | MaxMind-DB-test-decoder-value-limit.mmdb | Accept. 65,536 decoded values, exactly the limit. | | MaxMind-DB-test-decoder-value-limit-over.mmdb | Reject. 65,537 decoded values. | -| MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb | Accept. 65,535 values reached through a depth 15 fan-out. | +| MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb | Accept. 65,535 values reached through a depth-15 fan-out. | | MaxMind-DB-test-decoder-payload-limit.mmdb | Accept. 2,097,152 payload bytes, exactly 2 MiB. | | MaxMind-DB-test-decoder-payload-limit-over.mmdb | Reject. 2,097,153 payload bytes. | | MaxMind-DB-test-metadata-payload-limit.mmdb | Reject at open. The metadata alone materializes 2,228,190 bytes. | @@ -70,11 +76,19 @@ The spec recommends a limit of 65,536 decoded values. It does not require a single payload limit, because the right method and limit depend on the reader's language and API. -The payload files use 2 MiB (2\*\*21 bytes) because libmaxminddb and the MaxMind -readers use that value. A reader that picks a different payload limit is still -compliant. For that reader, treat the two `-decoder-payload-limit` files as an -example of the attack shape, not as a pass or fail requirement. Adjust the -expected boundary to the limit the reader actually uses. +The payload files use 2 MiB (2\*\*21 bytes), matching the default in the +companion [libmaxminddb](https://github.com/maxmind/libmaxminddb/pull/479) pull +request and the limits in the companion +[Go](https://github.com/oschwald/maxminddb-golang/pull/233), +[Python](https://github.com/maxmind/MaxMind-DB-Reader-python/pull/439), +[Ruby](https://github.com/maxmind/MaxMind-DB-Reader-ruby/pull/235), +[PHP](https://github.com/maxmind/MaxMind-DB-Reader-php/pull/281), +[Java](https://github.com/maxmind/MaxMind-DB-Reader-java/pull/442), and +[.NET](https://github.com/maxmind/MaxMind-DB-Reader-dotnet/pull/355) reader pull +requests. This shared test boundary is not a file format requirement. A reader +that picks a different payload limit or uses an equivalent strategy is still +compliant. Treat these files as examples of the attack shape and adjust the +expected boundary to the reader's policy. ### The payload files assume per-occurrence accounting @@ -82,11 +96,16 @@ The payload boundary files point 32 of their 33 elements at one shared value. They separate accept from reject only for a reader that charges every decoded occurrence. -A reader that memoizes pointer targets materializes the shared value one time, -about 64 KB. That reader accepts both files. This is correct behavior for it. -The spec lists safe memoization as one way to bound the amplification. Test such -a reader with the amplification files above, which no accounting model can -expand cheaply. +A reader that safely memoizes pointer targets can materialize the shared value +one time, about 64 KiB, and accept both files. That behavior is compliant. The +result can still contain many logical references to the shared value. A binding, +serializer, or conversion to owning values that copies each occurrence should +apply its own bound. The amplification files above can test that downstream +boundary. + +The pointer fan-out files test a reader without safe reuse or another work +bound. The worst-case payload file tests a reader whose only defense is the +decoded-value count. ## Usage From a93cc69516752f6f7934c0beea4fbadd4ba9e67d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:44:35 +0000 Subject: [PATCH 18/21] Polish resource limit specification wording --- MaxMind-DB-spec.md | 45 ++++++++++++++++++++-------------------- pkg/writer/pointerdos.go | 2 +- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/MaxMind-DB-spec.md b/MaxMind-DB-spec.md index 6e008890..38b68c04 100644 --- a/MaxMind-DB-spec.md +++ b/MaxMind-DB-spec.md @@ -544,9 +544,9 @@ section size for the database is limited to 4GB. ## Reader Resource Limits A crafted or corrupt data section can make a reader use far more time and memory -than the record's size suggests. A value can be a pointer, and several pointers -can share one target. A small data section can therefore describe a structure -that is huge, or effectively infinite, when fully expanded. +than a data entry's encoded size suggests. A value can be a pointer, and several +pointers can share one target. A small data section can therefore describe a +structure that is huge, or effectively infinite, when fully expanded. A reader should apply resource controls to operations that decode attacker-controlled values. The controls should bound the reader's time and @@ -579,26 +579,26 @@ collections, repeated recognized fields, or dynamically shaped values. A schema-aware reader can apply tighter semantic limits where it knows a collection's valid size is small. -### Maximum data structure depth +### Maximum Data Structure Depth -A reader that uses a nesting counter should limit how deeply it decodes one -record. The depth increases by one each time the reader enters a map or an -array, or follows a pointer. If the depth exceeds 512, the reader should stop -and reject the record. +A reader that uses a nesting counter should limit how deeply it decodes one data +entry. The depth increases by one each time the reader enters a map or an array, +or follows a pointer. If the depth exceeds 512, the reader should stop and +reject the data entry. This limit stops unbounded recursion, including a pointer cycle and a structure nested deeper than any real database needs, and it is a portable default. Iterative decoding avoids call-stack exhaustion, but it must still detect cycles and bound nesting and traversal work. -### Maximum decoded value count +### Maximum Decoded Value Count The depth limit does not bound the total amount of work. Arrays and maps can contain many pointers to the same target. Unless the reader safely reuses the target, it decodes that target once for every pointer occurrence. In the binary fan-out example used by the test data, each array contains two pointers to the level below, so each lower level doubles the work. Decoding the top value takes -exponentially more operations than the depth suggests. A record under one +exponentially more operations than the depth suggests. A data entry under one kilobyte can therefore take longer to decode than any real workload allows. A reader can bound this by counting the values it decodes and stopping if the @@ -623,23 +623,24 @@ restart between internal phases. For example, path navigation and decoding the selected value can share one budget or use another end-to-end control. Bounding each navigation step on its own does not bound their cumulative work. -A limit of 65,536 (2\*\*16) values is recommended. The largest records MaxMind -produces decode a few hundred values, so this leaves a wide margin. +A limit of 65,536 (2\*\*16) values is recommended. The largest data entries +MaxMind produces decode a few hundred values, so this leaves a wide margin. -### Maximum expanded payload work +### Bounding Expanded Payload -The value count limits how many fields a record decodes, not how many bytes they -hold. A single string or bytes value can be up to 16,843,036 bytes. An array of -65,535 pointers to one such value stays at the recommended value limit under the -flat rule above. It can still describe more than 1 TiB of repeated data in a -file barely larger than the value itself. +The value-count strategy limits how many values the reader decodes from a data +entry, not how many bytes those values contain. A single string or bytes value +can be up to 16,843,036 bytes. An array of 65,535 pointers to one such value +stays at the recommended value limit under the flat rule above. It can still +describe more than 1 TiB of repeated data in a file barely larger than the value +itself. A reader that copies, validates, or allocates these values should bound the total string and bytes data it materializes for one caller-requested decode operation. The right method and limit depend on the reader's language and API, so this specification does not require a single limit. As guidance, the largest -records MaxMind produces hold about a kilobyte of such data, so a few megabytes -is generous. +data entries MaxMind produces hold about a kilobyte of such data, so a few +megabytes is generous. Borrowing bytes from the data section or safely memoizing pointer targets can bound the reader's own materialization. The decoded result may still contain @@ -648,7 +649,7 @@ to owning values that copies each occurrence should bound that downstream work. Payload that a reader skips without copying, validating, or allocating need not consume a materialization budget, as long as the traversal to skip it is bounded -separately. A reader selecting part of a record likewise need not expand an +separately. A reader selecting part of a data entry likewise need not expand an unrequested pointer target. A reader whose API or validation rules require that work should bound it. @@ -669,7 +670,7 @@ A reader can bound this in several ways: When a reader's resource controls reject a decode, it should return an error rather than a partial result. It may treat a value outside its documented resource profile as invalid, and may make limits configurable for unusually -large valid records. +large valid data entries. ## Reference Implementations diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index 2626c09a..be7a1722 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -108,7 +108,7 @@ const ( // its elements decode to worstCasePointerCount+1 = 65,536 values, which meets // the recommended limit without exceeding it, so that reader does not reject // it. Copying each target materializes worstCasePointerCount*payloadScalarSize - // bytes, about 4 GiB, from a file of about 200 KiB. A limit on the copied + // bytes, about 4 GiB, from a file of about 192 KiB. A limit on the copied // bytes stops this, as can safe reuse or structural rejection. worstCasePointerCount = 65535 ) From 42c284e27d747ceb2b632142d599ea70903445a5 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:23:29 +0000 Subject: [PATCH 19/21] Improve raw encoder boundary coverage --- pkg/writer/rawmmdb_test.go | 98 ++++++++++++++++++++++++++++++-------- 1 file changed, 79 insertions(+), 19 deletions(-) diff --git a/pkg/writer/rawmmdb_test.go b/pkg/writer/rawmmdb_test.go index 6ef7fc71..0386a5e6 100644 --- a/pkg/writer/rawmmdb_test.go +++ b/pkg/writer/rawmmdb_test.go @@ -2,13 +2,14 @@ package writer import ( "bytes" + "fmt" "strings" "testing" ) -// TestWriteMapControlByte covers all three size forms. The fixtures only -// exercise the one-byte and three-byte forms, so a wrong byte in the two-byte -// form would otherwise reach a generated file unnoticed. +// TestWriteMapControlByte covers the direct form and all three extended size +// forms. Current fixtures exercise the one- and three-byte map headers, so +// explicit cases preserve the two- and four-byte boundaries. func TestWriteMapControlByte(t *testing.T) { tests := []struct { size int @@ -22,6 +23,7 @@ func TestWriteMapControlByte(t *testing.T) { {512, []byte{0xFE, 0x00, 0xE3}}, {65820, []byte{0xFE, 0xFF, 0xFF}}, {65821, []byte{0xFF, 0x00, 0x00, 0x00}}, + {1_000_000, []byte{0xFF, 0x0E, 0x41, 0x23}}, {maximumDataStructureSize, []byte{0xFF, 0xFF, 0xFF, 0xFF}}, } for _, test := range tests { @@ -49,6 +51,7 @@ func TestWriteStringControlByte(t *testing.T) { {285, []byte{0x5E, 0x00, 0x00}}, {65820, []byte{0x5E, 0xFF, 0xFF}}, {65821, []byte{0x5F, 0x00, 0x00, 0x00}}, + {1_000_000, []byte{0x5F, 0x0E, 0x41, 0x23}}, } for _, test := range tests { value := strings.Repeat("a", test.size) @@ -74,6 +77,7 @@ func TestWriteArrayHeader(t *testing.T) { {285, []byte{0x1E, 0x04, 0x00, 0x00}}, {65820, []byte{0x1E, 0x04, 0xFF, 0xFF}}, {65821, []byte{0x1F, 0x04, 0x00, 0x00, 0x00}}, + {1_000_000, []byte{0x1F, 0x04, 0x0E, 0x41, 0x23}}, {maximumDataStructureSize, []byte{0x1F, 0x04, 0xFF, 0xFF, 0xFF}}, } for _, test := range tests { @@ -96,32 +100,88 @@ func TestWriteSearchTreeRecordsMaximum(t *testing.T) { func TestRawWritersRejectInvalidInput(t *testing.T) { tests := []struct { - name string - call func() + name string + call func() + wantPanic string }{ - {"negative map size", func() { writeMap(make([]byte, 8), -1) }}, - {"oversized map", func() { writeMap(make([]byte, 8), maximumDataStructureSize+1) }}, - {"undersized large map", func() { writeLargeMap(make([]byte, 8), maximumSizeCode30) }}, - {"negative array size", func() { writeArrayHeader(make([]byte, 8), -1) }}, + { + "negative map size", + func() { writeMap(make([]byte, 8), -1) }, + "map size -1 is outside the supported range", + }, + { + "oversized map", + func() { writeMap(make([]byte, 8), maximumDataStructureSize+1) }, + "map size 16843037 is outside the supported range", + }, + { + "undersized large map", + func() { writeLargeMap(make([]byte, 8), maximumSizeCode30) }, + "map size 65820 is outside the case-31 range", + }, + { + "oversized large map", + func() { writeLargeMap(make([]byte, 8), maximumDataStructureSize+1) }, + "map size 16843037 is outside the case-31 range", + }, + { + "negative array size", + func() { writeArrayHeader(make([]byte, 8), -1) }, + "array size -1 is outside the supported range", + }, { "oversized array", func() { writeArrayHeader(make([]byte, 8), maximumDataStructureSize+1) }, + "array size 16843037 is outside the supported range", + }, + { + "undersized large array", + func() { writeLargeArray(make([]byte, 8), maximumSizeCode30) }, + "array size 65820 is outside the case-31 range", + }, + { + "oversized large array", + func() { writeLargeArray(make([]byte, 8), maximumDataStructureSize+1) }, + "array size 16843037 is outside the case-31 range", + }, + { + "negative scalar size", + func() { writeScalar(make([]byte, 8), -1, scalarTypeBytes) }, + "scalar size -1 is outside the supported range", + }, + { + "oversized scalar", + func() { writeScalar(make([]byte, 8), maximumDataStructureSize+1, scalarTypeBytes) }, + "scalar size 16843037 is outside the supported range", + }, + { + "invalid scalar type", + func() { writeScalar(make([]byte, 8), 0, 8) }, + "unsupported scalar type 8", + }, + { + "oversized left search-tree record", + func() { + writeSearchTreeRecords(make([]byte, 8), maximum24BitSearchTreeValue+1, 0) + }, + "search-tree record values 16777216 and 0 must fit in 24 bits", + }, + { + "oversized right search-tree record", + func() { + writeSearchTreeRecords(make([]byte, 8), 0, maximum24BitSearchTreeValue+1) + }, + "search-tree record values 0 and 16777216 must fit in 24 bits", }, - {"undersized large array", func() { writeLargeArray(make([]byte, 8), maximumSizeCode30) }}, - {"negative scalar size", func() { writeScalar(make([]byte, 8), -1, scalarTypeBytes) }}, - {"oversized scalar", func() { - writeScalar(make([]byte, 8), maximumDataStructureSize+1, scalarTypeBytes) - }}, - {"invalid scalar type", func() { writeScalar(make([]byte, 8), 0, 8) }}, - {"oversized search-tree record", func() { - writeSearchTreeRecords(make([]byte, 8), maximum24BitSearchTreeValue+1, 0) - }}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { defer func() { - if recover() == nil { + got := recover() + if got == nil { t.Error("call did not panic") + } else if message := fmt.Sprint(got); !strings.Contains(message, test.wantPanic) { + t.Errorf("panic = %q, want it to contain %q", message, test.wantPanic) } }() test.call() From 694aceeb8c530e9d49ac0d8f8e1a6eca9a1bca2e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 17:24:00 +0000 Subject: [PATCH 20/21] Clarify resource limit fixture behavior --- pkg/writer/pointerdos_test.go | 9 +++++---- test-data/README.md | 32 +++++++++++++++++--------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index ea79e02b..5a267fa7 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -432,10 +432,11 @@ func scalarPayloadSize(t *testing.T, data []byte, offset int) int { } } -// sumPointedPayload adds up the payload that every element of the array at top -// references, which is what a reader charging each occurrence materializes. It -// reads the generated bytes rather than recomputing the builder's arithmetic, so -// a change to the encoding or the element count fails the assertion. +// sumPointedPayload adds the payload referenced by each element of the array at +// top. This is the per-occurrence total used by the flat payload-budget fixtures; +// it does not imply that a memoizing reader materializes each target again. The +// function reads the generated bytes rather than recomputing the builder's +// arithmetic, so a change to the encoding or element count fails the assertion. func sumPointedPayload(t *testing.T, data []byte, top int) int { t.Helper() count, headerSize := arrayElementCount(t, data, top) diff --git a/test-data/README.md b/test-data/README.md index 29883507..ce934ef3 100644 --- a/test-data/README.md +++ b/test-data/README.md @@ -46,16 +46,16 @@ Use a reader-level test to verify the exact boundary that implementation uses. Do not decode these files without time and memory limits on the process. The worst-case file is about 192 KiB and describes about 4 GiB of repeated data. A -test harness that walks this whole directory and decodes every data entry will -hang or run out of memory. +test harness without resource controls that walks this whole directory and +decodes every data entry will hang or run out of memory. -| File | Behavior under test | -| --------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | -| MaxMind-DB-test-pointer-decoder-dos.mmdb | A depth-40 pointer fan-out. An unprotected decoder performs 2\*\*40 leaf decodes from 451 bytes. | -| MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb | The same fan-out in a conventional IPv6 database that maps the whole address space to the data entry. | -| MaxMind-DB-test-payload-amplification-dos.mmdb | 8,192 pointers to one 65,535-byte value of type `bytes`. A reader that copies each target materializes about 512 MiB. | -| MaxMind-DB-test-payload-amplification-dos-string.mmdb | The same shape with a UTF-8 string value, the type most bindings copy into a native string. | -| MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb | 65,535 pointers to that value. The data entry decodes to 65,536 values, meets the recommended value limit, and materializes about 4 GiB. | +| File | Behavior under test | +| --------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| MaxMind-DB-test-pointer-decoder-dos.mmdb | A depth-40 pointer fan-out. An unprotected decoder performs 2\*\*40 leaf decodes from 451 bytes. | +| MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb | The same fan-out in a conventional IPv6 database that maps the whole address space to the data entry. | +| MaxMind-DB-test-payload-amplification-dos.mmdb | 8,192 pointers to one 65,535-byte value of type `bytes`. A reader that copies each target materializes about 512 MiB. | +| MaxMind-DB-test-payload-amplification-dos-string.mmdb | The same shape with a UTF-8 string value, the type most bindings copy into a native string. | +| MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb | 65,535 pointers to that value. The data entry decodes to 65,536 values and meets the recommended value limit. Copying every occurrence materializes about 4 GiB. | The next table lists example boundary fixtures. Its results assume the flat value accounting rule and the 2 MiB payload limit described below. @@ -76,17 +76,19 @@ The spec recommends a limit of 65,536 decoded values. It does not require a single payload limit, because the right method and limit depend on the reader's language and API. -The payload files use 2 MiB (2\*\*21 bytes), matching the default in the -companion [libmaxminddb](https://github.com/maxmind/libmaxminddb/pull/479) pull -request and the limits in the companion +The payload files use 2 MiB (2\*\*21 bytes), matching the current companion pull +request heads for +[libmaxminddb](https://github.com/maxmind/libmaxminddb/pull/479), [Go](https://github.com/oschwald/maxminddb-golang/pull/233), [Python](https://github.com/maxmind/MaxMind-DB-Reader-python/pull/439), [Ruby](https://github.com/maxmind/MaxMind-DB-Reader-ruby/pull/235), [PHP](https://github.com/maxmind/MaxMind-DB-Reader-php/pull/281), [Java](https://github.com/maxmind/MaxMind-DB-Reader-java/pull/442), and -[.NET](https://github.com/maxmind/MaxMind-DB-Reader-dotnet/pull/355) reader pull -requests. This shared test boundary is not a file format requirement. A reader -that picks a different payload limit or uses an equivalent strategy is still +[.NET](https://github.com/maxmind/MaxMind-DB-Reader-dotnet/pull/355). In +libmaxminddb, 2 MiB is the default and can be changed at build time; the current +Go, Python, Ruby, PHP, Java, and .NET pull request heads use a fixed 2 MiB +limit. This shared test boundary is not a file format requirement. A reader that +picks a different payload limit or uses an equivalent strategy is still compliant. Treat these files as examples of the attack shape and adjust the expected boundary to the reader's policy. From 0f06955b11441dd61150e642850ec84f42b8997e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 1 Sep 2026 18:01:50 +0000 Subject: [PATCH 21/21] Clarify example payload limit terminology --- pkg/writer/pointerdos.go | 20 +++++++++++--------- pkg/writer/pointerdos_test.go | 23 +++++++++++------------ test-data/README.md | 2 +- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/pkg/writer/pointerdos.go b/pkg/writer/pointerdos.go index be7a1722..8069d5fd 100644 --- a/pkg/writer/pointerdos.go +++ b/pkg/writer/pointerdos.go @@ -90,7 +90,7 @@ func buildPointerFanOutData(depth int) (data []byte, top int) { const ( recommendedValueLimit = 1 << 16 - recommendedPayloadLimit = 1 << 21 + fixturePayloadLimit = 1 << 21 decodePathDecoyCount = 511 decodePathSharedKeySize = 4096 decodePathValueSize = 4091 @@ -251,8 +251,9 @@ func buildPayloadLimitData(smallSize int) (data []byte, top int) { // buildDecodePathSharedBudgetData produces a map whose first 511 keys point to // one shared 4 KiB string. Reading those keys and the final inline "target" key // consumes 2,093,062 bytes. The selected 4,091-byte value then takes the total -// to 2,097,153 bytes, exactly one byte above the recommended 2 MiB budget. This -// verifies that path navigation and selected-value decoding share one budget. +// to 2,097,153 bytes, exactly one byte above the 2 MiB example payload budget +// used by these fixtures. This verifies that path navigation and selected-value +// decoding share one budget. func buildDecodePathSharedBudgetData() (data []byte, top int) { data = make([]byte, 16*1024) pos := writeScalar(data, decodePathSharedKeySize, scalarTypeString) @@ -469,10 +470,10 @@ func pointerDoSFixtures() map[string][]byte { recommendedValueLimit, ), payloadLimitFixtureFilename: buildPayloadLimitDB( - recommendedPayloadLimit - 32*payloadScalarSize, + fixturePayloadLimit - 32*payloadScalarSize, ), payloadLimitOverFixtureFilename: buildPayloadLimitDB( - recommendedPayloadLimit - 32*payloadScalarSize + 1, + fixturePayloadLimit - 32*payloadScalarSize + 1, ), metadataLimitFixtureFilename: buildMetadataLimitDB(), decodePathBudgetFixtureFilename: buildDecodePathSharedBudgetDB(), @@ -485,10 +486,11 @@ func pointerDoSFixtures() map[string][]byte { // address space to the fan-out record. Three exercise payload amplification: a // moderate and a worst-case fixture with a shared bytes value, and one with a // shared string value, the type most bindings copy into a native string. It -// also writes exact boundary fixtures for both recommended limits and a -// metadata fixture that exceeds the payload limit while opening. A small path -// fixture verifies that navigation and selected-value decoding share the same -// budget. See buildPointerFanOutData and buildPayloadAmplificationData. +// also writes exact boundary fixtures for the recommended value-count limit and +// the 2 MiB example payload budget, plus a metadata fixture that exceeds that +// budget while opening. A small path fixture verifies that navigation and +// selected-value decoding share the same budget. See buildPointerFanOutData and +// buildPayloadAmplificationData. func (w *Writer) WritePointerDecoderDoSTestDB() error { for name, db := range pointerDoSFixtures() { path := filepath.Clean(filepath.Join(w.target, name)) diff --git a/pkg/writer/pointerdos_test.go b/pkg/writer/pointerdos_test.go index 5a267fa7..519b4375 100644 --- a/pkg/writer/pointerdos_test.go +++ b/pkg/writer/pointerdos_test.go @@ -185,8 +185,8 @@ func TestDecodePathSharedBudgetData(t *testing.T) { const targetKeySize = len("target") navigated := decodePathDecoyCount*decodePathSharedKeySize + targetKeySize - if got := navigated + decodePathValueSize; got != recommendedPayloadLimit+1 { - t.Errorf("navigated and selected payload = %d, want %d", got, recommendedPayloadLimit+1) + if got := navigated + decodePathValueSize; got != fixturePayloadLimit+1 { + t.Errorf("navigated and selected payload = %d, want %d", got, fixturePayloadLimit+1) } } @@ -206,11 +206,11 @@ func TestDecodePathSharedBudgetFixtureIsSemanticallyValid(t *testing.T) { if err := result.Err(); err != nil { t.Fatalf("lookup: %v", err) } - // The fixture sits one byte above the payload budget the spec calls a - // recommendation, so both outcomes are compliant: the reader returns the - // whole value, or it refuses on its own limit. Assert that disjunction. A - // partial value, a wrong length, or a panic still fails. The bytes are - // checked by TestDecodePathSharedBudgetData and + // The fixture sits one byte above its 2 MiB example payload budget. The spec + // does not prescribe this threshold, so both outcomes are compliant: the + // reader returns the whole value, or it refuses on its own limit. Assert that + // disjunction. A partial value, a wrong length, or a panic still fails. The + // bytes are checked by TestDecodePathSharedBudgetData and // TestPointerFanOutFixturesMatchCommitted, which do not depend on the reader. var value string err = result.DecodePath(&value, "target") @@ -226,11 +226,10 @@ func TestDecodePathSharedBudgetFixtureIsSemanticallyValid(t *testing.T) { func TestMetadataLimitFixtureIsSemanticallyValid(t *testing.T) { path := filepath.Clean(filepath.Join("..", "..", "test-data", metadataLimitFixtureFilename)) - // The metadata materializes 2,228,190 bytes while opening, above the payload - // budget the spec calls a recommendation. A reader that bounds metadata - // rejects the file here, which is the behavior this fixture exists to - // provoke, so treat that as a pass and check the sizes only when the reader - // accepts it. + // The metadata materializes 2,228,190 bytes while opening, above the 2 MiB + // example payload budget used by these fixtures. A reader that bounds metadata + // rejects the file here, which is the behavior this fixture exists to provoke, + // so treat that as a pass and check the sizes only when the reader accepts it. db, err := maxminddb.Open(path) if err != nil { if !isReaderResourceLimitError(err) { diff --git a/test-data/README.md b/test-data/README.md index ce934ef3..0307ae26 100644 --- a/test-data/README.md +++ b/test-data/README.md @@ -70,7 +70,7 @@ value accounting rule and the 2 MiB payload limit described below. | MaxMind-DB-test-metadata-payload-limit.mmdb | Reject at open. The metadata alone materializes 2,228,190 bytes. | | MaxMind-DB-test-decode-path-shared-budget.mmdb | Reject a path lookup. Navigation plus the selected value costs 2,097,153 bytes. | -### The boundary values are recommendations +### The value-count boundary is a recommendation The spec recommends a limit of 65,536 decoded values. It does not require a single payload limit, because the right method and limit depend on the reader's