Bound decoded values to prevent a pointer fan-out DoS (STF-1568)

Changes.md

@@ -1,5 +1,24 @@
 ## next release
 
+- Bounded the resources that `MMDB_get_entry_data_list()` spends decoding a
+  single entry. A crafted database could nest data-section pointers to shared
+  targets so that decoding one entry cost exponential time and memory, or point
+  many times at one large value so that a caller copying the result materialized
+  far more data than the file holds. The decoder now follows the Reader Resource
+  Limits section of the MaxMind DB specification. Each call is limited to 65,536
+  values and 2 MiB of string and bytes payload, in addition to the existing
+  recursive-decoder depth limit of 512. See the `MMDB_get_entry_data_list()`
+  documentation for details.
+  - Exceeding a limit returns the new `MMDB_DECODER_LIMIT_ERROR` status. A
+    full-list failure leaves the output set to `NULL`.
+  - `MMDB_get_value()`, `MMDB_vget_value()`, and `MMDB_aget_value()` now return
+    `MMDB_DECODER_LIMIT_ERROR` instead of `MMDB_INVALID_DATA_ERROR` when they
+    skip a subtree past the depth limit.
+  - `MMDB_open()` returns `MMDB_INVALID_METADATA_ERROR` when metadata processing
+    exceeds a decoder limit.
+  - The limits can be raised when building the library with
+    `-DMAXIMUM_DATA_STRUCTURE_DEPTH`, `-DMAXIMUM_DATA_STRUCTURE_VALUES`, and
+    `-DMAXIMUM_DATA_STRUCTURE_BYTES`.
 - Fixed an out-of-bounds read in `MMDB_lookup_sockaddr()` when callers passed a
   `sockaddr` with an unsupported address family. The function now rejects any
   family other than `AF_INET` and `AF_INET6` with

README.fuzzing.md

@@ -34,7 +34,7 @@ $ cmake --build . -j$(nproc)
 
 ```shell
 $ mkdir -p fuzz_mmdb_seed fuzz_mmdb_seed_corpus
-$ find ../t/maxmind-db/test-data/ -type f -size -4k -exec cp {} ./fuzz_mmdb_seed_corpus/ \;
+$ find ../t/maxmind-db/test-data/ -type f -size -256k -exec cp {} ./fuzz_mmdb_seed_corpus/ \;
 $ ./t/fuzz_mmdb fuzz_mmdb_seed/ fuzz_mmdb_seed_corpus/
 ```
 

doc/libmaxminddb.md

@@ -395,6 +395,9 @@ status codes are:
   array where none exist.
 - `MMDB_INVALID_NETWORK_ADDRESS_ERROR` - `MMDB_lookup_sockaddr()` was given a
   `sockaddr` whose family is neither `AF_INET` nor `AF_INET6`.
+- `MMDB_DECODER_LIMIT_ERROR` - decoding a data structure would exceed the
+  configured nesting depth, value-count, or string/bytes payload limit. The
+  structure may still be valid MaxMind DB data.
 
 All status codes should be treated as `int` values.
 
@@ -452,6 +455,10 @@ You can also pass `0` as the `flags` value in which case the database will be
 opened with the default flags. However, these defaults may change in future
 releases. The current default is `MMDB_MODE_MMAP`.
 
+If metadata processing exceeds one of the decoder resource limits described
+under `MMDB_get_entry_data_list()`, this function returns
+`MMDB_INVALID_METADATA_ERROR`.
+
 ## `MMDB_close()`
 
 ```c
@@ -640,6 +647,31 @@ This function allows you to get all of the data for a complex data structure at
 once, rather than looking up each piece using repeated calls to
 `MMDB_get_value()`.
 
+A crafted database can make a full decode expensive, so this function bounds the
+work and the caller-visible payload. Each call decodes at most 65,536 list
+values and 2 MiB of UTF-8 string and bytes payload. A structure exactly at
+either limit is accepted. The recursive-decoder depth limit of 512 also applies.
+Exceeding any limit returns `MMDB_DECODER_LIMIT_ERROR` and sets
+`entry_data_list` to `NULL`.
+
+The limits are per call and can be changed when rebuilding libmaxminddb by
+defining the positive integer macros `MAXIMUM_DATA_STRUCTURE_DEPTH`,
+`MAXIMUM_DATA_STRUCTURE_VALUES`, and `MAXIMUM_DATA_STRUCTURE_BYTES`. For
+example, pass `-DMAXIMUM_DATA_STRUCTURE_BYTES=3145728` in the library's compiler
+flags. Use an integer constant rather than an expression. A bare shift such as
+`1<<31` is evaluated as `int` and does not express a 2 GiB limit. The depth
+value must fit in `int`, the value count in `size_t`, and the byte count in
+`uint64_t`. This requires rebuilding the library itself. Defining a macro only
+while building an application does not change a packaged shared library.
+
+`MMDB_get_value()`, `MMDB_vget_value()`, and `MMDB_aget_value()` do not expand a
+complete structure and therefore do not charge the value-count or payload
+budgets. When they skip an unselected map or array, the depth limit applies to
+that subtree and may return `MMDB_DECODER_LIMIT_ERROR`; it does not bound the
+selected lookup path itself. Applications that cannot rebuild a packaged library
+can use those functions to retrieve a specific field from an otherwise
+over-limit record.
+
 ```c
 MMDB_lookup_result_s result =
     MMDB_lookup_sockaddr(&mmdb, address->ai_addr, &mmdb_error);
@@ -717,7 +749,9 @@ int MMDB_get_metadata_as_entry_data_list(
 
 This function allows you to retrieve the database metadata as a linked list of
 `MMDB_entry_data_list_s` structures. This can be a more convenient way to deal
-with the metadata than using the metadata structure directly.
+with the metadata than using the metadata structure directly. It uses the same
+per-call limits as `MMDB_get_entry_data_list()` and returns
+`MMDB_DECODER_LIMIT_ERROR` if the complete metadata list exceeds any of them.
 
 ```c
     MMDB_entry_data_list_s *entry_data_list, *first;

include/maxminddb.h

@@ -87,6 +87,7 @@ extern "C" {
     #define MMDB_INVALID_NODE_NUMBER_ERROR (10)
     #define MMDB_IPV6_LOOKUP_IN_IPV4_DATABASE_ERROR (11)
     #define MMDB_INVALID_NETWORK_ADDRESS_ERROR (12)
+    #define MMDB_DECODER_LIMIT_ERROR (13)
 
     #if !(MMDB_UINT128_IS_BYTE_ARRAY)
         #if MMDB_UINT128_USING_MODE

src/data-pool.c

@@ -9,28 +9,37 @@
 #include <stddef.h>
 #include <stdlib.h>
 
-// Allocate an MMDB_data_pool_s. It initially has space for size
-// MMDB_entry_data_list_s structs.
-MMDB_data_pool_s *data_pool_new(size_t const size) {
+// Allocate an MMDB_data_pool_s. It initially has space for up to size
+// MMDB_entry_data_list_s structs. Its total capacity will not exceed max_size.
+MMDB_data_pool_s *data_pool_new(size_t const size, size_t const max_size) {
     MMDB_data_pool_s *const pool = calloc(1, sizeof(MMDB_data_pool_s));
     if (!pool) {
         return NULL;
     }
 
-    if (size == 0 ||
-        !can_multiply(SIZE_MAX, size, sizeof(MMDB_entry_data_list_s))) {
+    if (size == 0 || max_size == 0) {
         data_pool_destroy(pool);
         return NULL;
     }
-    pool->size = size;
+    size_t initial_size = size;
+    if (initial_size > max_size) {
+        initial_size = max_size;
+    }
+    if (!can_multiply(SIZE_MAX, initial_size, sizeof(MMDB_entry_data_list_s))) {
+        data_pool_destroy(pool);
+        return NULL;
+    }
+    pool->size = initial_size;
     pool->blocks[0] = calloc(pool->size, sizeof(MMDB_entry_data_list_s));
     if (!pool->blocks[0]) {
         data_pool_destroy(pool);
         return NULL;
     }
     pool->blocks[0]->pool = pool;
 
-    pool->sizes[0] = size;
+    pool->sizes[0] = initial_size;
+    pool->capacity = initial_size;
+    pool->max_size = max_size;
 
     pool->block = pool->blocks[0];
 
@@ -62,8 +71,9 @@ void data_pool_destroy(MMDB_data_pool_s *const pool) {
     free(pool);
 }
 
-// Claim a new struct from the pool. Doing this may cause the pool's size to
-// grow.
+// Claim a new struct from the pool. Doing this may grow the pool. NULL means an
+// allocation failed or the pool reached its maximum capacity. Decoder callers
+// check their logical limit before reaching the capacity limit.
 MMDB_entry_data_list_s *data_pool_alloc(MMDB_data_pool_s *const pool) {
     if (!pool) {
         return NULL;
@@ -75,6 +85,10 @@ MMDB_entry_data_list_s *data_pool_alloc(MMDB_data_pool_s *const pool) {
         return element;
     }
 
+    if (pool->capacity == pool->max_size) {
+        return NULL;
+    }
+
     // Take it from a new block of memory.
 
     size_t const new_index = pool->index + 1;
@@ -83,10 +97,11 @@ MMDB_entry_data_list_s *data_pool_alloc(MMDB_data_pool_s *const pool) {
         return NULL;
     }
 
-    if (!can_multiply(SIZE_MAX, pool->size, 2)) {
-        return NULL;
+    size_t const remaining = pool->max_size - pool->capacity;
+    size_t new_size = remaining;
+    if (pool->size <= remaining / 2) {
+        new_size = pool->size * 2;
     }
-    size_t const new_size = pool->size * 2;
 
     if (!can_multiply(SIZE_MAX, new_size, sizeof(MMDB_entry_data_list_s))) {
         return NULL;
@@ -104,6 +119,7 @@ MMDB_entry_data_list_s *data_pool_alloc(MMDB_data_pool_s *const pool) {
 
     pool->size = new_size;
     pool->sizes[pool->index] = pool->size;
+    pool->capacity += new_size;
 
     MMDB_entry_data_list_s *const element = pool->block;
     pool->used = 1;

src/data-pool.h

@@ -6,12 +6,10 @@
 #include <stdbool.h>
 #include <stddef.h>
 
-// This should be large enough that we never need to grow the array of pointers
-// to blocks. 32 is enough. Even starting out of with size 1 (1 struct), the
-// 32nd element alone will provide 2**32 structs as we exponentially increase
-// the number in each block. Being confident that we do not have to grow the
-// array lets us avoid writing code to do that. That code would be risky as it
-// would rarely be hit and likely not be well tested.
+// Keep the block array fixed so that its own growth does not need a rarely used
+// reallocation path. Even starting with one struct, 32 geometrically growing
+// blocks cover every practical allocation; the last block may be clamped to the
+// configured capacity.
 #define DATA_POOL_NUM_BLOCKS 32
 
 // A pool of memory for MMDB_entry_data_list_s structs. This is so we can
@@ -33,6 +31,12 @@ typedef struct MMDB_data_pool_s {
     // How many used in the current block, counting by structs.
     size_t used;
 
+    // Total number of structs reserved across all blocks.
+    size_t capacity;
+
+    // Maximum total number of structs this pool may reserve.
+    size_t max_size;
+
     // The current block we're allocating out of.
     MMDB_entry_data_list_s *block;
 
@@ -45,7 +49,7 @@ typedef struct MMDB_data_pool_s {
 } MMDB_data_pool_s;
 
 bool can_multiply(size_t const, size_t const, size_t const);
-MMDB_data_pool_s *data_pool_new(size_t const);
+MMDB_data_pool_s *data_pool_new(size_t const, size_t const);
 void data_pool_destroy(MMDB_data_pool_s *const);
 MMDB_entry_data_list_s *data_pool_alloc(MMDB_data_pool_s *const);
 MMDB_entry_data_list_s *data_pool_to_list(MMDB_data_pool_s *const);