diff --git a/CHANGELOG.md b/CHANGELOG.md index cb1bbb3..11ff1b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ 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`. 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. 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 1bb6731..2802fee 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -47,6 +47,29 @@ 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; + + // 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 */ @@ -67,6 +90,23 @@ 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. 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, $byteBudget); + } + + /** + * @return array + */ + private function decodeWithBudget(int $offset, int $depth, int &$budget, int &$byteBudget): array { $ctrlByte = \ord(Util::read($this->fileStream, $offset, 1)); ++$offset; @@ -84,7 +124,22 @@ 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 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, $byteBudget); return [$result, $offset]; } @@ -108,7 +163,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, $byteBudget); } /** @@ -116,27 +171,54 @@ 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, int &$byteBudget): array { switch ($type) { case self::_MAP: - return $this->decodeMap($size, $offset); + return $this->decodeMap($size, $offset, $depth, $budget, $byteBudget); case self::_ARRAY: - return $this->decodeArray($size, $offset); + 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); @@ -172,15 +254,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. Pointer targets are charged separately when they + * are followed, which also bounds fan-out to scalar 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, int &$byteBudget): 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, $byteBudget); $array[] = $value; } @@ -258,13 +372,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, int &$byteBudget): 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, $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 e935452..446d606 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,164 @@ 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 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 + 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 + // 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); + $this->expectExceptionMessage( + "The MaxMind DB file's data section exceeds the maximum number of values" + ); + (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/Reader/ExtensionDosTest.php b/tests/MaxMind/Db/Test/Reader/ExtensionDosTest.php new file mode 100644 index 0000000..e02bb7c --- /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(); + } + } +} diff --git a/tests/MaxMind/Db/Test/ReaderTest.php b/tests/MaxMind/Db/Test/ReaderTest.php index ab24b40..f44f15d 100644 --- a/tests/MaxMind/Db/Test/ReaderTest.php +++ b/tests/MaxMind/Db/Test/ReaderTest.php @@ -291,6 +291,83 @@ 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. + $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 + { + $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"); + $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 + { + $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. + $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 + { + $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"); + $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 + { + $this->skipIfExtensionLoaded(); + // 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 b019327..d692a4b 160000 --- a/tests/data +++ b/tests/data @@ -1 +1 @@ -Subproject commit b019327b2c96a4efe08a9aa20c9e73150d104147 +Subproject commit d692a4b74c68c6e856d0bd85a38ee405b65c816f