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