From 24e28e59329a88ffb604f6cddd00370fca3c6474 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 00:20:09 +0000 Subject: [PATCH 01/11] 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. Pointer cycles and over-deep data are rejected the same way rather than exhausting the stack. This matches the reader resource limits now recommended by the MaxMind DB specification. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 6 ++ src/MaxMind/Db/Reader/Decoder.php | 82 +++++++++++++++++--- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 56 +++++++++++++ 3 files changed, 134 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb1bbb31..88729c93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ CHANGELOG 1.14.0 ------------------- +* Fixed a denial-of-service issue in the pure PHP decoder. A crafted database + could nest data-section pointers to shared targets so that decoding one record + cost exponential time and memory from a small file. The decoder now limits the + number of values it decodes for a single record and rejects a database that + exceeds it, along with pointer cycles and over-deep data, with an + `InvalidDatabaseException`. See GHSA-hj94-g986-h9r7. * 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..248fb31d 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -47,6 +47,14 @@ 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 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 +75,19 @@ 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. Both are call-local. + $budget = self::MAX_VALUES; + + 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 +105,13 @@ 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" + ); + } + + [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget); return [$result, $offset]; } @@ -108,7 +135,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 +143,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 +199,47 @@ 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, and the pointer fan-out is bounded because the + * exponentially re-decoded nodes are containers. + */ + 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 +317,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..b5620dbf 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,61 @@ 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); + (new Decoder($handle, 0))->decode($prev); + } + + 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. A map that declares 32,769 entries reaches 65,538 values, just + // past the 65,536 limit, and is rejected before any entry is read. 0xfe + // is a map with size code 30, then the two size bytes for + // 32,769 - 285 = 32,484 (0x7ee4). + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\xfe\x7e\xe4"); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + (new Decoder($handle, 0))->decode(0); + } + // @phpstan-ignore-next-line private function checkDecoding(string $type, array $input, $expected, $name = null): void { From 5d63b00a4c27780a684235b125c248d4999388b4 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 21:54:20 +0000 Subject: [PATCH 02/11] fixup! Bound decoder work to prevent a pointer fan-out denial of service --- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 59 ++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index b5620dbf..cd3c4721 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -448,6 +448,65 @@ public function testPointerFanOutIsBounded(): void (new Decoder($handle, 0))->decode($prev); } + public function testMapPointerFanOutIsBounded(): void + { + // Each map has two scalar 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\xa0" . $this->encodePointer1($prev) + . "\xa0" . $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 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); + } + public function testCyclicPointerThrows(): void { // A pointer to itself must throw a catchable InvalidDatabaseException From 1db1dfc447c3d1769537741f4e8d87d9e5742cbc Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 21:56:36 +0000 Subject: [PATCH 03/11] fixup! Bound decoder work to prevent a pointer fan-out denial of service --- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index cd3c4721..3cee52ce 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -450,7 +450,7 @@ public function testPointerFanOutIsBounded(): void public function testMapPointerFanOutIsBounded(): void { - // Each map has two scalar keys whose values point to the map below. + // 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; @@ -458,8 +458,8 @@ public function testMapPointerFanOutIsBounded(): void $prev = 0; for ($i = 0; $i < $depth; ++$i) { $offset = \strlen($buf); - $buf .= "\xe2\xa0" . $this->encodePointer1($prev) - . "\xa0" . $this->encodePointer1($prev); + $buf .= "\xe2\x41a" . $this->encodePointer1($prev) + . "\x41b" . $this->encodePointer1($prev); $prev = $offset; } From 7e30dc397c7c6979bcde909877982e3dfe6b5f22 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 22:08:59 +0000 Subject: [PATCH 04/11] fixup! Bound decoder work to prevent a pointer fan-out denial of service --- src/MaxMind/Db/Reader/Decoder.php | 13 +++++++++++-- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 248fb31d..7e3c6b62 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -111,6 +111,15 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget): array ); } + // The container containing this pointer pays for the pointer value. + // Pay separately for the referenced target each time it is decoded. + if ($budget === 0) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum number of values" + ); + } + --$budget; + [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget); return [$result, $offset]; @@ -204,8 +213,8 @@ private function verifySize(int $expected, int $actual): void * 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, and the pointer fan-out is bounded because the - * exponentially re-decoded nodes are containers. + * loop reads anything. Pointer targets are charged separately when they + * are followed, which also bounds fan-out to scalar targets. */ private function enterContainer( int $size, diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index 3cee52ce..6c8f8c59 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -474,6 +474,26 @@ public function testMapPointerFanOutIsBounded(): void (new Decoder($handle, 0))->decode($prev); } + public function testFlatScalarPointerFanOutIsBounded(): void + { + // The array's 32,769 elements and their referenced scalar targets total + // 65,538 values, just past the 65,536 limit. The complete fixture is + // only about 64 KiB and does not require a large scalar allocation. + $buf = "\xa0"; // scalar target: uint16 with value 0 + $buf .= "\x1e\x04\x7e\xe4"; // array with 32,769 elements + $buf .= str_repeat("\x20\x00", 32769); // pointers to offset 0 + + $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(1); + } + public function testPointerFreeContainerAtMaximumDepthDecodes(): void { $buf = "\xa0"; // leaf: uint16 with value 0 From cc43aaf52ee4f23d126cac4296f6bd2fb8ab354f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 22:33:32 +0000 Subject: [PATCH 05/11] fixup! Bound decoder work to prevent a pointer fan-out denial of service --- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index 6c8f8c59..b9cd959e 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -445,6 +445,9 @@ public function testPointerFanOutIsBounded(): void 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); } From acb6fd6a921eca0bc7135307aa368748a4241696 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 23:34:38 +0000 Subject: [PATCH 06/11] fixup! Bound decoder work to prevent a pointer fan-out denial of service --- src/MaxMind/Db/Reader/Decoder.php | 4 ++-- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/MaxMind/Db/Reader/Decoder.php b/src/MaxMind/Db/Reader/Decoder.php index 7e3c6b62..0ab941a3 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -105,7 +105,7 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget): array return [$pointer]; } - if ($depth > self::MAX_DEPTH) { + if ($depth >= self::MAX_DEPTH) { throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum depth" ); @@ -222,7 +222,7 @@ private function enterContainer( int &$budget, int $valuesPerEntry = 1 ): void { - if ($depth > self::MAX_DEPTH) { + if ($depth >= self::MAX_DEPTH) { throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum depth" ); diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index b9cd959e..ce0e115d 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -500,7 +500,7 @@ public function testFlatScalarPointerFanOutIsBounded(): void public function testPointerFreeContainerAtMaximumDepthDecodes(): void { $buf = "\xa0"; // leaf: uint16 with value 0 - for ($i = 0; $i <= 512; ++$i) { + for ($i = 0; $i < 512; ++$i) { $buf = "\x01\x04" . $buf; // array with one element } @@ -515,7 +515,7 @@ public function testPointerFreeContainerAtMaximumDepthDecodes(): void public function testPointerFreeContainerOverMaximumDepthIsBounded(): void { $buf = "\xa0"; // leaf: uint16 with value 0 - for ($i = 0; $i <= 513; ++$i) { + for ($i = 0; $i < 513; ++$i) { $buf = "\x01\x04" . $buf; // array with one element } From 9e07d09a0e83a28550a56f004a7cdcdb1293eff4 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 23:35:11 +0000 Subject: [PATCH 07/11] fixup! Bound decoder work to prevent a pointer fan-out denial of service --- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index ce0e115d..4f906b0f 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -554,6 +554,9 @@ public function testOversizedMapIsBounded(): void 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); } From eafe8620f4770ae004c2b949e2c4d70d625eb544 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 27 Aug 2026 13:50:41 +0000 Subject: [PATCH 08/11] Bound decoder payload to stop an amplification denial of service The value and depth limits stop the pointer fan-out, but not payload amplification: an array of pointers to one large string or bytes value keeps the value count low while a reader that copies each target materializes the value once per pointer. A file of a few hundred kilobytes can force gigabytes. Add a second, independent per-lookup limit that bounds the total string and bytes payload copied for one decode to 2 MiB, matching libmaxminddb and the Go reader. The byte budget is charged wherever a string or bytes value is decoded, including inline inside a pointed-to container, so a shared target recharges each time it is followed. This also covers a pointer-backed map key, which decodes through the same path. The budget is call-local and passed by reference, so concurrent lookups do not share state. Also reject a fixed-width scalar whose declared size exceeds 16 bytes before the bytes are read, so an oversized variable-length integer cannot amplify the read. Both new limits reject with the existing InvalidDatabaseException, so there is no public API change and no behavior change for valid records. Bump the test-data submodule for the payload-amplification fixtures and add regression tests for each attack shape. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 5 +- src/MaxMind/Db/Reader/Decoder.php | 78 +++++++++++++++----- tests/MaxMind/Db/Test/Reader/DecoderTest.php | 18 +++++ tests/MaxMind/Db/Test/ReaderTest.php | 57 ++++++++++++++ tests/data | 2 +- 5 files changed, 140 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88729c93..afcfe79a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,10 @@ CHANGELOG cost exponential time and memory from a small file. The decoder now limits the number of values it decodes for a single record and rejects a database that exceeds it, along with pointer cycles and over-deep data, with an - `InvalidDatabaseException`. See GHSA-hj94-g986-h9r7. + `InvalidDatabaseException`. The decoder also limits the total string and + bytes payload it copies for a single record, so a database that references + one large value through many pointers is rejected instead of copying the + value once per pointer. See GHSA-hj94-g986-h9r7. * 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 0ab941a3..155f289a 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -55,6 +55,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 */ @@ -77,17 +92,21 @@ 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. Both are call-local. + // 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. $budget = self::MAX_VALUES; + $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 +139,7 @@ private function decodeWithBudget(int $offset, int $depth, int &$budget): array } --$budget; - [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget); + [$result] = $this->decodeWithBudget($pointer, $depth + 1, $budget, $byteBudget); return [$result, $offset]; } @@ -144,7 +163,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 +171,50 @@ 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]; + } + + // Every remaining type is a fixed-width scalar. 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); @@ -241,14 +283,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; } @@ -326,7 +368,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); @@ -334,8 +376,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 4f906b0f..446d6066 100644 --- a/tests/MaxMind/Db/Test/Reader/DecoderTest.php +++ b/tests/MaxMind/Db/Test/Reader/DecoderTest.php @@ -560,6 +560,24 @@ public function testOversizedMapIsBounded(): void (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. 0xd1 is a uint32 with the size + // encoded directly as 17. + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\xd1" . str_repeat("\x00", 17)); + 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..20b0c01b 100644 --- a/tests/MaxMind/Db/Test/ReaderTest.php +++ b/tests/MaxMind/Db/Test/ReaderTest.php @@ -291,6 +291,63 @@ public function testBrokenDataPointer(): void $reader->get('1.1.1.16'); } + 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->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage("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->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage("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 keeps the produced payload just under the byte budget + // while fanning out through tens of thousands of pointers, so a bound + // on decoded values rejects it. + $this->expectException(InvalidDatabaseException::class); + $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. + $reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder-payload-limit.mmdb'); + $this->assertIsArray($reader->get('1.1.1.1')); + $reader->close(); + } + + public function testPayloadOverLimitIsRejected(): void + { + // One byte past the limit must be rejected. + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage("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->expectException(InvalidDatabaseException::class); + $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 b019327b..d692a4b7 160000 --- a/tests/data +++ b/tests/data @@ -1 +1 @@ -Subproject commit b019327b2c96a4efe08a9aa20c9e73150d104147 +Subproject commit d692a4b74c68c6e856d0bd85a38ee405b65c816f From 58b8b42de9ebb2a541523d199324c13c3e319f15 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 27 Aug 2026 14:35:06 +0000 Subject: [PATCH 09/11] Document the integer and metadata guards in the DoS fix The changelog described only the value, depth, and payload limits. State the two other user-observable effects of the same fix: the decoder rejects a scalar whose declared length is more than 16 bytes, and all of the limits apply to the metadata read when a database is opened. Also correct a comment in Decoder.php. The remaining types are not all fixed-width scalars: the container (12) and end-marker (13) types and any unknown extended type also reach that point, where the size guard or the default case rejects them. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 6 +++++- src/MaxMind/Db/Reader/Decoder.php | 10 +++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afcfe79a..11ff1b7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,11 @@ CHANGELOG `InvalidDatabaseException`. The decoder also limits the total string and bytes payload it copies for a single record, so a database that references one large value through many pointers is rejected instead of copying the - value once per pointer. See GHSA-hj94-g986-h9r7. + value once per pointer. The decoder also rejects a scalar whose declared + length is more than 16 bytes, the width of the widest fixed-width type, so an + oversized variable-length integer cannot amplify a read. All of these limits + now also apply to the metadata that is read when a database is opened. See + GHSA-hj94-g986-h9r7. * 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 155f289a..2802feeb 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -202,9 +202,13 @@ private function decodeByType(int $type, int $offset, int $size, int $depth, int return [Util::read($this->fileStream, $offset, $size), $offset + $size]; } - // Every remaining type is a fixed-width scalar. Reject an oversized - // declared size before materializing the bytes, so an oversized - // variable-length integer cannot amplify the read. + // 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)" From e711d950bd1166480171eb0a7a4531f7b5a57910 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 27 Aug 2026 18:15:34 +0000 Subject: [PATCH 10/11] Test the extension rejects the decoder DoS fixtures The DoS tests in ReaderTest assert the pure-PHP decoder's messages, so they cover only that path. When the maxminddb extension is loaded, Reader decodes through libmaxminddb, which has its own copy of the limits, and nothing asserted that path rejects the DoS fixtures. Add extension-path checks that decode each DoS fixture through the loaded extension and assert an InvalidDatabaseException. The limits live in libmaxminddb, so the checks first probe a fixture one byte over the 2 MiB payload limit, which is small and safe to decode. A libmaxminddb with the fix rejects it with the decoder-limit message and the checks run; an older one decodes it and the checks skip, rather than run the large DoS fixtures through a decoder that would exhaust memory. Co-Authored-By: Claude Opus 4.8 --- .../Db/Test/Reader/ExtensionDosTest.php | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 tests/MaxMind/Db/Test/Reader/ExtensionDosTest.php diff --git a/tests/MaxMind/Db/Test/Reader/ExtensionDosTest.php b/tests/MaxMind/Db/Test/Reader/ExtensionDosTest.php new file mode 100644 index 00000000..e02bb7c8 --- /dev/null +++ b/tests/MaxMind/Db/Test/Reader/ExtensionDosTest.php @@ -0,0 +1,103 @@ +markTestSkipped('maxminddb extension not loaded'); + } + + // Probe with a fixture one byte over the 2 MiB payload limit. A patched + // libmaxminddb rejects it with the decoder-limit message. An older one + // decodes it, which is only about 2 MiB and so safe, but means the + // large DoS fixtures below would exhaust memory, so skip instead of + // running them. + try { + $this->lookup('MaxMind-DB-test-decoder-payload-limit-over.mmdb'); + } catch (InvalidDatabaseException $e) { + if (str_contains($e->getMessage(), self::LIMIT_MESSAGE)) { + return; + } + } + $this->markTestSkipped( + 'linked libmaxminddb predates the decoder resource limits ' + . '(needs the fix that adds MMDB_DECODER_LIMIT_ERROR)' + ); + } + + public function testPointerFanOutFixtureIsRejected(): void + { + // A record that nests arrays of pointers to the level below, the + // classic 2**depth fan-out. + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage(self::LIMIT_MESSAGE); + $this->lookup('MaxMind-DB-test-pointer-decoder-dos.mmdb'); + } + + public function testPayloadAmplificationIsRejected(): void + { + // An array of pointers to one large value. + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage(self::LIMIT_MESSAGE); + $this->lookup('MaxMind-DB-test-payload-amplification-dos.mmdb'); + } + + public function testStringPayloadAmplificationIsRejected(): void + { + // The UTF-8 string variant, so the string decode path is exercised. + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage(self::LIMIT_MESSAGE); + $this->lookup('MaxMind-DB-test-payload-amplification-dos-string.mmdb'); + } + + public function testWorstCasePayloadAmplificationIsRejected(): void + { + // The largest fan-out that stays under the value limit. + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage(self::LIMIT_MESSAGE); + $this->lookup('MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb'); + } + + /** + * Look up any IPv4 address, which each DoS fixture resolves to its single + * crafted record. + */ + private function lookup(string $fileName): void + { + $reader = new Reader('tests/data/test-data/' . $fileName); + + try { + $reader->get('1.1.1.1'); + } finally { + $reader->close(); + } + } +} From 62b7265b319b913d221eed9bc7f66baac8493ddd Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 27 Aug 2026 18:19:12 +0000 Subject: [PATCH 11/11] Skip the pure-PHP DoS tests when the extension is loaded These tests assert the pure-PHP decoder's "exceeds the maximum payload size" message. When the maxminddb extension is loaded, Reader decodes through libmaxminddb, which reports different text, so the assertions failed. They also fed the large amplification and fan-out fixtures to the extension's decoder, which on a libmaxminddb without the fix would exhaust memory. Skip them when the extension is loaded. The extension path is covered safely by ExtensionDosTest, which probes a small fixture first and runs the large fixtures only against a libmaxminddb that enforces the limits. Co-Authored-By: Claude Opus 4.8 --- tests/MaxMind/Db/Test/ReaderTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/MaxMind/Db/Test/ReaderTest.php b/tests/MaxMind/Db/Test/ReaderTest.php index 20b0c01b..f44f15d9 100644 --- a/tests/MaxMind/Db/Test/ReaderTest.php +++ b/tests/MaxMind/Db/Test/ReaderTest.php @@ -291,8 +291,24 @@ public function testBrokenDataPointer(): void $reader->get('1.1.1.16'); } + // The DoS and payload-limit tests below assert the pure-PHP decoder's + // messages, so they skip when the maxminddb extension is loaded: that path + // decodes through libmaxminddb, which reports different text, and + // ExtensionDosTest covers it. Skipping also keeps the large amplification + // and fan-out fixtures away from a possibly-unpatched extension, whose + // decoder would otherwise exhaust memory. + private function skipIfExtensionLoaded(): void + { + if (\extension_loaded('maxminddb')) { + $this->markTestSkipped( + 'covers the pure-PHP decoder; the extension path is covered by ExtensionDosTest' + ); + } + } + public function testPayloadAmplificationDosIsRejected(): void { + $this->skipIfExtensionLoaded(); // 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. @@ -304,6 +320,7 @@ public function testPayloadAmplificationDosIsRejected(): void public function testStringPayloadAmplificationDosIsRejected(): void { + $this->skipIfExtensionLoaded(); // The string variant, so the UTF-8 path is charged as well as bytes. $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage("The MaxMind DB file's data section exceeds the maximum payload size"); @@ -313,6 +330,7 @@ public function testStringPayloadAmplificationDosIsRejected(): void public function testWorstCasePayloadAmplificationDosIsRejected(): void { + $this->skipIfExtensionLoaded(); // The worst case keeps the produced payload just under the byte budget // while fanning out through tens of thousands of pointers, so a bound // on decoded values rejects it. @@ -332,6 +350,7 @@ public function testPayloadAtLimitDecodes(): void public function testPayloadOverLimitIsRejected(): void { + $this->skipIfExtensionLoaded(); // One byte past the limit must be rejected. $this->expectException(InvalidDatabaseException::class); $this->expectExceptionMessage("The MaxMind DB file's data section exceeds the maximum payload size"); @@ -341,6 +360,7 @@ public function testPayloadOverLimitIsRejected(): void public function testMetadataPayloadLimitIsRejectedOnOpen(): void { + $this->skipIfExtensionLoaded(); // Metadata is decoded while opening the database, so the same bound // must guard that path. $this->expectException(InvalidDatabaseException::class);