diff --git a/CHANGELOG.md b/CHANGELOG.md index cb1bbb3..1cebbbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,24 @@ 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, 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 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/composer.json b/composer.json index 7ce7766..f0a9fad 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": { diff --git a/ext/bundled-include/maxminddb_config.h b/ext/bundled-include/maxminddb_config.h index c578a4e..0c12d19 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 09a0540..0077fd7 160000 --- a/ext/libmaxminddb +++ b/ext/libmaxminddb @@ -1 +1 @@ -Subproject commit 09a0540fea89a16e5c6a9e21e93ee9aece6639e3 +Subproject commit 0077fd76d00a1656b9cb3028d467736504794f41 diff --git a/src/MaxMind/Db/Reader.php b/src/MaxMind/Db/Reader.php index 0524ff1..923dc02 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 1bb6731..02d9a10 100644 --- a/src/MaxMind/Db/Reader/Decoder.php +++ b/src/MaxMind/Db/Reader/Decoder.php @@ -30,6 +30,29 @@ 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; + + /** + * 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; @@ -47,6 +70,30 @@ 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. + 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 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 + // 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 */ @@ -68,7 +115,25 @@ public function __construct( */ public function decode(int $offset): array { - $ctrlByte = \ord(Util::read($this->fileStream, $offset, 1)); + // 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; + + // 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); + } + + /** + * @return array + */ + private function decodeWithBudget(int $offset, int $depth, bool $allowPointer = true): array + { + $ctrlByte = \ord($this->read($offset, 1)); ++$offset; $type = $ctrlByte >> 5; @@ -77,6 +142,12 @@ public function decode(int $offset): 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 @@ -84,13 +155,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 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, false); return [$result, $offset]; } if ($type === self::_EXTENDED) { - $nextByte = \ord(Util::read($this->fileStream, $offset, 1)); + $nextByte = \ord($this->read($offset, 1)); $type = $nextByte + 7; @@ -108,7 +188,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); } /** @@ -116,27 +196,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): array { switch ($type) { case self::_MAP: - return $this->decodeMap($size, $offset); + return $this->decodeMap($size, $offset, $depth); case self::_ARRAY: - return $this->decodeArray($size, $offset); + return $this->decodeArray($size, $offset, $depth); 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 > $this->byteBudget) { + throw new InvalidDatabaseException( + 'The MaxMind DB file exceeds the maximum payload size' + ); + } + $this->byteBudget -= $size; + + return [$this->read($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); + $bytes = $this->read($offset, $size); switch ($type) { - case self::_BYTES: - case self::_UTF8_STRING: - return [$bytes, $newOffset]; - case self::_DOUBLE: $this->verifySize(8, $size); @@ -163,6 +270,36 @@ private function decodeByType(int $type, int $offset, int $size): array } } + /** + * Reads from the stream, seeking only when the read does not continue where + * the previous one ended. + * + * @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) { @@ -172,15 +309,40 @@ private function verifySize(int $expected, int $actual): void } } + /** + * 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, + int $depth, + int $valuesPerEntry = 1 + ): void { + if ($depth >= self::MAX_DEPTH) { + throw new InvalidDatabaseException( + 'The MaxMind DB file exceeds the maximum depth' + ); + } + if ($size > intdiv($this->budget, $valuesPerEntry)) { + throw new InvalidDatabaseException( + 'The MaxMind DB file exceeds the maximum number of values' + ); + } + $this->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): array { + $this->enterContainer($size, $depth); + $array = []; for ($i = 0; $i < $size; ++$i) { - [$value, $offset] = $this->decode($offset); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1); $array[] = $value; } @@ -258,13 +420,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): array { + // A map entry decodes a key and a value, so it costs two values. + $this->enterContainer($size, $depth, 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); + [$value, $offset] = $this->decodeWithBudget($offset, $depth + 1); $map[$key] = $value; } @@ -278,7 +443,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) { @@ -404,7 +569,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 c2c3212..c5485ea 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; } } diff --git a/tests/MaxMind/Db/Test/Reader/DecoderTest.php b/tests/MaxMind/Db/Test/Reader/DecoderTest.php index e935452..b8e465b 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,315 @@ 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 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 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 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 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 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 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 + // 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 exceeds the maximum number of values' + ); + (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 (0x1efee4). + $handle = fopen('php://memory', 'rwb'); + fwrite($handle, "\x5f\x1e\xfe\xe4"); + fseek($handle, 0); + + $this->expectException(InvalidDatabaseException::class); + $this->expectExceptionMessage( + 'The MaxMind DB file 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); + } + + 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 { diff --git a/tests/MaxMind/Db/Test/Reader/ReentrancyTest.php b/tests/MaxMind/Db/Test/Reader/ReentrancyTest.php new file mode 100644 index 0000000..568b358 --- /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 0000000..73afef7 --- /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(); + } +} diff --git a/tests/MaxMind/Db/Test/ReaderTest.php b/tests/MaxMind/Db/Test/ReaderTest.php index ab24b40..11c258d 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,151 @@ 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 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 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 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 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 exceeds the maximum payload size'); + } + 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 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 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 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); diff --git a/tests/data b/tests/data index b2a3df1..363086b 160000 --- a/tests/data +++ b/tests/data @@ -1 +1 @@ -Subproject commit b2a3df13c0e274d7a2dca3d5415465a3a9670e23 +Subproject commit 363086b7d90650100e91f954937794c6a090c2a0