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

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

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": {

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 */

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;
+        }
     }
 
     /**