From f15a433da90c8ab6adaed40024e59e4a0186ec75 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 00:20:09 +0000 Subject: [PATCH 01/13] Bound decoder work to prevent a pointer fan-out denial of service A crafted data section could nest pointers to shared targets so that decoding one record cost exponential time and memory from a small file (GHSA-hj94-g986-h9r7). The pure PHP decoder now limits the number of values it decodes for a single record and rejects a database that exceeds the limit with an InvalidDatabaseException. The limit is 65,536, far above the few hundred values the largest real records decode. The count follows the flat rule from the MaxMind DB specification: the root is one value, each array and map charges its declared children before it reads any of them, and a pointer costs nothing beyond the value it resolves to. Pointer cycles and over-deep data are rejected by a depth limit of 512 rather than exhausting the stack, which PHP cannot recover from. Both limits are the ones the specification recommends. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 8 + src/MaxMind/Db/Reader/Decoder.php | 93 ++++++++- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 191 +++++++++++++++++++ 3 files changed, 282 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb1bbb31..18ed7cac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ CHANGELOG 1.14.0 ------------------- +* Bounded the resources that the pure PHP decoder spends on a single lookup. A + crafted database could nest data-section pointers to shared targets so that + decoding one record cost exponential time and memory. The decoder now follows + the Reader Resource Limits section of the MaxMind DB specification. Each + lookup is limited to 65,536 values and 512 levels of nesting. + * Exceeding a limit throws an `InvalidDatabaseException`. + * Opening a database whose metadata exceeds a limit throws the same + exception. * The Windows build configuration now accepts either `libmaxminddb.lib` or `maxminddb.lib` when building the extension. The `lib` prefix was removed in libmaxminddb 1.6.0, but the libmaxminddb that PHP publishes for Windows diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 1bb67316..50388c1b 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -47,6 +47,18 @@ class Decoder private const _BOOLEAN = 14; private const _FLOAT = 15; + // Per-lookup decode limits recommended by the MaxMind DB specification. The + // depth limit stops pointer cycles and over-deep data. The value limit + // stops a pointer fan-out, where nested pointers to shared targets would + // otherwise cost 2**depth decode operations. The count follows the + // specification's flat rule: the root is one value, each array and map + // charges its declared children (a map entry costs two, key and value), + // and a pointer costs nothing beyond the value it resolves to, which its + // container already charged. The largest real records decode a few hundred + // values, so the limit leaves a wide margin. + private const MAX_DEPTH = 512; + private const MAX_VALUES = 1 << 16; + /** * @param resource $fileStream */ @@ -67,6 +79,21 @@ public function __construct( * @return array */ public function decode(int $offset): array + { + // Bound the work per lookup so a crafted database cannot exhaust CPU or + // memory. $budget is passed by reference so the running count is shared + // across the recursion. It is call-local, so concurrent lookups do not + // share state. The root value is charged here; containers charge their + // children. + $budget = self::MAX_VALUES - 1; + + return $this->decodeWithBudget($offset, 0, $budget); + } + + /** + * @return array + */ + private function decodeWithBudget(int $offset, int $depth, int &$budget): array { $ctrlByte = \ord(Util::read($this->fileStream, $offset, 1)); ++$offset; @@ -84,7 +111,16 @@ public function decode(int $offset): array return [$pointer]; } - [$result] = $this->decode($pointer); + if ($depth >= self::MAX_DEPTH) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth" + ); + } + + // The value at the pointer's position was charged by its containing + // array or map, so the target costs nothing more. Only the depth + // grows. + [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget); return [$result, $offset]; } @@ -108,7 +144,7 @@ public function decode(int $offset): array [$size, $offset] = $this->sizeFromCtrlByte($ctrlByte, $offset); - return $this->decodeByType($type, $offset, $size); + return $this->decodeByType($type, $offset, $size, $depth, $budget); } /** @@ -116,14 +152,14 @@ public function decode(int $offset): array * * @return array{0:mixed, 1:int} */ - private function decodeByType(int $type, int $offset, int $size): array + private function decodeByType(int $type, int $offset, int $size, int $depth, int &$budget): array { switch ($type) { case self::_MAP: - return $this->decodeMap($size, $offset); + return $this->decodeMap($size, $offset, $depth, $budget); case self::_ARRAY: - return $this->decodeArray($size, $offset); + return $this->decodeArray($size, $offset, $depth, $budget); case self::_BOOLEAN: return [$this->decodeBoolean($size), $offset]; @@ -172,15 +208,49 @@ private function verifySize(int $expected, int $actual): void } } + /** + * Applies the per-lookup limits when entering a container. The depth limit + * stops cycles and over-deep data (checked here and at pointer follows, + * the only places depth grows). The value budget is charged per declared + * element up front, so an oversized declared size is rejected before the + * loop reads anything. A pointer element costs nothing more when it is + * followed: its slot is charged here, and a container it resolves to + * charges its own children each time it is decoded, which is what bounds + * a fan-out through shared targets. + */ + private function enterContainer( + int $size, + int $depth, + int &$budget, + int $valuesPerEntry = 1 + ): void { + if ($depth >= self::MAX_DEPTH) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth" + ); + } + // Compare with a division rather than multiplying the declared size, so + // an oversized declaration cannot overflow the integer on 32-bit builds + // before the budget check runs. + if ($size > intdiv($budget, $valuesPerEntry)) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum number of values" + ); + } + $budget -= $size * $valuesPerEntry; + } + /** * @return array{0:array, 1:int} */ - private function decodeArray(int $size, int $offset): array + private function decodeArray(int $size, int $offset, int $depth, int &$budget): array { + $this->enterContainer($size, $depth, $budget); + $array = []; for ($i = 0; $i < $size; ++$i) { - [$value, $offset] = $this->decode($offset); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget); $array[] = $value; } @@ -258,13 +328,16 @@ private function decodeInt32(string $bytes, int $size): int /** * @return array{0:array, 1:int} */ - private function decodeMap(int $size, int $offset): array + private function decodeMap(int $size, int $offset, int $depth, int &$budget): array { + // A map entry decodes a key and a value, so it costs two values. + $this->enterContainer($size, $depth, $budget, 2); + $map = []; for ($i = 0; $i < $size; ++$i) { - [$key, $offset] = $this->decode($offset); - [$value, $offset] = $this->decode($offset); + [$key, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget); $map[$key] = $value; } diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index e9354526..2c1b4820 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -5,6 +5,7 @@ namespace MaxMind\Db\Test\Reader; use MaxMind\Db\Reader\Decoder; +use MaxMind\Db\Reader\InvalidDatabaseException; use PHPUnit\Framework\TestCase; /** @@ -419,6 +420,196 @@ private function validateTypeDecodingList(string $type, array $tests): void } } + private function encodePointer1(int $target): string + { + // One-byte-payload pointer (type 1, pointer_size 1) with base 0. + return \chr((1 << 5) | (($target >> 8) & 0x7)) . \chr($target & 0xFF); + } + + public function testPointerFanOutIsBounded(): void + { + // A data section of nested arrays, each holding two pointers to the + // node below, would cost 2**depth decode operations. The decoder bounds + // the number of values it decodes per lookup and rejects the database. + $depth = 100; + $buf = "\xa0"; // leaf: uint16 with value 0 + $prev = 0; + for ($i = 0; $i < $depth; ++$i) { + $offset = \strlen($buf); + $buf .= "\x02\x04" . $this->encodePointer1($prev) . $this->encodePointer1($prev); + $prev = $offset; + } + + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $buf); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum number of values" + ); + (new Decoder($handle, 0))->decode($prev); + } + + public function testMapPointerFanOutIsBounded(): void + { + // Each map has two distinct UTF-8 keys whose values point to the map below. + // This makes the decoder visit the shared target twice per layer while + // keeping the fixture itself small. + $depth = 100; + $buf = "\xa0"; // leaf: uint16 with value 0 + $prev = 0; + for ($i = 0; $i < $depth; ++$i) { + $offset = \strlen($buf); + $buf .= "\xe2\x41a" . $this->encodePointer1($prev) + . "\x41b" . $this->encodePointer1($prev); + $prev = $offset; + } + + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $buf); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum number of values" + ); + (new Decoder($handle, 0))->decode($prev); + } + + public function testOversizedArrayIsBounded(): void + { + // The root is one value and an array charges one value per declared + // element, so an array that declares 65,536 elements reaches 65,537 + // values, one past the limit. It is rejected on its header alone: no + // element follows in the stream, so reading one would fail with a + // different error. 0x1e is an array (extended type 0x04) with size + // code 30, then the two size bytes for 65,536 - 285 = 65,251 (0xfee3). + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\x1e\x04\xfe\xe3"); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum number of values" + ); + (new Decoder($handle, 0))->decode(0); + } + + public function testPointerFreeContainerAtMaximumDepthDecodes(): void + { + $buf = "\xa0"; // leaf: uint16 with value 0 + for ($i = 0; $i < 512; ++$i) { + $buf = "\x01\x04" . $buf; // array with one element + } + + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $buf); + fseek($handle, 0); + + [, $offset] = (new Decoder($handle, 0))->decode(0); + $this->assertSame(\strlen($buf), $offset); + } + + public function testPointerFreeContainerOverMaximumDepthIsBounded(): void + { + $buf = "\xa0"; // leaf: uint16 with value 0 + for ($i = 0; $i < 513; ++$i) { + $buf = "\x01\x04" . $buf; // array with one element + } + + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $buf); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum depth" + ); + (new Decoder($handle, 0))->decode(0); + } + + /** + * Builds a chain of one-element arrays where each element is a pointer to + * the array below, so every level costs one container entry and one + * pointer follow. Returns the buffer and the offset of the top array. + * + * @return array{0:string, 1:int} + */ + private function pointerChain(int $arrays): array + { + $buf = "\xa0"; // leaf: uint16 with value 0 + $prev = 0; + for ($i = 0; $i < $arrays; ++$i) { + $offset = \strlen($buf); + $buf .= "\x01\x04" . $this->encodePointer1($prev); + $prev = $offset; + } + + return [$buf, $prev]; + } + + public function testPointerChainAtMaximumDepthDecodes(): void + { + // A pointer follow counts as one level, like entering a container, so + // 256 arrays reached through 256 pointers enter exactly 512 levels. + [$buf, $top] = $this->pointerChain(256); + + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $buf); + fseek($handle, 0); + + [$value, $offset] = (new Decoder($handle, 0))->decode($top); + $this->assertIsArray($value); + $this->assertSame(\strlen($buf), $offset); + } + + public function testPointerChainOverMaximumDepthIsBounded(): void + { + // One more array makes 513 levels. + [$buf, $top] = $this->pointerChain(257); + + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $buf); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum depth" + ); + (new Decoder($handle, 0))->decode($top); + } + + public function testCyclicPointerThrows(): void + { + // A pointer to itself must throw a catchable InvalidDatabaseException + // rather than recursing until the stack is exhausted. + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\x20\x00"); // pointer (base 0) to offset 0, itself + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + (new Decoder($handle, 0))->decode(0); + } + + public function testOversizedMapIsBounded(): void + { + // A map entry decodes a key and a value, so a map of N entries costs 2N + // values on top of the root. A map that declares 32,768 entries reaches + // 65,537 values, one past the 65,536 limit, and is rejected on its + // header alone, before any entry is read. 0xfe is a map with size code + // 30, then the two size bytes for 32,768 - 285 = 32,483 (0x7ee3). + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\xfe\x7e\xe3"); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum number of values" + ); + (new Decoder($handle, 0))->decode(0); + } + // @phpstan-ignore-next-line private function checkDecoding(string $type, array $input, $expected, $name = null): void { From 753d6146b7e4f266d1cb2abde108ec4365d616cb Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 27 Aug 2026 13:50:41 +0000 Subject: [PATCH 02/13] Bound decoder payload to stop an amplification denial of service Pointers to a shared string or bytes value can amplify copied payload without exceeding the decoded-value limit. Bound each decode to 2 MiB of string and bytes payload. Charge each occurrence before reading it, including map keys and values reached through pointers. Reject scalar declarations above 16 bytes before reading their payload. Both checks throw InvalidDatabaseException. The budgets are passed by reference within one decode call. Update the shared fixtures and test amplification, payload boundaries, and metadata rejection through both the PHP reader and the extension. Assert each implementation's error text and the full boundary result. Probe the extension with a small over-limit record before larger DoS fixtures. Skip libraries older than 1.14.0 without the fix, accept working backports, and fail if 1.14.0 or later does not enforce the limit. Unexpected probe errors propagate. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 10 +- src/MaxMind/Db/Reader/Decoder.php | 85 +++++++++++---- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 39 +++++++ tests/MaxMind/Db/Test/ReaderTest.php | 105 +++++++++++++++++++ tests/data | 2 +- 5 files changed, 217 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18ed7cac..6bd7d572 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,16 @@ CHANGELOG * Bounded the resources that the pure PHP decoder spends on a single lookup. A crafted database could nest data-section pointers to shared targets so that - decoding one record cost exponential time and memory. The decoder now follows - the Reader Resource Limits section of the MaxMind DB specification. Each - lookup is limited to 65,536 values and 512 levels of nesting. + decoding one record cost exponential time and memory, or point many times at + one large value so that the decoder copied far more data than the file holds. + The decoder now follows the Reader Resource Limits section of the MaxMind DB + specification. Each lookup is limited to 65,536 values, 512 levels of + nesting, and 2 MiB of string and bytes payload. * Exceeding a limit throws an `InvalidDatabaseException`. * Opening a database whose metadata exceeds a limit throws the same exception. + * A scalar that declares more than 16 bytes, the width of the widest + fixed-width type, is rejected as invalid data. * The Windows build configuration now accepts either `libmaxminddb.lib` or `maxminddb.lib` when building the extension. The `lib` prefix was removed in libmaxminddb 1.6.0, but the libmaxminddb that PHP publishes for Windows diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 50388c1b..fdeb2304 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -59,6 +59,21 @@ class Decoder private const MAX_DEPTH = 512; private const MAX_VALUES = 1 << 16; + // The value limit alone does not stop payload amplification: an array of + // pointers to one large string or bytes value keeps the value count low + // while forcing the reader to copy the target once per pointer. This + // second, independent limit bounds the total string and bytes payload + // copied for one lookup to 2 MiB, matching libmaxminddb and the Go reader. + // No real record approaches it, and re-decoding a shared target charges its + // payload again, so the fan-out is bounded. + private const MAX_PAYLOAD_BYTES = 1 << 21; + + // A fixed-width scalar (a float, double, or integer) never needs more than + // 16 bytes (the width of a uint128). A larger declared size is either + // corrupt or an attempt to amplify the read of an oversized variable-length + // integer, so it is rejected before the bytes are materialized. + private const MAX_SCALAR_BYTES = 16; + /** * @param resource $fileStream */ @@ -81,19 +96,22 @@ public function __construct( public function decode(int $offset): array { // Bound the work per lookup so a crafted database cannot exhaust CPU or - // memory. $budget is passed by reference so the running count is shared - // across the recursion. It is call-local, so concurrent lookups do not - // share state. The root value is charged here; containers charge their - // children. + // memory. The two budgets are passed by reference so the running totals + // are shared across the recursion. $budget counts decoded values and + // stops the pointer fan-out; $byteBudget counts copied string and bytes + // payload and stops payload amplification. Both are call-local, so + // concurrent lookups do not share state. The root value is charged + // here; containers charge their children. $budget = self::MAX_VALUES - 1; + $byteBudget = self::MAX_PAYLOAD_BYTES; - return $this->decodeWithBudget($offset, 0, $budget); + return $this->decodeWithBudget($offset, 0, $budget, $byteBudget); } /** * @return array */ - private function decodeWithBudget(int $offset, int $depth, int &$budget): array + private function decodeWithBudget(int $offset, int $depth, int &$budget, int &$byteBudget): array { $ctrlByte = \ord(Util::read($this->fileStream, $offset, 1)); ++$offset; @@ -120,7 +138,7 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget): array // The value at the pointer's position was charged by its containing // array or map, so the target costs nothing more. Only the depth // grows. - [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget); + [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget, $byteBudget); return [$result, $offset]; } @@ -144,7 +162,7 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget): array [$size, $offset] = $this->sizeFromCtrlByte($ctrlByte, $offset); - return $this->decodeByType($type, $offset, $size, $depth, $budget); + return $this->decodeByType($type, $offset, $size, $depth, $budget, $byteBudget); } /** @@ -152,27 +170,54 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget): array * * @return array{0:mixed, 1:int} */ - private function decodeByType(int $type, int $offset, int $size, int $depth, int &$budget): array + private function decodeByType(int $type, int $offset, int $size, int $depth, int &$budget, int &$byteBudget): array { switch ($type) { case self::_MAP: - return $this->decodeMap($size, $offset, $depth, $budget); + return $this->decodeMap($size, $offset, $depth, $budget, $byteBudget); case self::_ARRAY: - return $this->decodeArray($size, $offset, $depth, $budget); + return $this->decodeArray($size, $offset, $depth, $budget, $byteBudget); case self::_BOOLEAN: return [$this->decodeBoolean($size), $offset]; + + case self::_BYTES: + case self::_UTF8_STRING: + // A string or bytes value is copied into a native string, so N + // pointers to one large value copy N times its length. Charge + // the payload against the byte budget wherever it is decoded, + // including inline inside a pointed-to container, so a shared + // target recharges each time it is followed. Compare before + // subtracting so an oversized declared size cannot drive the + // budget negative. A total exactly at the limit is allowed. + if ($size > $byteBudget) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum payload size" + ); + } + $byteBudget -= $size; + + return [Util::read($this->fileStream, $offset, $size), $offset + $size]; + } + + // The remaining valid types are fixed-width scalars, none wider than a + // uint128. A few other control bytes also reach here: the container + // (12) and end-marker (13) types, and any unknown extended type. The + // size guard below rejects one that declares an oversized size, and the + // default case at the end of the switch rejects the rest. Reject an + // oversized declared size before materializing the bytes, so an + // oversized variable-length integer cannot amplify the read. + if ($size > self::MAX_SCALAR_BYTES) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains bad data (unknown data type or corrupt data)" + ); } $newOffset = $offset + $size; $bytes = Util::read($this->fileStream, $offset, $size); switch ($type) { - case self::_BYTES: - case self::_UTF8_STRING: - return [$bytes, $newOffset]; - case self::_DOUBLE: $this->verifySize(8, $size); @@ -243,14 +288,14 @@ private function enterContainer( /** * @return array{0:array, 1:int} */ - private function decodeArray(int $size, int $offset, int $depth, int &$budget): array + private function decodeArray(int $size, int $offset, int $depth, int &$budget, int &$byteBudget): array { $this->enterContainer($size, $depth, $budget); $array = []; for ($i = 0; $i < $size; ++$i) { - [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget, $byteBudget); $array[] = $value; } @@ -328,7 +373,7 @@ private function decodeInt32(string $bytes, int $size): int /** * @return array{0:array, 1:int} */ - private function decodeMap(int $size, int $offset, int $depth, int &$budget): array + private function decodeMap(int $size, int $offset, int $depth, int &$budget, int &$byteBudget): array { // A map entry decodes a key and a value, so it costs two values. $this->enterContainer($size, $depth, $budget, 2); @@ -336,8 +381,8 @@ private function decodeMap(int $size, int $offset, int $depth, int &$budget): ar $map = []; for ($i = 0; $i < $size; ++$i) { - [$key, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget); - [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget); + [$key, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget, $byteBudget); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget, $byteBudget); $map[$key] = $value; } diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index 2c1b4820..d19ee81c 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -610,6 +610,45 @@ public function testOversizedMapIsBounded(): void (new Decoder($handle, 0))->decode(0); } + public function testOversizedStringIsRejectedBeforeRead(): void + { + // A UTF-8 string that declares 2,097,153 bytes, one past the 2 MiB + // payload limit, with no payload behind it. The payload check must + // reject it before the read, so the error is the payload limit and not + // the short read that would otherwise follow. 0x5f is a string with + // size code 31, then three size bytes for + // 2,097,153 - 65,821 = 2,031,332 (0x1eff64). + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\x5f\x1e\xff\x64"); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum payload size" + ); + (new Decoder($handle, 0))->decode(0); + } + + public function testOversizedVariableLengthIntegerIsBounded(): void + { + // A fixed-width scalar never needs more than 16 bytes. A uint32 (type 6) + // that declares a 17-byte payload is an oversized variable-length + // integer: a reader that copies the declared bytes before range-checking + // copies an attacker-controlled length. The fixture is the header + // alone: 0xd1, a uint32 with the size encoded directly as 17. No + // payload follows, so the decoder must reject the size before it + // reads. A read would fail with the short-read error instead. + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\xd1"); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + "The MaxMind DB file's data section contains bad data (unknown data type or corrupt data)" + ); + (new Decoder($handle, 0))->decode(0); + } + // @phpstan-ignore-next-line private function checkDecoding(string $type, array $input, $expected, $name = null): void { diff --git a/tests/MaxMind/Db/Test/ReaderTest.php b/tests/MaxMind/Db/Test/ReaderTest.php index ab24b40c..01f7f836 100644 --- a/tests/MaxMind/Db/Test/ReaderTest.php +++ b/tests/MaxMind/Db/Test/ReaderTest.php @@ -15,6 +15,8 @@ */ class ReaderTest extends TestCase { + private const EXTENSION_LIMIT_MESSAGE = 'exceeds the configured resource limits'; + public function testReader(): void { foreach ([24, 28, 32] as $recordSize) { @@ -291,6 +293,109 @@ public function testBrokenDataPointer(): void $reader->get('1.1.1.16'); } + private function requireDecoderLimits(): void + { + if (!\extension_loaded('maxminddb')) { + return; + } + + // Probe with 2 MiB plus one byte before fixtures that could exhaust an + // unpatched library. Unexpected errors must still fail the test. + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-payload-limit-over.mmdb'); + + try { + $reader->get('1.1.1.1'); + } catch (InvalidDatabaseException $e) { + if (!str_contains($e->getMessage(), self::EXTENSION_LIMIT_MESSAGE)) { + throw $e; + } + + return; + } finally { + $reader->close(); + } + + // libmaxminddb 1.14.0 introduced these limits. Older versions may + // carry a backport, which the probe above also accepts. + if (\defined('MaxMind\Db\Reader::MMDB_LIB_VERSION') + && version_compare(Reader::MMDB_LIB_VERSION, '1.14.0', '>=')) { + $this->fail('the linked libmaxminddb did not enforce its decoder resource limits'); + } + $this->markTestSkipped('linked libmaxminddb predates the decoder resource limits'); + } + + private function expectDecoderLimit(string $message): void + { + $this->requireDecoderLimits(); + $this->expectException(InvalidDatabaseException::class); + if (\extension_loaded('maxminddb')) { + $message = self::EXTENSION_LIMIT_MESSAGE; + } + $this->expectExceptionMessage($message); + } + + public function testPayloadAmplificationDosIsRejected(): void + { + // An array of pointers to one large value. The value count stays low, + // but a reader that copies each target materializes the value once per + // pointer. The produced-payload byte budget rejects it. + $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-payload-amplification-dos.mmdb'); + $reader->get('1.1.1.1'); + } + + public function testStringPayloadAmplificationDosIsRejected(): void + { + // The string variant, so the UTF-8 path is charged as well as bytes. + $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-payload-amplification-dos-string.mmdb'); + $reader->get('1.1.1.1'); + } + + public function testWorstCasePayloadAmplificationDosIsRejected(): void + { + // The worst case sits exactly at the value limit: 65,535 pointers to + // one 64 KiB value. Only the payload budget rejects it. + $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb'); + $reader->get('1.1.1.1'); + } + + public function testPayloadAtLimitDecodes(): void + { + // A record whose produced payload is exactly at the byte budget must + // still decode, so the limit does not reject legitimate data. + $expected = array_fill(0, 32, str_repeat("\x00", 65535)); + $expected[] = str_repeat("\x00", 32); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-payload-limit.mmdb'); + $this->assertSame($expected, $reader->get('1.1.1.1')); + $reader->close(); + } + + public function testPayloadOverLimitIsRejected(): void + { + // One byte past the limit must be rejected. + $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-payload-limit-over.mmdb'); + $reader->get('1.1.1.1'); + } + + public function testMetadataPayloadLimitIsRejectedOnOpen(): void + { + // Metadata is decoded while opening the database, so the same bound + // must guard that path. + $this->requireDecoderLimits(); + $this->expectException(InvalidDatabaseException::class); + if (\extension_loaded('maxminddb')) { + // libmaxminddb reports metadata limits as invalid metadata, and + // the extension uses its standard database-open error. + $this->expectExceptionMessage('Error opening database file'); + } else { + $this->expectExceptionMessage("The MaxMind DB file's data section exceeds the maximum payload size"); + } + new Reader('tests/data/test-data/MaxMind-DB-test-metadata-payload-limit.mmdb'); + } + public function testMissingDatabase(): void { $this->expectException(\InvalidArgumentException::class); diff --git a/tests/data b/tests/data index b2a3df13..363086b7 160000 --- a/tests/data +++ b/tests/data @@ -1 +1 @@ -Subproject commit b2a3df13c0e274d7a2dca3d5415465a3a9670e23 +Subproject commit 363086b7d90650100e91f954937794c6a090c2a0 From 9b474f578d180597aeb4a829fb420216eef68dc7 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 5 Sep 2026 01:06:35 +0000 Subject: [PATCH 03/13] Exercise the shared fan-out and value-limit fixtures Run the shared IPv4 and IPv6 pointer fan-out fixtures through both reader implementations and assert InvalidDatabaseException with the expected limit message. Assert all 65,535 array elements at the 65,536-value boundary, including on a second lookup with the same reader. Also accept the depth-15 pointer fan-out with 65,535 values and reject the fixture one value over the limit. Co-Authored-By: Claude Fable 5.1 --- tests/MaxMind/Db/Test/ReaderTest.php | 42 ++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/MaxMind/Db/Test/ReaderTest.php b/tests/MaxMind/Db/Test/ReaderTest.php index 01f7f836..68d2dba2 100644 --- a/tests/MaxMind/Db/Test/ReaderTest.php +++ b/tests/MaxMind/Db/Test/ReaderTest.php @@ -396,6 +396,48 @@ public function testMetadataPayloadLimitIsRejectedOnOpen(): void new Reader('tests/data/test-data/MaxMind-DB-test-metadata-payload-limit.mmdb'); } + public function testPointerFanOutDosIsRejected(): void + { + // Nested arrays of pointers to the level below: 2**40 leaf decodes + // from 451 bytes. The value budget rejects it. + $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum number of values"); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-pointer-decoder-dos.mmdb'); + $reader->get('1.1.1.1'); + } + + public function testPointerFanOutDosIpv6IsRejected(): void + { + // The same fan-out in a conventional IPv6 database. + $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum number of values"); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb'); + $reader->get('::1'); + } + + public function testValueCountAtLimitDecodes(): void + { + // Exactly 65,536 values must decode, and so must a second lookup on + // the same reader, because the budget belongs to one call. + $expected = array_fill(0, 65535, 0); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-value-limit.mmdb'); + $this->assertSame($expected, $reader->get('1.1.1.1')); + $this->assertSame($expected, $reader->get('1.1.1.1')); + $reader->close(); + + // 65,535 values reached through a depth-15 pointer fan-out. Under the + // flat rule a pointer costs nothing beyond the value it resolves to. + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb'); + $this->assertIsArray($reader->get('1.1.1.1')); + $reader->close(); + } + + public function testValueCountOverLimitIsRejected(): void + { + // One value past the limit must be rejected. + $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum number of values"); + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-value-limit-over.mmdb'); + $reader->get('1.1.1.1'); + } + public function testMissingDatabase(): void { $this->expectException(\InvalidArgumentException::class); From cef34e72f1f07adc794eb1989cce5f9907a7fded Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 5 Sep 2026 01:06:35 +0000 Subject: [PATCH 04/13] Bump libmaxminddb to pick up the decoder resource limits Update the bundled library from 1.13.3 to 1.14.0 and match its reported version. The new library bounds MMDB_get_entry_data_list() to 65,536 values and 2 MiB of payload per call. The extension already converts MMDB_DECODER_LIMIT_ERROR into InvalidDatabaseException. The shared ReaderTest limit checks now run against bundled builds instead of skipping. A failed probe on 1.14.0 or later fails the tests automatically. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 3 +++ ext/bundled-include/maxminddb_config.h | 2 +- ext/libmaxminddb | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bd7d572..34c709f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ CHANGELOG exception. * A scalar that declares more than 16 bytes, the width of the widest fixed-width type, is rejected as invalid data. +* The bundled libmaxminddb used by `--with-maxminddb-bundled` builds of the + extension now applies the same decoder limits. The extension throws an + `InvalidDatabaseException` when a lookup exceeds them. * The Windows build configuration now accepts either `libmaxminddb.lib` or `maxminddb.lib` when building the extension. The `lib` prefix was removed in libmaxminddb 1.6.0, but the libmaxminddb that PHP publishes for Windows diff --git a/ext/bundled-include/maxminddb_config.h b/ext/bundled-include/maxminddb_config.h index c578a4eb..0c12d19a 100644 --- a/ext/bundled-include/maxminddb_config.h +++ b/ext/bundled-include/maxminddb_config.h @@ -31,7 +31,7 @@ * every build, so a stale value here fails CI rather than shipping. */ #ifndef PACKAGE_VERSION -#define PACKAGE_VERSION "1.13.3" +#define PACKAGE_VERSION "1.14.0" #endif #endif /* MAXMINDDB_CONFIG_H */ diff --git a/ext/libmaxminddb b/ext/libmaxminddb index 09a0540f..0077fd76 160000 --- a/ext/libmaxminddb +++ b/ext/libmaxminddb @@ -1 +1 @@ -Subproject commit 09a0540fea89a16e5c6a9e21e93ee9aece6639e3 +Subproject commit 0077fd76d00a1656b9cb3028d467736504794f41 From 91cf91b9176892f48a94d92576b8227264592261 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 5 Sep 2026 02:15:37 +0000 Subject: [PATCH 05/13] Keep the decode budgets on the decoder instance The value and payload budgets were passed by reference through every recursive decode call. On GeoLite2-City lookups that cost about 3.5% per lookup against main, most of the total cost of the resource limits. Keep both budgets as decoder properties instead and reset them at the start of each decode() call, so every call still starts with the full allowance. No other lookup can observe them mid-decode: PHP runs one request per thread, and the decoder never yields while it decodes. The same GeoLite2-City benchmark then runs within about 1% of main. Co-Authored-By: Claude Fable 5.1 --- src/MaxMind/Db/Reader/Decoder.php | 72 +++++++++++++++++++------------ 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index fdeb2304..9fb74b68 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -30,6 +30,21 @@ class Decoder */ private $switchByteOrder; + /** + * Remaining decoded-value allowance for the current decode() call. + * + * @var int + */ + private $budget = 0; + + /** + * Remaining string and bytes payload allowance for the current decode() + * call. + * + * @var int + */ + private $byteBudget = 0; + private const _EXTENDED = 0; private const _POINTER = 1; private const _UTF8_STRING = 2; @@ -96,22 +111,24 @@ public function __construct( public function decode(int $offset): array { // Bound the work per lookup so a crafted database cannot exhaust CPU or - // memory. The two budgets are passed by reference so the running totals - // are shared across the recursion. $budget counts decoded values and - // stops the pointer fan-out; $byteBudget counts copied string and bytes - // payload and stops payload amplification. Both are call-local, so - // concurrent lookups do not share state. The root value is charged - // here; containers charge their children. - $budget = self::MAX_VALUES - 1; - $byteBudget = self::MAX_PAYLOAD_BYTES; - - return $this->decodeWithBudget($offset, 0, $budget, $byteBudget); + // memory. $budget counts decoded values and stops the pointer fan-out; + // $byteBudget counts copied string and bytes payload and stops payload + // amplification. Both live on the decoder and are reset here, so every + // call starts with the full allowance. Passing them by reference + // through each recursive call instead costs a few percent per lookup. + // No other lookup can observe them mid-decode: PHP runs one request + // per thread, and the decoder never yields while it decodes. The root + // value is charged here; containers charge their children. + $this->budget = self::MAX_VALUES - 1; + $this->byteBudget = self::MAX_PAYLOAD_BYTES; + + return $this->decodeWithBudget($offset, 0); } /** * @return array */ - private function decodeWithBudget(int $offset, int $depth, int &$budget, int &$byteBudget): array + private function decodeWithBudget(int $offset, int $depth): array { $ctrlByte = \ord(Util::read($this->fileStream, $offset, 1)); ++$offset; @@ -138,7 +155,7 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget, int &$b // The value at the pointer's position was charged by its containing // array or map, so the target costs nothing more. Only the depth // grows. - [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget, $byteBudget); + [$result] = $this->decodeWithBudget($pointer, $depth + 1); return [$result, $offset]; } @@ -162,7 +179,7 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget, int &$b [$size, $offset] = $this->sizeFromCtrlByte($ctrlByte, $offset); - return $this->decodeByType($type, $offset, $size, $depth, $budget, $byteBudget); + return $this->decodeByType($type, $offset, $size, $depth); } /** @@ -170,14 +187,14 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget, int &$b * * @return array{0:mixed, 1:int} */ - private function decodeByType(int $type, int $offset, int $size, int $depth, int &$budget, int &$byteBudget): array + private function decodeByType(int $type, int $offset, int $size, int $depth): array { switch ($type) { case self::_MAP: - return $this->decodeMap($size, $offset, $depth, $budget, $byteBudget); + return $this->decodeMap($size, $offset, $depth); case self::_ARRAY: - return $this->decodeArray($size, $offset, $depth, $budget, $byteBudget); + return $this->decodeArray($size, $offset, $depth); case self::_BOOLEAN: return [$this->decodeBoolean($size), $offset]; @@ -191,12 +208,12 @@ private function decodeByType(int $type, int $offset, int $size, int $depth, int // target recharges each time it is followed. Compare before // subtracting so an oversized declared size cannot drive the // budget negative. A total exactly at the limit is allowed. - if ($size > $byteBudget) { + if ($size > $this->byteBudget) { throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum payload size" ); } - $byteBudget -= $size; + $this->byteBudget -= $size; return [Util::read($this->fileStream, $offset, $size), $offset + $size]; } @@ -266,7 +283,6 @@ private function verifySize(int $expected, int $actual): void private function enterContainer( int $size, int $depth, - int &$budget, int $valuesPerEntry = 1 ): void { if ($depth >= self::MAX_DEPTH) { @@ -277,25 +293,25 @@ private function enterContainer( // Compare with a division rather than multiplying the declared size, so // an oversized declaration cannot overflow the integer on 32-bit builds // before the budget check runs. - if ($size > intdiv($budget, $valuesPerEntry)) { + if ($size > intdiv($this->budget, $valuesPerEntry)) { throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum number of values" ); } - $budget -= $size * $valuesPerEntry; + $this->budget -= $size * $valuesPerEntry; } /** * @return array{0:array, 1:int} */ - private function decodeArray(int $size, int $offset, int $depth, int &$budget, int &$byteBudget): array + private function decodeArray(int $size, int $offset, int $depth): array { - $this->enterContainer($size, $depth, $budget); + $this->enterContainer($size, $depth); $array = []; for ($i = 0; $i < $size; ++$i) { - [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget, $byteBudget); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1); $array[] = $value; } @@ -373,16 +389,16 @@ private function decodeInt32(string $bytes, int $size): int /** * @return array{0:array, 1:int} */ - private function decodeMap(int $size, int $offset, int $depth, int &$budget, int &$byteBudget): array + private function decodeMap(int $size, int $offset, int $depth): array { // A map entry decodes a key and a value, so it costs two values. - $this->enterContainer($size, $depth, $budget, 2); + $this->enterContainer($size, $depth, 2); $map = []; for ($i = 0; $i < $size; ++$i) { - [$key, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget, $byteBudget); - [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1, $budget, $byteBudget); + [$key, $offset] = $this->decodeWithBudget($offset, $depth + 1); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1); $map[$key] = $value; } From 5658655d7145050b2a12de53d21ba0f93cc29220 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 5 Sep 2026 02:20:35 +0000 Subject: [PATCH 06/13] Reduce stream calls in the pure PHP reader Every read seeked first and then called ftell() to check the length. fseek() discards PHP's read buffer, so each small read of a control byte, a size, or a scalar became its own system call. Most of a record is laid out in order, so nearly all of those seeks landed where the stream already was. Give the decoder its own read method that tracks the stream position and seeks only when a read does not continue from the previous one, which is a pointer follow or the first read of a call. The position is reset at the start of each decode() call because the search tree walk moves the stream between calls. Util::read, which the tree walk still uses, checks the length with strlen() instead of ftell(); string length is stored, so the comment claiming ftell() was faster no longer holds. On GeoLite2-City, 60,000 lookups per process over five alternating runs, main takes 172.5 to 173.4 us per lookup and this branch 105.2 to 106.0, about 40% faster, with identical results. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 3 ++ src/MaxMind/Db/Reader/Decoder.php | 57 +++++++++++++++++++++++++++---- src/MaxMind/Db/Reader/Util.php | 6 ++-- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34c709f1..1cebbbd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ CHANGELOG * The bundled libmaxminddb used by `--with-maxminddb-bundled` builds of the extension now applies the same decoder limits. The extension throws an `InvalidDatabaseException` when a lookup exceeds them. +* The pure PHP reader is about 40% faster on City lookups. It no longer seeks + before a read that continues where the last one ended, and it checks read + lengths with `strlen()` instead of `ftell()`. * The Windows build configuration now accepts either `libmaxminddb.lib` or `maxminddb.lib` when building the extension. The `lib` prefix was removed in libmaxminddb 1.6.0, but the libmaxminddb that PHP publishes for Windows diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 9fb74b68..9f8b2d7d 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -45,6 +45,14 @@ class Decoder */ private $byteBudget = 0; + /** + * Stream position after the last read of the current decode() call, or -1 + * when unknown. + * + * @var int + */ + private $position = -1; + private const _EXTENDED = 0; private const _POINTER = 1; private const _UTF8_STRING = 2; @@ -122,6 +130,10 @@ public function decode(int $offset): array $this->budget = self::MAX_VALUES - 1; $this->byteBudget = self::MAX_PAYLOAD_BYTES; + // Other readers of the stream, such as the search tree walk, may have + // moved it since the last call. + $this->position = -1; + return $this->decodeWithBudget($offset, 0); } @@ -130,7 +142,7 @@ public function decode(int $offset): array */ private function decodeWithBudget(int $offset, int $depth): array { - $ctrlByte = \ord(Util::read($this->fileStream, $offset, 1)); + $ctrlByte = \ord($this->read($offset, 1)); ++$offset; $type = $ctrlByte >> 5; @@ -161,7 +173,7 @@ private function decodeWithBudget(int $offset, int $depth): array } if ($type === self::_EXTENDED) { - $nextByte = \ord(Util::read($this->fileStream, $offset, 1)); + $nextByte = \ord($this->read($offset, 1)); $type = $nextByte + 7; @@ -215,7 +227,7 @@ private function decodeByType(int $type, int $offset, int $size, int $depth): ar } $this->byteBudget -= $size; - return [Util::read($this->fileStream, $offset, $size), $offset + $size]; + return [$this->read($offset, $size), $offset + $size]; } // The remaining valid types are fixed-width scalars, none wider than a @@ -232,7 +244,7 @@ private function decodeByType(int $type, int $offset, int $size, int $depth): ar } $newOffset = $offset + $size; - $bytes = Util::read($this->fileStream, $offset, $size); + $bytes = $this->read($offset, $size); switch ($type) { case self::_DOUBLE: @@ -261,6 +273,39 @@ private function decodeByType(int $type, int $offset, int $size, int $depth): ar } } + /** + * Reads from the stream, seeking only when the read does not continue where + * the previous one ended. Most values in a record are laid out in order, and + * fseek() discards PHP's read buffer, so seeking before every read turned + * each small read into a system call. Skipping the seek makes a City lookup + * about 40% faster. + * + * @param int<0, max> $numberOfBytes + */ + private function read(int $offset, int $numberOfBytes): string + { + if ($numberOfBytes === 0) { + return ''; + } + + $stream = $this->fileStream; + if ($offset !== $this->position && fseek($stream, $offset) !== 0) { + $this->position = -1; + + throw new InvalidDatabaseException('The MaxMind DB file contains bad data'); + } + + $value = fread($stream, $numberOfBytes); + if ($value === false || \strlen($value) !== $numberOfBytes) { + $this->position = -1; + + throw new InvalidDatabaseException('The MaxMind DB file contains bad data'); + } + $this->position = $offset + $numberOfBytes; + + return $value; + } + private function verifySize(int $expected, int $actual): void { if ($expected !== $actual) { @@ -412,7 +457,7 @@ private function decodePointer(int $ctrlByte, int $offset): array { $pointerSize = (($ctrlByte >> 3) & 0x3) + 1; - $buffer = Util::read($this->fileStream, $offset, $pointerSize); + $buffer = $this->read($offset, $pointerSize); $offset += $pointerSize; switch ($pointerSize) { @@ -538,7 +583,7 @@ private function sizeFromCtrlByte(int $ctrlByte, int $offset): array } $bytesToRead = $size - 28; - $bytes = Util::read($this->fileStream, $offset, $bytesToRead); + $bytes = $this->read($offset, $bytesToRead); if ($size === 29) { $size = 29 + \ord($bytes); diff --git a/src/MaxMind/Db/Reader/Util.php b/src/MaxMind/Db/Reader/Util.php index c2c3212d..c5485ea7 100644 --- a/src/MaxMind/Db/Reader/Util.php +++ b/src/MaxMind/Db/Reader/Util.php @@ -18,10 +18,8 @@ public static function read($stream, int $offset, int $numberOfBytes): string if (fseek($stream, $offset) === 0) { $value = fread($stream, $numberOfBytes); - // We check that the number of bytes read is equal to the number - // asked for. We use ftell as getting the length of $value is - // much slower. - if ($value !== false && ftell($stream) - $offset === $numberOfBytes) { + // Check that the number of bytes read is the number asked for. + if ($value !== false && \strlen($value) === $numberOfBytes) { return $value; } } From 3241a02c7fc1d1e434869e92dafd8e38de368cca Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 9 Sep 2026 22:23:54 +0000 Subject: [PATCH 07/13] Reject pointers that directly target another pointer Enforce the format rule before following a second pointer. This removes the chain multiplier from repeated pointer decoding without adding stream reads. Keep pointers inside target containers valid, with the depth limit still bounding container cycles. --- src/MaxMind/Db/Reader/Decoder.php | 10 +++++++-- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 22 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 9f8b2d7d..837f18fc 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -140,7 +140,7 @@ public function decode(int $offset): array /** * @return array */ - private function decodeWithBudget(int $offset, int $depth): array + private function decodeWithBudget(int $offset, int $depth, bool $allowPointer = true): array { $ctrlByte = \ord($this->read($offset, 1)); ++$offset; @@ -151,6 +151,12 @@ private function decodeWithBudget(int $offset, int $depth): array // use the size to determine the length of the pointer and then follow // it. if ($type === self::_POINTER) { + if (!$allowPointer) { + throw new InvalidDatabaseException( + 'The MaxMind DB file contains a pointer to another pointer' + ); + } + [$pointer, $offset] = $this->decodePointer($ctrlByte, $offset); // for unit testing @@ -167,7 +173,7 @@ private function decodeWithBudget(int $offset, int $depth): array // The value at the pointer's position was charged by its containing // array or map, so the target costs nothing more. Only the depth // grows. - [$result] = $this->decodeWithBudget($pointer, $depth + 1); + [$result] = $this->decodeWithBudget($pointer, $depth + 1, false); return [$result, $offset]; } diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index d19ee81c..41c67bc0 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -592,6 +592,28 @@ public function testCyclicPointerThrows(): void (new Decoder($handle, 0))->decode(0); } + public function testPointerToPointerIsRejected(): void + { + $handle = fopen('php://memory', 'rwb'); + // Two pointers followed by a scalar. Direct pointer chains are invalid. + fwrite($handle, "\x20\x02\x20\x04\xa0"); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage('contains a pointer to another pointer'); + (new Decoder($handle, 0))->decode(0); + } + + public function testPointerThroughContainerCycleIsBounded(): void + { + $handle = fopen('php://memory', 'rwb'); + // An array containing a pointer back to itself still needs a depth bound. + fwrite($handle, "\x01\x04\x20\x00"); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage('exceeds the maximum depth'); + (new Decoder($handle, 0))->decode(0); + } + public function testOversizedMapIsBounded(): void { // A map entry decodes a key and a value, so a map of N entries costs 2N From bca4e14cbd0612940726dbef298f206c89fc8aa6 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 9 Sep 2026 22:29:08 +0000 Subject: [PATCH 08/13] Reject nested lookups on the same reader A PHP stream wrapper can call back into the reader during a read. Reject the nested lookup before it moves the shared stream or resets the decoder budgets, and clear the guard when the outer lookup returns or throws. Cover callbacks that propagate or catch the rejection, and verify that the same reader can perform another lookup afterward. --- src/MaxMind/Db/Reader.php | 30 +++++-- src/MaxMind/Db/Reader/Decoder.php | 6 +- .../MaxMind/Db/Test/Reader/ReentrancyTest.php | 86 +++++++++++++++++++ .../Db/Test/Reader/ReentrantStream.php | 78 +++++++++++++++++ 4 files changed, 191 insertions(+), 9 deletions(-) create mode 100644 tests/MaxMind/Db/Test/Reader/ReentrancyTest.php create mode 100644 tests/MaxMind/Db/Test/Reader/ReentrantStream.php diff --git a/src/MaxMind/Db/Reader.php b/src/MaxMind/Db/Reader.php index 0524ff16..923dc02f 100644 --- a/src/MaxMind/Db/Reader.php +++ b/src/MaxMind/Db/Reader.php @@ -45,6 +45,11 @@ class Reader */ private $fileHandle; + /** + * @var bool + */ + private $lookupInProgress = false; + /** * @var int */ @@ -118,7 +123,7 @@ public function __construct(string $database) * * @param string $ipAddress the IP address to look up * - * @throws \BadMethodCallException if this method is called on a closed database + * @throws \BadMethodCallException if the database is closed or another lookup is in progress * @throws \InvalidArgumentException if something other than a single IP address is passed to the method * @throws InvalidDatabaseException * if the database is invalid or there is an error reading @@ -143,7 +148,7 @@ public function get(string $ipAddress) * * @param string $ipAddress the IP address to look up * - * @throws \BadMethodCallException if this method is called on a closed database + * @throws \BadMethodCallException if the database is closed or another lookup is in progress * @throws \InvalidArgumentException if something other than a single IP address is passed to the method * @throws InvalidDatabaseException * if the database is invalid or there is an error reading @@ -166,12 +171,25 @@ public function getWithPrefixLen(string $ipAddress): array ); } - [$pointer, $prefixLen] = $this->findAddressInTree($ipAddress); - if ($pointer === 0) { - return [null, $prefixLen]; + if ($this->lookupInProgress) { + throw new \BadMethodCallException( + 'A lookup is already in progress on this reader. Use a separate reader for nested lookups.' + ); } + // A stream wrapper can call back into this reader during a read. + // Reject nested lookups before they can move the shared stream. + $this->lookupInProgress = true; + + try { + [$pointer, $prefixLen] = $this->findAddressInTree($ipAddress); + if ($pointer === 0) { + return [null, $prefixLen]; + } - return [$this->resolveDataPointer($pointer), $prefixLen]; + return [$this->resolveDataPointer($pointer), $prefixLen]; + } finally { + $this->lookupInProgress = false; + } } /** diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 837f18fc..84c691f6 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -124,9 +124,9 @@ public function decode(int $offset): array // amplification. Both live on the decoder and are reset here, so every // call starts with the full allowance. Passing them by reference // through each recursive call instead costs a few percent per lookup. - // No other lookup can observe them mid-decode: PHP runs one request - // per thread, and the decoder never yields while it decodes. The root - // value is charged here; containers charge their children. + // Reader prevents nested lookups from moving its stream during a + // decode. The root value is charged here; containers charge their + // children. $this->budget = self::MAX_VALUES - 1; $this->byteBudget = self::MAX_PAYLOAD_BYTES; diff --git a/tests/MaxMind/Db/Test/Reader/ReentrancyTest.php b/tests/MaxMind/Db/Test/Reader/ReentrancyTest.php new file mode 100644 index 00000000..568b3583 --- /dev/null +++ b/tests/MaxMind/Db/Test/Reader/ReentrancyTest.php @@ -0,0 +1,86 @@ +checkReentrantLookup(false); + } + + public function testCaughtReentrantLookupDoesNotCorruptRecord(): void + { + $this->checkReentrantLookup(true); + } + + private function checkReentrantLookup(bool $catchRejection): void + { + if (\extension_loaded('maxminddb')) { + $this->markTestSkipped('the extension does not open PHP stream wrappers'); + } + + $file = 'tests/data/test-data/GeoIP2-City-Test.mmdb'; + $data = file_get_contents($file); + $this->assertIsString($data); + ReentrantStream::$data = $data; + $plainReader = new Reader($file); + $expected = $plainReader->get('81.2.69.160'); + $plainReader->close(); + + $this->assertTrue(stream_wrapper_register('mmdbreentrant', ReentrantStream::class)); + $reader = null; + + try { + $reader = new Reader('mmdbreentrant://db'); + $dataStart = $reader->metadata()->searchTreeSize + 16; + $rejected = false; + ReentrantStream::$onRead = function (int $offset) use ($reader, $catchRejection, $dataStart, &$rejected): void { + if ($offset < $dataStart) { + return; + } + // Reenter during a read from the data section. Disable the callback + // before the nested lookup so the test cannot recurse forever. + ReentrantStream::$onRead = null; + + try { + $reader->get('216.160.83.56'); + } catch (\BadMethodCallException $e) { + $this->assertStringContainsString('A lookup is already in progress', $e->getMessage()); + $rejected = true; + if (!$catchRejection) { + throw $e; + } + } + }; + + try { + $actual = $reader->getWithPrefixLen('81.2.69.160'); + $this->assertTrue($catchRejection, 'the nested lookup must throw'); + $this->assertSame($expected, $actual[0]); + } catch (\BadMethodCallException $e) { + $this->assertFalse($catchRejection); + $this->assertStringContainsString('A lookup is already in progress', $e->getMessage()); + } + $this->assertTrue($rejected, 'the stream callback must attempt a nested lookup'); + $this->assertSame($expected, $reader->get('81.2.69.160')); + } finally { + ReentrantStream::$onRead = null; + ReentrantStream::$data = ''; + if ($reader !== null) { + $reader->close(); + } + $this->assertTrue(stream_wrapper_unregister('mmdbreentrant')); + } + } +} diff --git a/tests/MaxMind/Db/Test/Reader/ReentrantStream.php b/tests/MaxMind/Db/Test/Reader/ReentrantStream.php new file mode 100644 index 00000000..73afef70 --- /dev/null +++ b/tests/MaxMind/Db/Test/Reader/ReentrantStream.php @@ -0,0 +1,78 @@ +position; + $bytes = substr(self::$data, $this->position, $count); + $this->position += \strlen($bytes); + $callback = self::$onRead; + if ($callback !== null) { + $callback($offset); + } + + return $bytes; + } + + public function stream_seek(int $offset, int $whence): bool + { + if ($whence === \SEEK_SET) { + $this->position = $offset; + } elseif ($whence === \SEEK_CUR) { + $this->position += $offset; + } else { + $this->position = \strlen(self::$data) + $offset; + } + + return true; + } + + public function stream_tell(): int + { + return $this->position; + } + + public function stream_eof(): bool + { + return $this->position >= \strlen(self::$data); + } + + /** @return array */ + public function stream_stat(): array + { + return ['size' => \strlen(self::$data), 'mode' => 0100444]; + } + + /** @return array */ + public function url_stat(string $path, int $flags): array + { + return $this->stream_stat(); + } +} From b5759a9452a9cb8915832d6fe7ca45895b087557 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 9 Sep 2026 22:29:47 +0000 Subject: [PATCH 09/13] Test a string exactly one byte over the payload limit Correct the encoded size to 2,097,153 bytes. The previous header declared 2,097,281 bytes, so it did not test the boundary described by the comment. --- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index 41c67bc0..b6d9f212 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -639,9 +639,9 @@ public function testOversizedStringIsRejectedBeforeRead(): void // reject it before the read, so the error is the payload limit and not // the short read that would otherwise follow. 0x5f is a string with // size code 31, then three size bytes for - // 2,097,153 - 65,821 = 2,031,332 (0x1eff64). + // 2,097,153 - 65,821 = 2,031,332 (0x1efee4). $handle = fopen('php://memory', 'rwb'); - fwrite($handle, "\x5f\x1e\xff\x64"); + fwrite($handle, "\x5f\x1e\xfe\xe4"); fseek($handle, 0); $this->expectException(InvalidDatabaseException::class); From 8451e5b48f25ff9d76a8b73ab58e0ecf293e144e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 9 Sep 2026 22:30:10 +0000 Subject: [PATCH 10/13] Test decoder state reset between calls Reuse the same decoder after a partially charged value or payload budget fails, then decode a record at the corresponding limit. Also move the stream between calls to verify that the cached position is discarded. --- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 58 ++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index b6d9f212..bd8799e7 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -671,6 +671,64 @@ public function testOversizedVariableLengthIntegerIsBounded(): void (new Decoder($handle, 0))->decode(0); } + public function testValueBudgetResetsAfterFailedDecode(): void + { + // Charge two children, then encounter an array that exceeds the limit. + $invalid = "\x02\x04\xa0\x1e\x04\xfe\xe3"; + // The next record uses all 65,536 values, including the root. + $valid = "\x1e\x04\xfe\xe2" . str_repeat("\xa0", 65535); + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $invalid . $valid); + $decoder = new Decoder($handle); + + try { + $decoder->decode(0); + $this->fail('the first record must exceed the value limit'); + } catch (InvalidDatabaseException $e) { + $this->assertStringContainsString('exceeds the maximum number of values', $e->getMessage()); + } + + $this->assertSame( + [array_fill(0, 65535, 0), \strlen($invalid . $valid)], + $decoder->decode(\strlen($invalid)) + ); + } + + public function testPayloadBudgetResetsAfterFailedDecode(): void + { + // A one-byte string followed by a 2 MiB string exceeds the payload limit. + $invalid = "\x02\x04\x41x\x5f\x1e\xfe\xe3"; + $payload = str_repeat('x', 1 << 21); + $valid = "\x5f\x1e\xfe\xe3" . $payload; + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, $invalid . $valid); + $decoder = new Decoder($handle); + + try { + $decoder->decode(0); + $this->fail('the first record must exceed the payload limit'); + } catch (InvalidDatabaseException $e) { + $this->assertStringContainsString('exceeds the maximum payload size', $e->getMessage()); + } + + $this->assertSame( + [$payload, \strlen($invalid . $valid)], + $decoder->decode(\strlen($invalid)) + ); + } + + public function testDecodeAfterExternalSeek(): void + { + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\xa1\x01\xa1\x02"); + $decoder = new Decoder($handle); + $this->assertSame([1, 2], $decoder->decode(0)); + + $this->assertSame(0, fseek($handle, 0)); + // Offset 2 matches the cached position, but the stream has moved. + $this->assertSame([2, 4], $decoder->decode(2)); + } + // @phpstan-ignore-next-line private function checkDecoding(string $type, array $input, $expected, $name = null): void { From 109614de282ad4eb6c8083c84c466f64f840eb2f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 9 Sep 2026 22:30:40 +0000 Subject: [PATCH 11/13] Use section-neutral decoder limit errors The decoder applies the same limits to metadata and lookup records. Remove the data-section label so metadata failures describe the correct scope without adding context to the decode path. --- src/MaxMind/Db/Reader/Decoder.php | 8 ++++---- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 14 +++++++------- tests/MaxMind/Db/Test/ReaderTest.php | 16 ++++++++-------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 84c691f6..f4404ef8 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -166,7 +166,7 @@ private function decodeWithBudget(int $offset, int $depth, bool $allowPointer = if ($depth >= self::MAX_DEPTH) { throw new InvalidDatabaseException( - "The MaxMind DB file's data section exceeds the maximum depth" + 'The MaxMind DB file exceeds the maximum depth' ); } @@ -228,7 +228,7 @@ private function decodeByType(int $type, int $offset, int $size, int $depth): ar // budget negative. A total exactly at the limit is allowed. if ($size > $this->byteBudget) { throw new InvalidDatabaseException( - "The MaxMind DB file's data section exceeds the maximum payload size" + 'The MaxMind DB file exceeds the maximum payload size' ); } $this->byteBudget -= $size; @@ -338,7 +338,7 @@ private function enterContainer( ): void { if ($depth >= self::MAX_DEPTH) { throw new InvalidDatabaseException( - "The MaxMind DB file's data section exceeds the maximum depth" + 'The MaxMind DB file exceeds the maximum depth' ); } // Compare with a division rather than multiplying the declared size, so @@ -346,7 +346,7 @@ private function enterContainer( // before the budget check runs. if ($size > intdiv($this->budget, $valuesPerEntry)) { throw new InvalidDatabaseException( - "The MaxMind DB file's data section exceeds the maximum number of values" + 'The MaxMind DB file exceeds the maximum number of values' ); } $this->budget -= $size * $valuesPerEntry; diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index bd8799e7..b8e465be 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -446,7 +446,7 @@ public function testPointerFanOutIsBounded(): void $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage( - "The MaxMind DB file's data section exceeds the maximum number of values" + 'The MaxMind DB file exceeds the maximum number of values' ); (new Decoder($handle, 0))->decode($prev); } @@ -472,7 +472,7 @@ public function testMapPointerFanOutIsBounded(): void $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage( - "The MaxMind DB file's data section exceeds the maximum number of values" + 'The MaxMind DB file exceeds the maximum number of values' ); (new Decoder($handle, 0))->decode($prev); } @@ -491,7 +491,7 @@ public function testOversizedArrayIsBounded(): void $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage( - "The MaxMind DB file's data section exceeds the maximum number of values" + 'The MaxMind DB file exceeds the maximum number of values' ); (new Decoder($handle, 0))->decode(0); } @@ -524,7 +524,7 @@ public function testPointerFreeContainerOverMaximumDepthIsBounded(): void $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage( - "The MaxMind DB file's data section exceeds the maximum depth" + 'The MaxMind DB file exceeds the maximum depth' ); (new Decoder($handle, 0))->decode(0); } @@ -575,7 +575,7 @@ public function testPointerChainOverMaximumDepthIsBounded(): void $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage( - "The MaxMind DB file's data section exceeds the maximum depth" + 'The MaxMind DB file exceeds the maximum depth' ); (new Decoder($handle, 0))->decode($top); } @@ -627,7 +627,7 @@ public function testOversizedMapIsBounded(): void $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage( - "The MaxMind DB file's data section exceeds the maximum number of values" + 'The MaxMind DB file exceeds the maximum number of values' ); (new Decoder($handle, 0))->decode(0); } @@ -646,7 +646,7 @@ public function testOversizedStringIsRejectedBeforeRead(): void $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage( - "The MaxMind DB file's data section exceeds the maximum payload size" + 'The MaxMind DB file exceeds the maximum payload size' ); (new Decoder($handle, 0))->decode(0); } diff --git a/tests/MaxMind/Db/Test/ReaderTest.php b/tests/MaxMind/Db/Test/ReaderTest.php index 68d2dba2..11c258d0 100644 --- a/tests/MaxMind/Db/Test/ReaderTest.php +++ b/tests/MaxMind/Db/Test/ReaderTest.php @@ -339,7 +339,7 @@ public function testPayloadAmplificationDosIsRejected(): void // An array of pointers to one large value. The value count stays low, // but a reader that copies each target materializes the value once per // pointer. The produced-payload byte budget rejects it. - $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $this->expectDecoderLimit('The MaxMind DB file exceeds the maximum payload size'); $reader = new Reader('tests/data/test-data/MaxMind-DB-test-payload-amplification-dos.mmdb'); $reader->get('1.1.1.1'); } @@ -347,7 +347,7 @@ public function testPayloadAmplificationDosIsRejected(): void public function testStringPayloadAmplificationDosIsRejected(): void { // The string variant, so the UTF-8 path is charged as well as bytes. - $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $this->expectDecoderLimit('The MaxMind DB file exceeds the maximum payload size'); $reader = new Reader('tests/data/test-data/MaxMind-DB-test-payload-amplification-dos-string.mmdb'); $reader->get('1.1.1.1'); } @@ -356,7 +356,7 @@ public function testWorstCasePayloadAmplificationDosIsRejected(): void { // The worst case sits exactly at the value limit: 65,535 pointers to // one 64 KiB value. Only the payload budget rejects it. - $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $this->expectDecoderLimit('The MaxMind DB file exceeds the maximum payload size'); $reader = new Reader('tests/data/test-data/MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb'); $reader->get('1.1.1.1'); } @@ -375,7 +375,7 @@ public function testPayloadAtLimitDecodes(): void public function testPayloadOverLimitIsRejected(): void { // One byte past the limit must be rejected. - $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum payload size"); + $this->expectDecoderLimit('The MaxMind DB file exceeds the maximum payload size'); $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-payload-limit-over.mmdb'); $reader->get('1.1.1.1'); } @@ -391,7 +391,7 @@ public function testMetadataPayloadLimitIsRejectedOnOpen(): void // the extension uses its standard database-open error. $this->expectExceptionMessage('Error opening database file'); } else { - $this->expectExceptionMessage("The MaxMind DB file's data section exceeds the maximum payload size"); + $this->expectExceptionMessage('The MaxMind DB file exceeds the maximum payload size'); } new Reader('tests/data/test-data/MaxMind-DB-test-metadata-payload-limit.mmdb'); } @@ -400,7 +400,7 @@ public function testPointerFanOutDosIsRejected(): void { // Nested arrays of pointers to the level below: 2**40 leaf decodes // from 451 bytes. The value budget rejects it. - $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum number of values"); + $this->expectDecoderLimit('The MaxMind DB file exceeds the maximum number of values'); $reader = new Reader('tests/data/test-data/MaxMind-DB-test-pointer-decoder-dos.mmdb'); $reader->get('1.1.1.1'); } @@ -408,7 +408,7 @@ public function testPointerFanOutDosIsRejected(): void public function testPointerFanOutDosIpv6IsRejected(): void { // The same fan-out in a conventional IPv6 database. - $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum number of values"); + $this->expectDecoderLimit('The MaxMind DB file exceeds the maximum number of values'); $reader = new Reader('tests/data/test-data/MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb'); $reader->get('::1'); } @@ -433,7 +433,7 @@ public function testValueCountAtLimitDecodes(): void public function testValueCountOverLimitIsRejected(): void { // One value past the limit must be rejected. - $this->expectDecoderLimit("The MaxMind DB file's data section exceeds the maximum number of values"); + $this->expectDecoderLimit('The MaxMind DB file exceeds the maximum number of values'); $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-value-limit-over.mmdb'); $reader->get('1.1.1.1'); } From 8e829f54ab0de17eccd949749adccf00b154aa62 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 9 Sep 2026 22:31:57 +0000 Subject: [PATCH 12/13] Declare the PHP 8 test polyfill directly The shared decoder-limit probe uses str_contains() on supported PHP 7 versions. Require its polyfill in require-dev instead of depending on the formatter to install it. --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 7ce7766c..f0a9fadf 100644 --- a/composer.json +++ b/composer.json @@ -28,7 +28,8 @@ "friendsofphp/php-cs-fixer": "3.*", "phpunit/phpunit": ">=8.0.0,<10.0.0", "squizlabs/php_codesniffer": "4.*", - "phpstan/phpstan": "*" + "phpstan/phpstan": "*", + "symfony/polyfill-php80": "^1.33" }, "autoload": { "psr-4": { From 87b1147e9faf949e1f2ed9ac9525713882c4cc43 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 9 Sep 2026 22:31:58 +0000 Subject: [PATCH 13/13] Clarify decoder limit and stream comments Keep the budget counting and sequential-read rules. Remove repeated explanations, benchmark percentages, and unsupported claims about buffer invalidation, integer overflow, and record sizes. --- src/MaxMind/Db/Reader/Decoder.php | 40 ++++++++----------------------- 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index f4404ef8..02d9a108 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -77,18 +77,15 @@ class Decoder // specification's flat rule: the root is one value, each array and map // charges its declared children (a map entry costs two, key and value), // and a pointer costs nothing beyond the value it resolves to, which its - // container already charged. The largest real records decode a few hundred - // values, so the limit leaves a wide margin. + // container already charged. private const MAX_DEPTH = 512; private const MAX_VALUES = 1 << 16; // The value limit alone does not stop payload amplification: an array of // pointers to one large string or bytes value keeps the value count low // while forcing the reader to copy the target once per pointer. This - // second, independent limit bounds the total string and bytes payload - // copied for one lookup to 2 MiB, matching libmaxminddb and the Go reader. - // No real record approaches it, and re-decoding a shared target charges its - // payload again, so the fan-out is bounded. + // second limit bounds the total string and bytes payload copied for one + // lookup to 2 MiB. Re-decoding a shared target charges its payload again. private const MAX_PAYLOAD_BYTES = 1 << 21; // A fixed-width scalar (a float, double, or integer) never needs more than @@ -118,15 +115,9 @@ public function __construct( */ public function decode(int $offset): array { - // Bound the work per lookup so a crafted database cannot exhaust CPU or - // memory. $budget counts decoded values and stops the pointer fan-out; - // $byteBudget counts copied string and bytes payload and stops payload - // amplification. Both live on the decoder and are reset here, so every - // call starts with the full allowance. Passing them by reference - // through each recursive call instead costs a few percent per lookup. - // Reader prevents nested lookups from moving its stream during a - // decode. The root value is charged here; containers charge their - // children. + // Reset both budgets for each lookup. Charge the root value here and + // container children when entering each container. Reader prevents + // nested lookups from moving its stream during a decode. $this->budget = self::MAX_VALUES - 1; $this->byteBudget = self::MAX_PAYLOAD_BYTES; @@ -281,10 +272,7 @@ private function decodeByType(int $type, int $offset, int $size, int $depth): ar /** * Reads from the stream, seeking only when the read does not continue where - * the previous one ended. Most values in a record are laid out in order, and - * fseek() discards PHP's read buffer, so seeking before every read turned - * each small read into a system call. Skipping the seek makes a City lookup - * about 40% faster. + * the previous one ended. * * @param int<0, max> $numberOfBytes */ @@ -322,14 +310,9 @@ private function verifySize(int $expected, int $actual): void } /** - * Applies the per-lookup limits when entering a container. The depth limit - * stops cycles and over-deep data (checked here and at pointer follows, - * the only places depth grows). The value budget is charged per declared - * element up front, so an oversized declared size is rejected before the - * loop reads anything. A pointer element costs nothing more when it is - * followed: its slot is charged here, and a container it resolves to - * charges its own children each time it is decoded, which is what bounds - * a fan-out through shared targets. + * Charges declared children before decoding them. An oversized container + * fails before any child is read. Each visit to a shared container charges + * its children again, which bounds pointer fan-out. */ private function enterContainer( int $size, @@ -341,9 +324,6 @@ private function enterContainer( 'The MaxMind DB file exceeds the maximum depth' ); } - // Compare with a division rather than multiplying the declared size, so - // an oversized declaration cannot overflow the integer on 32-bit builds - // before the budget check runs. if ($size > intdiv($this->budget, $valuesPerEntry)) { throw new InvalidDatabaseException( 'The MaxMind DB file exceeds the maximum number of values'