Bound decoder work to prevent a pointer fan-out DoS (STF-1488)

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

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<mixed>
      */
     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<mixed>
+     */
+    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,35 +163,62 @@ 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);
     }
 
     /**
      * @param int<0, max> $size
      *
      * @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<mixed>, 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<string, mixed>, 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;
         }