From 70d386296654f212275c42fb92602cc23a837504 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 2 Sep 2026 21:56:20 +0000 Subject: [PATCH 01/17] Bound decoded values to prevent a pointer fan-out denial of service A crafted data section could nest pointers to shared targets so that MMDB_get_entry_data_list decoded one entry with exponential time and memory from a small file (GHSA-hj94-g986-h9r7). The existing depth limit did not stop this: the blow-up comes from width (a shared graph re-walked), not from a single deep path. The decoder now counts the list nodes it allocates for one entry and returns the new MMDB_DECODER_LIMIT_ERROR when an entry needs more than 65,536, far above the few hundred values the largest records MaxMind produces decode. The limit can be raised when building the library with -DMAXIMUM_DATA_STRUCTURE_VALUES=. MMDB_open decodes the languages and description structures as complete lists, so it reports an over-limit structure as MMDB_INVALID_METADATA_ERROR. The data pool no longer doubles a block past the limit, which halves the memory an entry at the limit reserves. This follows the Reader Resource Limits guidance in the MaxMind DB specification. Co-Authored-By: Claude Opus 4.8 --- Changes.md | 12 +++++ doc/libmaxminddb.md | 28 ++++++++++- include/maxminddb.h | 1 + src/data-pool.c | 30 ++++++++---- src/data-pool.h | 8 +++- src/maxminddb.c | 113 ++++++++++++++++++++++++++++++++++---------- 6 files changed, 158 insertions(+), 34 deletions(-) diff --git a/Changes.md b/Changes.md index c4b2af66..c0bd2f8d 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,17 @@ ## 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. The + decoder now follows the proposed Reader Resource Limits guidance for the + MaxMind DB specification and stops at 65,536 values per entry. See the + `MMDB_get_entry_data_list()` documentation for details. + - Exceeding the limit returns the new `MMDB_DECODER_LIMIT_ERROR` status and + leaves the output list set to `NULL`. + - `MMDB_open()` returns `MMDB_INVALID_METADATA_ERROR` when the `languages` or + `description` metadata exceeds the limit. + - The limit can be raised when building the library with + `-DMAXIMUM_DATA_STRUCTURE_VALUES`. - 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 diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index 10ecc743..97c36306 100644 --- a/doc/libmaxminddb.md +++ b/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 an entry as a complete list would + exceed the configured value-count limit. The entry may still be valid + MaxMind DB data. All status codes should be treated as `int` values. @@ -452,6 +455,11 @@ 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`. +Opening a database decodes its `languages` and `description` metadata. If one +of these structures exceeds the decoder resource limit described under +`MMDB_get_entry_data_list()`, this function returns +`MMDB_INVALID_METADATA_ERROR`. + ## `MMDB_close()` ```c @@ -640,6 +648,22 @@ 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()`. +To bound the work produced by crafted databases, this function decodes at most +65,536 list values per call. A structure exactly at the limit is accepted. If a +structure exceeds the limit, the function returns `MMDB_DECODER_LIMIT_ERROR` +and sets `entry_data_list` to `NULL`. + +The limit is per call and may be changed when rebuilding libmaxminddb by +defining the positive integer macro `MAXIMUM_DATA_STRUCTURE_VALUES`. For +example, pass `-DMAXIMUM_DATA_STRUCTURE_VALUES=1000000` in the library's +compiler flags. This requires rebuilding the library itself; defining the 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 this budget. 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 +741,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 limit as `MMDB_get_entry_data_list()` and returns +`MMDB_DECODER_LIMIT_ERROR` if the complete metadata list exceeds it. ```c MMDB_entry_data_list_s *entry_data_list, *first; diff --git a/include/maxminddb.h b/include/maxminddb.h index 59f404db..ea0d9691 100644 --- a/include/maxminddb.h +++ b/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 diff --git a/src/data-pool.c b/src/data-pool.c index 3bc63286..321c5d18 100644 --- a/src/data-pool.c +++ b/src/data-pool.c @@ -9,16 +9,22 @@ #include #include -// 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 and will never reserve more than max_size. +MMDB_data_pool_s *data_pool_new(size_t 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; + } + if (size > max_size) { + size = max_size; + } + if (!can_multiply(SIZE_MAX, size, sizeof(MMDB_entry_data_list_s))) { data_pool_destroy(pool); return NULL; } @@ -31,6 +37,8 @@ MMDB_data_pool_s *data_pool_new(size_t const size) { pool->blocks[0]->pool = pool; pool->sizes[0] = size; + pool->capacity = size; + pool->max_size = max_size; pool->block = pool->blocks[0]; @@ -75,6 +83,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 +95,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 +117,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; diff --git a/src/data-pool.h b/src/data-pool.h index 9e61b768..7e779f04 100644 --- a/src/data-pool.h +++ b/src/data-pool.h @@ -33,6 +33,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 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 +51,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); diff --git a/src/maxminddb.c b/src/maxminddb.c index 8d82a9e7..7c9b38ca 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -35,6 +35,19 @@ typedef ADDRESS_FAMILY sa_family_t; #define MMDB_DATA_SECTION_SEPARATOR (16) #define MAXIMUM_DATA_STRUCTURE_DEPTH (512) +// The maximum number of data-section values decoded for a single entry. This +// bounds 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 this leaves a wide margin. See the proposed "Reader +// Resource Limits" guidance for the MaxMind DB specification. +#ifndef MAXIMUM_DATA_STRUCTURE_VALUES + #define MAXIMUM_DATA_STRUCTURE_VALUES (1U << 16) +#endif + +#if MAXIMUM_DATA_STRUCTURE_VALUES < 1 || \ + MAXIMUM_DATA_STRUCTURE_VALUES > SIZE_MAX + #error "MAXIMUM_DATA_STRUCTURE_VALUES must be between 1 and SIZE_MAX" +#endif #ifdef MMDB_DEBUG #define DEBUG_MSG(msg) fprintf(stderr, msg "\n") @@ -131,6 +144,10 @@ typedef struct record_info_s { uint8_t right_record_offset; } record_info_s; +typedef struct MMDB_decode_state_s { + size_t values; +} MMDB_decode_state_s; + #define METADATA_MARKER "\xab\xcd\xefMaxMind.com" /* This is 128kb */ #define METADATA_BLOCK_MAX_SIZE 131072 @@ -193,7 +210,12 @@ static int get_entry_data_list(const MMDB_s *const mmdb, uint32_t offset, MMDB_entry_data_list_s *const entry_data_list, MMDB_data_pool_s *const pool, + MMDB_decode_state_s *const decode_state, int depth); +static int +alloc_entry_data_list(MMDB_data_pool_s *const pool, + MMDB_decode_state_s *const decode_state, + MMDB_entry_data_list_s **const entry_data_list); static float get_ieee754_float(const uint8_t *restrict p); static double get_ieee754_double(const uint8_t *restrict p); static uint32_t get_uint32(const uint8_t *p); @@ -284,6 +306,11 @@ int MMDB_open(const char *const filename, uint32_t flags, MMDB_s *const mmdb) { mmdb->metadata_section_size = metadata_size; status = read_metadata(mmdb); + if (MMDB_DECODER_LIMIT_ERROR == status) { + // The languages and description structures are decoded as complete + // lists. Metadata that exceeds the decoder limits is invalid metadata. + status = MMDB_INVALID_METADATA_ERROR; + } if (MMDB_SUCCESS != status) { goto cleanup; } @@ -1688,19 +1715,23 @@ int MMDB_get_entry_data_list(MMDB_entry_s *start, MMDB_entry_data_list_s **const entry_data_list) { *entry_data_list = NULL; - MMDB_data_pool_s *const pool = data_pool_new(MMDB_POOL_INIT_SIZE); + size_t const maximum_values = (size_t)(MAXIMUM_DATA_STRUCTURE_VALUES); + MMDB_data_pool_s *const pool = + data_pool_new(MMDB_POOL_INIT_SIZE, maximum_values); if (!pool) { return MMDB_OUT_OF_MEMORY_ERROR; } - MMDB_entry_data_list_s *const list = data_pool_alloc(pool); - if (!list) { + MMDB_decode_state_s decode_state = {0}; + MMDB_entry_data_list_s *list = NULL; + int status = alloc_entry_data_list(pool, &decode_state, &list); + if (MMDB_SUCCESS != status) { data_pool_destroy(pool); - return MMDB_OUT_OF_MEMORY_ERROR; + return status; } - int const status = - get_entry_data_list(start->mmdb, start->offset, list, pool, 0); + status = get_entry_data_list( + start->mmdb, start->offset, list, pool, &decode_state, 0); if (MMDB_SUCCESS != status) { data_pool_destroy(pool); return status; @@ -1715,10 +1746,29 @@ int MMDB_get_entry_data_list(MMDB_entry_s *start, return status; } +static int +alloc_entry_data_list(MMDB_data_pool_s *const pool, + MMDB_decode_state_s *const decode_state, + MMDB_entry_data_list_s **const entry_data_list) { + size_t const maximum_values = (size_t)(MAXIMUM_DATA_STRUCTURE_VALUES); + if (decode_state->values >= maximum_values) { + DEBUG_MSG("reached the maximum number of data structure values"); + return MMDB_DECODER_LIMIT_ERROR; + } + + *entry_data_list = data_pool_alloc(pool); + if (!*entry_data_list) { + return MMDB_OUT_OF_MEMORY_ERROR; + } + decode_state->values++; + return MMDB_SUCCESS; +} + static int get_entry_data_list(const MMDB_s *const mmdb, uint32_t offset, MMDB_entry_data_list_s *const entry_data_list, MMDB_data_pool_s *const pool, + MMDB_decode_state_s *const decode_state, int depth) { if (depth >= MAXIMUM_DATA_STRUCTURE_DEPTH) { DEBUG_MSG("reached the maximum data structure depth"); @@ -1745,8 +1795,12 @@ static int get_entry_data_list(const MMDB_s *const mmdb, if (entry_data_list->entry_data.type == MMDB_DATA_TYPE_ARRAY || entry_data_list->entry_data.type == MMDB_DATA_TYPE_MAP) { - int status = get_entry_data_list( - mmdb, last_offset, entry_data_list, pool, depth); + int status = get_entry_data_list(mmdb, + last_offset, + entry_data_list, + pool, + decode_state, + depth); if (MMDB_SUCCESS != status) { DEBUG_MSG("get_entry_data_list on pointer failed."); return status; @@ -1764,14 +1818,19 @@ static int get_entry_data_list(const MMDB_s *const mmdb, return MMDB_INVALID_DATA_ERROR; } while (array_size-- > 0) { - MMDB_entry_data_list_s *entry_data_list_to = - data_pool_alloc(pool); - if (!entry_data_list_to) { - return MMDB_OUT_OF_MEMORY_ERROR; + MMDB_entry_data_list_s *entry_data_list_to = NULL; + int status = alloc_entry_data_list( + pool, decode_state, &entry_data_list_to); + if (MMDB_SUCCESS != status) { + return status; } - int status = get_entry_data_list( - mmdb, array_offset, entry_data_list_to, pool, depth); + status = get_entry_data_list(mmdb, + array_offset, + entry_data_list_to, + pool, + decode_state, + depth); if (MMDB_SUCCESS != status) { DEBUG_MSG("get_entry_data_list on array element failed."); return status; @@ -1793,13 +1852,15 @@ static int get_entry_data_list(const MMDB_s *const mmdb, return MMDB_INVALID_DATA_ERROR; } while (size-- > 0) { - MMDB_entry_data_list_s *list_key = data_pool_alloc(pool); - if (!list_key) { - return MMDB_OUT_OF_MEMORY_ERROR; + MMDB_entry_data_list_s *list_key = NULL; + int status = + alloc_entry_data_list(pool, decode_state, &list_key); + if (MMDB_SUCCESS != status) { + return status; } - int status = - get_entry_data_list(mmdb, offset, list_key, pool, depth); + status = get_entry_data_list( + mmdb, offset, list_key, pool, decode_state, depth); if (MMDB_SUCCESS != status) { DEBUG_MSG("get_entry_data_list on map key failed."); return status; @@ -1807,13 +1868,14 @@ static int get_entry_data_list(const MMDB_s *const mmdb, offset = list_key->entry_data.offset_to_next; - MMDB_entry_data_list_s *list_value = data_pool_alloc(pool); - if (!list_value) { - return MMDB_OUT_OF_MEMORY_ERROR; + MMDB_entry_data_list_s *list_value = NULL; + status = alloc_entry_data_list(pool, decode_state, &list_value); + if (MMDB_SUCCESS != status) { + return status; } - status = - get_entry_data_list(mmdb, offset, list_value, pool, depth); + status = get_entry_data_list( + mmdb, offset, list_value, pool, decode_state, depth); if (MMDB_SUCCESS != status) { DEBUG_MSG("get_entry_data_list on map element failed."); return status; @@ -2286,6 +2348,9 @@ const char *MMDB_strerror(int error_code) { case MMDB_INVALID_NETWORK_ADDRESS_ERROR: return "The sockaddr family is unsupported; only AF_INET and " "AF_INET6 are accepted"; + case MMDB_DECODER_LIMIT_ERROR: + return "The decoded data structure exceeds the configured resource " + "limits"; default: return "Unknown error code"; } From 3d8b5ec67da48a53d9a20827659f1893a077a9eb Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 2 Sep 2026 21:56:28 +0000 Subject: [PATCH 02/17] Bound the total payload decoded for a single entry The value-count limit bounds how many nodes MMDB_get_entry_data_list produces, but not how many bytes they reference. libmaxminddb borrows payload bytes rather than copying them, so the node count alone bounds the library's own memory. A crafted database can still point many times at one large value, producing a bounded node list that together references far more bytes than the file holds. A caller that copies each node into a string then materializes that amplified total, for example about 512 MiB from an 82 KiB file. Charge the total string and bytes payload decoded for a single entry against a per-entry byte budget and return MMDB_DECODER_LIMIT_ERROR when it would exceed MAXIMUM_DATA_STRUCTURE_BYTES (2 MiB, overridable at build time with -DMAXIMUM_DATA_STRUCTURE_BYTES=). The check runs before the add, so the uint64 total cannot wrap even under a raised limit. Integers are size-validated and tiny, floats are fixed width, and container sizes are element counts, so only string and bytes payloads are charged. This also rejects a rare format-valid record whose own string and bytes fields exceed the limit. The largest records MaxMind produces hold about a kilobyte of payload, so the limit leaves a wide margin while stopping the amplification for every caller of the API. See GHSA-hj94-g986-h9r7. Co-Authored-By: Claude Opus 4.8 --- Changes.md | 16 +++++++++------- doc/libmaxminddb.md | 42 +++++++++++++++++++++++------------------- src/maxminddb.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/Changes.md b/Changes.md index c0bd2f8d..50b84913 100644 --- a/Changes.md +++ b/Changes.md @@ -2,16 +2,18 @@ - 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. The - decoder now follows the proposed Reader Resource Limits guidance for the - MaxMind DB specification and stops at 65,536 values per entry. See the + 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 proposed Reader + Resource Limits guidance for the MaxMind DB specification and stops at 65,536 + values or 2 MiB of string and bytes payload per entry. See the `MMDB_get_entry_data_list()` documentation for details. - - Exceeding the limit returns the new `MMDB_DECODER_LIMIT_ERROR` status and + - Exceeding a limit returns the new `MMDB_DECODER_LIMIT_ERROR` status and leaves the output list set to `NULL`. - `MMDB_open()` returns `MMDB_INVALID_METADATA_ERROR` when the `languages` or - `description` metadata exceeds the limit. - - The limit can be raised when building the library with - `-DMAXIMUM_DATA_STRUCTURE_VALUES`. + `description` metadata exceeds a limit. + - The limits can be raised when building the library with + `-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 diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index 97c36306..26086f22 100644 --- a/doc/libmaxminddb.md +++ b/doc/libmaxminddb.md @@ -395,9 +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 an entry as a complete list would - exceed the configured value-count limit. The entry may still be valid - MaxMind DB data. +- `MMDB_DECODER_LIMIT_ERROR` - decoding an entry as a complete list would exceed + the configured value-count or string/bytes payload limit. The entry may still + be valid MaxMind DB data. All status codes should be treated as `int` values. @@ -455,8 +455,8 @@ 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`. -Opening a database decodes its `languages` and `description` metadata. If one -of these structures exceeds the decoder resource limit described under +Opening a database decodes its `languages` and `description` metadata. If one of +these structures exceeds the decoder resource limits described under `MMDB_get_entry_data_list()`, this function returns `MMDB_INVALID_METADATA_ERROR`. @@ -648,20 +648,24 @@ 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()`. -To bound the work produced by crafted databases, this function decodes at most -65,536 list values per call. A structure exactly at the limit is accepted. If a -structure exceeds the limit, the function returns `MMDB_DECODER_LIMIT_ERROR` -and sets `entry_data_list` to `NULL`. - -The limit is per call and may be changed when rebuilding libmaxminddb by -defining the positive integer macro `MAXIMUM_DATA_STRUCTURE_VALUES`. For -example, pass `-DMAXIMUM_DATA_STRUCTURE_VALUES=1000000` in the library's -compiler flags. This requires rebuilding the library itself; defining the macro -only while building an application does not change a packaged shared library. +A crafted database can make a full decode expensive, so this function bounds the +work and the caller-visible payload. It decodes at most 65,536 list values and +at most 2 MiB of UTF-8 string and bytes payload per call. A structure exactly at +either limit is accepted. If a structure exceeds either limit, the function +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_VALUES` and +`MAXIMUM_DATA_STRUCTURE_BYTES`. For example, pass +`-DMAXIMUM_DATA_STRUCTURE_BYTES=3145728` in the library's compiler flags. Give +the value as a plain integer. The compiler evaluates an expression such as +`1<<31` as `int`, which overflows above 2,147,483,647. This requires rebuilding +the library itself. Defining the 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 this budget. Applications that -cannot rebuild a packaged library can use those functions to retrieve a +complete structure and therefore do not charge these two budgets. Applications +that cannot rebuild a packaged library can use those functions to retrieve a specific field from an otherwise over-limit record. ```c @@ -742,8 +746,8 @@ 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. It uses the same -per-call limit as `MMDB_get_entry_data_list()` and returns -`MMDB_DECODER_LIMIT_ERROR` if the complete metadata list exceeds it. +per-call limits as `MMDB_get_entry_data_list()` and returns +`MMDB_DECODER_LIMIT_ERROR` if the complete metadata list exceeds either one. ```c MMDB_entry_data_list_s *entry_data_list, *first; diff --git a/src/maxminddb.c b/src/maxminddb.c index 7c9b38ca..c19b7d99 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -49,6 +49,23 @@ typedef ADDRESS_FAMILY sa_family_t; #error "MAXIMUM_DATA_STRUCTURE_VALUES must be between 1 and SIZE_MAX" #endif +// The maximum total bytes of string and bytes payloads decoded for a single +// entry. libmaxminddb borrows payload bytes (each node points into the data +// section, it does not copy), so the value count above already bounds the +// library's own memory. But a fan-out of pointers to one large value produces +// many nodes that all reference it. A caller that copies each node into a +// language string then materializes far more than the file holds. This bounds +// that copied total. The largest real records hold about a kilobyte of +// payload, so 2 MiB leaves a wide margin while stopping the amplification. It +// can be raised at build time with -DMAXIMUM_DATA_STRUCTURE_BYTES=. +#ifndef MAXIMUM_DATA_STRUCTURE_BYTES + #define MAXIMUM_DATA_STRUCTURE_BYTES (1U << 21) +#endif + +#if MAXIMUM_DATA_STRUCTURE_BYTES < 1 + #error "MAXIMUM_DATA_STRUCTURE_BYTES must be at least 1" +#endif + #ifdef MMDB_DEBUG #define DEBUG_MSG(msg) fprintf(stderr, msg "\n") #define DEBUG_MSGF(fmt, ...) fprintf(stderr, fmt "\n", __VA_ARGS__) @@ -146,6 +163,7 @@ typedef struct record_info_s { typedef struct MMDB_decode_state_s { size_t values; + uint64_t bytes; } MMDB_decode_state_s; #define METADATA_MARKER "\xab\xcd\xefMaxMind.com" @@ -1888,6 +1906,26 @@ static int get_entry_data_list(const MMDB_s *const mmdb, break; } + // Charge the copied payload. Only string and bytes carry a variable-length + // payload that a caller copies. Integers are size-validated and tiny, + // floats are fixed width, and container data_size is an element count, not + // bytes. Pointers have been resolved to their target above, so a pointer to + // a string is charged here as the string. This runs once per node, so a + // fan-out that references one large value many times is charged each time. + // Check before adding so even an overridden maximum cannot make the + // uint64 counter wrap. + if (entry_data_list->entry_data.type == MMDB_DATA_TYPE_UTF8_STRING || + entry_data_list->entry_data.type == MMDB_DATA_TYPE_BYTES) { + uint64_t const maximum_bytes = (uint64_t)(MAXIMUM_DATA_STRUCTURE_BYTES); + uint64_t const data_size = entry_data_list->entry_data.data_size; + if (data_size > maximum_bytes || + decode_state->bytes > maximum_bytes - data_size) { + DEBUG_MSG("reached the maximum data structure bytes"); + return MMDB_DECODER_LIMIT_ERROR; + } + decode_state->bytes += data_size; + } + return MMDB_SUCCESS; } From 044c3c21d2883a1f1470469c61387149fc811d91 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 2 Sep 2026 21:56:35 +0000 Subject: [PATCH 03/17] Update the test-data submodule for the pointer DoS fixtures Bump t/maxmind-db to the merged MaxMind-DB commit that adds the pointer fan-out, payload amplification, and limit boundary fixtures, so the new regression tests can use them. Co-Authored-By: Claude Opus 4.8 --- t/maxmind-db | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/maxmind-db b/t/maxmind-db index e7b00186..363086b7 160000 --- a/t/maxmind-db +++ b/t/maxmind-db @@ -1 +1 @@ -Subproject commit e7b0018644317ad6f33eb408f4479ccc4ab0e6fd +Subproject commit 363086b7d90650100e91f954937794c6a090c2a0 From b7dc5ab98f4df254473e737f3471fbaacd757028 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 2 Sep 2026 21:56:36 +0000 Subject: [PATCH 04/17] Add regression tests for the decoder resource limits Exercise MMDB_get_entry_data_list against the coordinated fixtures. The value-count fan-out, the payload amplification, and its worst case under the value-count limit are each rejected with MMDB_DECODER_LIMIT_ERROR and leave a NULL output list. The boundary fixtures decode at each limit and are rejected one past it. A normal record still decodes, confirming no false rejection, a rejected decode does not affect a later one, confirming the counters are per call, and MMDB_get_value still reads a field from an over-limit record. MMDB_open reports over-limit metadata as MMDB_INVALID_METADATA_ERROR. decoder_limits_t.pl compiles the library with overridden limits. It checks that valid overrides compile without warnings, that invalid ones fail with a message naming the range, and that an override takes effect at runtime. The data pool tests cover the new maximum size: the initial block is clamped to it, growth stops at it, and an allocation past it is refused. Co-Authored-By: Claude Opus 4.8 --- t/CMakeLists.txt | 1 + t/Makefile.am | 8 +- t/data-pool-t.c | 64 +++++++++-- t/decoder_limits_t.pl | 239 ++++++++++++++++++++++++++++++++++++++++++ t/pointer_dos_t.c | 238 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 537 insertions(+), 13 deletions(-) create mode 100755 t/decoder_limits_t.pl create mode 100644 t/pointer_dos_t.c diff --git a/t/CMakeLists.txt b/t/CMakeLists.txt index 04627b60..bb9c23c7 100644 --- a/t/CMakeLists.txt +++ b/t/CMakeLists.txt @@ -22,6 +22,7 @@ set(TEST_TARGET_NAMES metadata_t no_map_get_value_t overflow_bounds_t + pointer_dos_t read_node_t version_t ) diff --git a/t/Makefile.am b/t/Makefile.am index 630c664c..f2c01aca 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -11,7 +11,8 @@ CFLAGS += -I$(top_srcdir)/src noinst_LTLIBRARIES = libmmdbtest.la libmmdbtest_la_SOURCES = maxminddb_test_helper.c maxminddb_test_helper.h -EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ +EXTRA_DIST = compile_c++_t.pl decoder_limits_t.pl external_symbols_t.pl \ + mmdblookup_t.pl \ libtap/COPYING libtap/INSTALL libtap/Makefile libtap/README.md \ libtap/tap.c libtap/tap.h maxmind-db @@ -24,7 +25,7 @@ check_PROGRAMS = \ get_value_pointer_bug_t invalid_sockaddr_t \ ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ metadata_marker_t metadata_pointers_t no_map_get_value_t \ - overflow_bounds_t read_node_t \ + overflow_bounds_t pointer_dos_t read_node_t \ threads_t version_t data_pool_t_LDFLAGS = $(AM_LDFLAGS) -lm @@ -32,6 +33,7 @@ data_pool_t_SOURCES = data-pool-t.c ../src/data-pool.c threads_t_CFLAGS = $(CFLAGS) -pthread -TESTS = $(check_PROGRAMS) compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl +TESTS = $(check_PROGRAMS) compile_c++_t.pl decoder_limits_t.pl \ + external_symbols_t.pl mmdblookup_t.pl LDADD = libmmdbtest.la libtap/libtap.a diff --git a/t/data-pool-t.c b/t/data-pool-t.c index 6952c035..0b2f48ca 100644 --- a/t/data-pool-t.c +++ b/t/data-pool-t.c @@ -24,20 +24,30 @@ int main(void) { static void test_data_pool_new(void) { { - MMDB_data_pool_s *const pool = data_pool_new(0); + MMDB_data_pool_s *const pool = data_pool_new(0, 512); ok(!pool, "size 0 is not valid"); } { - MMDB_data_pool_s *const pool = data_pool_new(SIZE_MAX - 10); + MMDB_data_pool_s *const pool = data_pool_new(SIZE_MAX - 10, SIZE_MAX); ok(!pool, "very large size is not valid"); } { - MMDB_data_pool_s *const pool = data_pool_new(512); + MMDB_data_pool_s *const pool = data_pool_new(512, 1024); ok(pool != NULL, "size 512 is valid"); cmp_ok(pool->size, "==", 512, "size is 512"); cmp_ok(pool->used, "==", 0, "used size is 0"); + cmp_ok(pool->capacity, "==", 512, "capacity is 512"); + cmp_ok(pool->max_size, "==", 1024, "maximum size is 1024"); + data_pool_destroy(pool); + } + + { + MMDB_data_pool_s *const pool = data_pool_new(512, 10); + ok(pool != NULL, "maximum smaller than initial size is valid"); + cmp_ok(pool->size, "==", 10, "initial size is clamped to maximum"); + cmp_ok(pool->capacity, "==", 10, "capacity is clamped to maximum"); data_pool_destroy(pool); } } @@ -48,7 +58,7 @@ static void test_data_pool_destroy(void) { } { - MMDB_data_pool_s *const pool = data_pool_new(512); + MMDB_data_pool_s *const pool = data_pool_new(512, 512); ok(pool != NULL, "created pool"); data_pool_destroy(pool); } @@ -56,7 +66,7 @@ static void test_data_pool_destroy(void) { static void test_data_pool_alloc(void) { { - MMDB_data_pool_s *const pool = data_pool_new(1); + MMDB_data_pool_s *const pool = data_pool_new(1, 3); ok(pool != NULL, "created pool"); cmp_ok(pool->used, "==", 0, "used size starts at 0"); @@ -75,6 +85,12 @@ static void test_data_pool_alloc(void) { cmp_ok(pool->size, "==", 2, "size is 2 (new block)"); cmp_ok(pool->used, "==", 1, "used size is 1 in current block"); + MMDB_entry_data_list_s *const entry3 = data_pool_alloc(pool); + ok(entry3 != NULL, "got the final allowed entry"); + ok(data_pool_alloc(pool) == NULL, + "allocation past maximum capacity is rejected"); + cmp_ok(pool->capacity, "==", 3, "capacity does not exceed maximum"); + ok(entry1->entry_data.offset == 123, "accessing the original entry's memory is ok"); @@ -83,7 +99,8 @@ static void test_data_pool_alloc(void) { { size_t const initial_size = 10; - MMDB_data_pool_s *const pool = data_pool_new(initial_size); + MMDB_data_pool_s *const pool = + data_pool_new(initial_size, initial_size * 3); ok(pool != NULL, "created pool"); MMDB_entry_data_list_s *entry1 = NULL; @@ -124,12 +141,33 @@ static void test_data_pool_alloc(void) { data_pool_destroy(pool); } + + { + size_t const maximum_size = 65536; + MMDB_data_pool_s *const pool = data_pool_new(64, maximum_size); + ok(pool != NULL, "created a decoder-sized pool"); + for (size_t i = 0; i < maximum_size; i++) { + assert(data_pool_alloc(pool) != NULL); + } + cmp_ok(pool->capacity, + "==", + maximum_size, + "final block is clamped to the remaining capacity"); + cmp_ok(pool->sizes[pool->index], + "==", + 64, + "the clamped final block reserves only 64 entries"); + ok(data_pool_alloc(pool) == NULL, + "decoder-sized pool refuses a 65,537th entry"); + data_pool_destroy(pool); + } } static void test_data_pool_to_list(void) { { size_t const initial_size = 16; - MMDB_data_pool_s *const pool = data_pool_new(initial_size); + MMDB_data_pool_s *const pool = + data_pool_new(initial_size, initial_size); ok(pool != NULL, "created pool"); MMDB_entry_data_list_s *const entry1 = data_pool_alloc(pool); @@ -162,7 +200,8 @@ static void test_data_pool_to_list(void) { { size_t const initial_size = 1; - MMDB_data_pool_s *const pool = data_pool_new(initial_size); + MMDB_data_pool_s *const pool = + data_pool_new(initial_size, initial_size); ok(pool != NULL, "created pool"); MMDB_entry_data_list_s *const entry1 = data_pool_alloc(pool); @@ -180,7 +219,8 @@ static void test_data_pool_to_list(void) { { size_t const initial_size = 2; - MMDB_data_pool_s *const pool = data_pool_new(initial_size); + MMDB_data_pool_s *const pool = + data_pool_new(initial_size, initial_size); ok(pool != NULL, "created pool"); MMDB_entry_data_list_s *const entry1 = data_pool_alloc(pool); @@ -271,7 +311,11 @@ static void test_data_pool_to_list(void) { // this frequently. static bool create_and_check_list(size_t const initial_size, size_t const element_count) { - MMDB_data_pool_s *const pool = data_pool_new(initial_size); + size_t max_size = initial_size; + if (element_count > initial_size) { + max_size = element_count; + } + MMDB_data_pool_s *const pool = data_pool_new(initial_size, max_size); assert(pool != NULL); assert(pool->used == 0); diff --git a/t/decoder_limits_t.pl b/t/decoder_limits_t.pl new file mode 100755 index 00000000..65123ab1 --- /dev/null +++ b/t/decoder_limits_t.pl @@ -0,0 +1,239 @@ +#!/usr/bin/env perl + +use strict; +use warnings; + +use Cwd qw( abs_path ); +use FindBin qw( $Bin ); + +eval <<'EOF'; +use Test::More 0.88; +use File::Temp qw( tempdir ); +use IPC::Run3 qw( run3 ); +EOF + +if ($@) { + print + "1..0 # skip decoder limit override tests need Test::More 0.88, File::Temp, and IPC::Run3\n"; + exit 0; +} + +my $root = abs_path("$Bin/.."); +my $include_dir = "$root/include"; +my $src_dir = "$root/src"; +my $cc = $ENV{CC} || 'cc'; + +# The checks below rebuild the library with -Werror. Only gcc and clang are +# known to compile it cleanly with the flags used here, so skip elsewhere +# instead of failing on a missing compiler or an unrelated warning. +my $cc_version = `$cc --version 2>&1`; +if ( $? != 0 || $cc_version !~ /gcc|clang|Free Software Foundation/ ) { + plan( skip_all => "decoder limit override tests need gcc or clang" ); +} + +# Keep instrumentation such as -fsanitize=address from the environment, but +# not its warning flags. Those vary by CI job and would trip -Werror below. +my @instrumentation = grep { /^-f/ } + map { split ' ' } grep { defined } @ENV{ 'CFLAGS', 'LDFLAGS' }; + +my @base = ( + $cc, + @instrumentation, + '-std=c99', + '-Wall', + '-Wextra', + '-Werror', + '-Wno-unused-function', + '-Wno-unused-parameter', + '-DPACKAGE_VERSION="test"', + "-I$include_dir", + "-I$src_dir", +); + +for my $definition ('-DMAXIMUM_DATA_STRUCTURE_VALUES=1000000') { + my ( $status, $stderr ) = _run( + @base, + $definition, + '-fsyntax-only', + "$src_dir/maxminddb.c", + ); + is( $status, 0, "$definition compiles without warnings" ) + or diag($stderr); +} + +for my $definition ( + '-DMAXIMUM_DATA_STRUCTURE_VALUES=0', + '-DMAXIMUM_DATA_STRUCTURE_VALUES=-1', + '-DMAXIMUM_DATA_STRUCTURE_VALUES=SIZE_MAX+1', + '-DMAXIMUM_DATA_STRUCTURE_BYTES=0', + '-DMAXIMUM_DATA_STRUCTURE_BYTES=-1', +) { + my ( $status, $stderr ) = _run( + @base, + $definition, + '-fsyntax-only', + "$src_dir/maxminddb.c", + ); + isnt( $status, 0, "$definition is rejected" ); + like( + $stderr, + qr/MAXIMUM_DATA_STRUCTURE_\w+ must be/, + "$definition explains its range" + ); +} + +my $tempdir = tempdir( CLEANUP => 1 ); +my $source = "$tempdir/override.c"; +open my $fh, '>', $source or die $!; +print {$fh} <<'EOF' or die $!; +#include +#include +#include + +static int fail(const char *path, const char *what, int status, int code) { + fprintf(stderr, "%s: %s: %s\n", path, what, MMDB_strerror(status)); + return code; +} + +static int lookup(const char *path, MMDB_s *mmdb, MMDB_lookup_result_s *result) { + int status = MMDB_open(path, MMDB_MODE_MMAP, mmdb); + if (status != MMDB_SUCCESS) { + return fail(path, "open", status, 1); + } + int gai_error, mmdb_error; + *result = MMDB_lookup_string(mmdb, "1.1.1.1", &gai_error, &mmdb_error); + if (gai_error != 0 || mmdb_error != MMDB_SUCCESS || !result->found_entry) { + MMDB_close(mmdb); + return fail(path, "lookup of 1.1.1.1 found no entry", mmdb_error, 2); + } + return 0; +} + +static int decode(const char *path, size_t expected_count) { + MMDB_s mmdb; + MMDB_lookup_result_s result; + int code = lookup(path, &mmdb, &result); + if (code != 0) { + return code; + } + MMDB_entry_data_list_s *list = NULL; + int status = MMDB_get_entry_data_list(&result.entry, &list); + if (status != MMDB_SUCCESS) { + MMDB_close(&mmdb); + return fail(path, "full decode", status, 3); + } + size_t count = 0; + for (MMDB_entry_data_list_s *node = list; node; node = node->next) { + count++; + } + MMDB_free_entry_data_list(list); + MMDB_close(&mmdb); + if (count != expected_count) { + fprintf(stderr, "%s: decoded %zu values, expected %zu\n", path, count, + expected_count); + return 4; + } + return 0; +} + +static int reject(const char *path) { + MMDB_s mmdb; + MMDB_lookup_result_s result; + int code = lookup(path, &mmdb, &result); + if (code != 0) { + return code; + } + MMDB_entry_data_list_s *list = NULL; + int status = MMDB_get_entry_data_list(&result.entry, &list); + MMDB_free_entry_data_list(list); + MMDB_close(&mmdb); + if (status != MMDB_DECODER_LIMIT_ERROR) { + return fail(path, "full decode was not rejected", status, 7); + } + return 0; +} + +int main(int argc, char **argv) { + if (argc == 2) { + return reject(argv[1]); + } + if (argc != 3) { + return 5; + } + int status = decode(argv[1], 65537); + return status == 0 ? decode(argv[2], 34) : status; +} +EOF +close $fh or die $!; + +# Each override rebuilds the library with the given definitions, then runs the +# program above. Two fixtures means decode both and check the value counts; +# one fixture means expect MMDB_DECODER_LIMIT_ERROR. +my @overrides = ( + { + desc => 'limits one above the defaults', + definitions => [ + '-DMAXIMUM_DATA_STRUCTURE_VALUES=65537', + '-DMAXIMUM_DATA_STRUCTURE_BYTES=2097153', + ], + fixtures => [ + 'MaxMind-DB-test-decoder-value-limit-over.mmdb', + 'MaxMind-DB-test-decoder-payload-limit-over.mmdb', + ], + }, + { + desc => 'a 2 GiB payload limit', + definitions => ['-DMAXIMUM_DATA_STRUCTURE_BYTES=2147483648'], + fixtures => + ['MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb'], + }, + { + # The fixture's first value is 65,535 bytes, one more than the limit, + # so the single-value check rejects it before any total accumulates. + desc => 'a payload limit below a single value', + definitions => ['-DMAXIMUM_DATA_STRUCTURE_BYTES=65534'], + fixtures => ['MaxMind-DB-test-payload-amplification-dos.mmdb'], + }, +); + +my $count = 0; +for my $override (@overrides) { + my $executable = "$tempdir/override-" . $count++; + my ( $compile_status, $compile_stderr ) = _run( + @base, + @{ $override->{definitions} }, + "$src_dir/maxminddb.c", + "$src_dir/data-pool.c", + $source, + '-lm', + '-o', + $executable, + ); + is( $compile_status, 0, "$override->{desc} compiles and links" ) + or diag($compile_stderr); + next if $compile_status != 0; + + my ( $status, $stderr ) = _run( + $executable, + map { "$Bin/maxmind-db/test-data/$_" } @{ $override->{fixtures} }, + ); + is( $status, 0, "$override->{desc} takes effect at runtime" ) + or diag($stderr); +} + +done_testing(); + +sub _run { + my @command = @_; + my ( $stdout, $stderr ); + run3( \@command, \undef, \$stdout, \$stderr ); + my $wait = $?; + + # A child killed by a signal has no exit code, so report the signal as a + # failure instead of letting $? >> 8 read as success. + if ( $wait & 127 ) { + my $signal = $wait & 127; + return ( 128 + $signal, "$stderr\nkilled by signal $signal\n" ); + } + return ( $wait >> 8, $stderr ); +} diff --git a/t/pointer_dos_t.c b/t/pointer_dos_t.c new file mode 100644 index 00000000..2d77e7c1 --- /dev/null +++ b/t/pointer_dos_t.c @@ -0,0 +1,238 @@ +#include "maxminddb_test_helper.h" + +static void test_record_rejected(const char *fixture, + const char *address, + const char *desc) { + char *path = test_database_path(fixture); + MMDB_s *mmdb = open_ok(path, MMDB_MODE_MMAP, desc); + free(path); + if (!mmdb) { + return; + } + + MMDB_lookup_result_s result = + lookup_string_ok(mmdb, address, fixture, desc); + ok(result.found_entry, "%s: entry found", desc); + if (result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + int const status = + MMDB_get_entry_data_list(&result.entry, &entry_data_list); + cmp_ok(status, + "==", + MMDB_DECODER_LIMIT_ERROR, + "%s: full decode returns MMDB_DECODER_LIMIT_ERROR", + desc); + ok(entry_data_list == NULL, + "%s: error leaves the output list set to NULL", + desc); + MMDB_free_entry_data_list(entry_data_list); + } + + MMDB_close(mmdb); + free(mmdb); +} + +static void test_record_allowed(const char *fixture, + const char *address, + size_t expected_values, + uint64_t expected_payload, + const char *desc) { + char *path = test_database_path(fixture); + MMDB_s *mmdb = open_ok(path, MMDB_MODE_MMAP, desc); + free(path); + if (!mmdb) { + return; + } + + MMDB_lookup_result_s result = + lookup_string_ok(mmdb, address, fixture, desc); + ok(result.found_entry, "%s: entry found", desc); + if (result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + int const status = + MMDB_get_entry_data_list(&result.entry, &entry_data_list); + cmp_ok(status, "==", MMDB_SUCCESS, "%s: full decode succeeds", desc); + ok(entry_data_list != NULL, "%s: full decode returns a list", desc); + + size_t values = 0; + uint64_t payload = 0; + for (MMDB_entry_data_list_s *node = entry_data_list; node; + node = node->next) { + values++; + if (node->entry_data.type == MMDB_DATA_TYPE_UTF8_STRING || + node->entry_data.type == MMDB_DATA_TYPE_BYTES) { + payload += node->entry_data.data_size; + } + } + cmp_ok(values, + "==", + expected_values, + "%s: decoded the expected number of values", + desc); + cmp_ok(payload, + "==", + expected_payload, + "%s: decoded the expected payload bytes", + desc); + MMDB_free_entry_data_list(entry_data_list); + } + + MMDB_close(mmdb); + free(mmdb); +} + +static void test_per_call_state(void) { + const char *fixture = "MaxMind-DB-test-payload-amplification-dos.mmdb"; + char *path = test_database_path(fixture); + MMDB_s *mmdb = open_ok(path, MMDB_MODE_MMAP, "per-call state"); + free(path); + if (!mmdb) { + return; + } + + MMDB_lookup_result_s result = + lookup_string_ok(mmdb, "1.1.1.1", fixture, "per-call state"); + ok(result.found_entry, "per-call state: entry found"); + if (result.found_entry) { + for (int i = 1; i <= 2; i++) { + MMDB_entry_data_list_s *list = NULL; + int const status = MMDB_get_entry_data_list(&result.entry, &list); + cmp_ok(status, + "==", + MMDB_DECODER_LIMIT_ERROR, + "per-call: attack decode %d is rejected", + i); + ok(list == NULL, + "per-call: attack decode %d leaves a NULL list", + i); + MMDB_free_entry_data_list(list); + } + + MMDB_entry_data_list_s *metadata = NULL; + int const status = + MMDB_get_metadata_as_entry_data_list(mmdb, &metadata); + cmp_ok(status, + "==", + MMDB_SUCCESS, + "per-call: metadata decode on the same reader succeeds"); + ok(metadata != NULL, + "per-call: metadata decode on the same reader returns a list"); + MMDB_free_entry_data_list(metadata); + } + + MMDB_close(mmdb); + free(mmdb); +} + +static void test_targeted_lookup_bypasses_full_decode_limit(void) { + const char *fixture = "MaxMind-DB-test-decoder-payload-limit-over.mmdb"; + const char *desc = "targeted oversized lookup"; + char *path = test_database_path(fixture); + MMDB_s *mmdb = open_ok(path, MMDB_MODE_MMAP, desc); + free(path); + if (!mmdb) { + return; + } + + MMDB_lookup_result_s result = + lookup_string_ok(mmdb, "1.1.1.1", fixture, desc); + ok(result.found_entry, "%s: entry found", desc); + if (result.found_entry) { + MMDB_entry_data_s entry_data; + int const status = + MMDB_get_value(&result.entry, &entry_data, "0", NULL); + cmp_ok(status, + "==", + MMDB_SUCCESS, + "targeted lookup succeeds without expanding the structure"); + if (status == MMDB_SUCCESS) { + ok(entry_data.has_data, "targeted lookup returns data"); + cmp_ok(entry_data.type, + "==", + MMDB_DATA_TYPE_BYTES, + "targeted lookup returns the bytes value"); + cmp_ok(entry_data.data_size, + "==", + 65535, + "targeted lookup returns the complete bytes value"); + } + } + + MMDB_close(mmdb); + free(mmdb); +} + +static void test_metadata_limit_error(void) { + char *db_file = + test_database_path("MaxMind-DB-test-metadata-payload-limit.mmdb"); + MMDB_s mmdb; + int const status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, + "==", + MMDB_INVALID_METADATA_ERROR, + "metadata decoder limit is reported as invalid metadata by open"); + if (status == MMDB_SUCCESS) { + MMDB_close(&mmdb); + } + free(db_file); +} + +int main(void) { + plan(NO_PLAN); + + is(MMDB_strerror(MMDB_DECODER_LIMIT_ERROR), + "The decoded data structure exceeds the configured resource limits", + "decoder limit status has a distinct error message"); + + test_record_rejected("MaxMind-DB-test-pointer-decoder-dos.mmdb", + "1.1.1.1", + "IPv4 value-count fan-out"); + test_record_rejected("MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb", + "2001:db8::1", + "IPv6 value-count fan-out"); + test_record_rejected("MaxMind-DB-test-payload-amplification-dos.mmdb", + "1.1.1.1", + "bytes payload amplification"); + test_record_rejected( + "MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb", + "1.1.1.1", + "worst-case bytes payload amplification"); + test_record_rejected( + "MaxMind-DB-test-payload-amplification-dos-string.mmdb", + "1.1.1.1", + "string payload amplification"); + + test_record_allowed("MaxMind-DB-test-decoder-value-limit.mmdb", + "1.1.1.1", + 65536, + 0, + "exact value-count limit"); + test_record_allowed( + "MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb", + "1.1.1.1", + 65535, + 0, + "pointer-heavy record under the value-count limit"); + test_record_rejected("MaxMind-DB-test-decoder-value-limit-over.mmdb", + "1.1.1.1", + "one over the value-count limit"); + test_record_allowed("MaxMind-DB-test-decoder-payload-limit.mmdb", + "1.1.1.1", + 34, + 2097152, + "exact payload-byte limit"); + test_record_rejected("MaxMind-DB-test-decoder-payload-limit-over.mmdb", + "1.1.1.1", + "one over the payload-byte limit"); + + test_record_allowed("GeoIP2-City-Test.mmdb", + "81.2.69.142", + 120, + 679, + "normal production-style record"); + test_per_call_state(); + test_targeted_lookup_bypasses_full_decode_limit(); + test_metadata_limit_error(); + + done_testing(); +} From 27d69978138ceda8d43ee882cf01f246129f4464 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 2 Sep 2026 21:56:54 +0000 Subject: [PATCH 05/17] Add braces to the single-line if statements in the fuzz harness The rest of the code base braces every if body. No functional change. Co-Authored-By: Claude Opus 4.8 --- t/fuzz_mmdb.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/t/fuzz_mmdb.c b/t/fuzz_mmdb.c index e9289431..c6492e83 100644 --- a/t/fuzz_mmdb.c +++ b/t/fuzz_mmdb.c @@ -13,21 +13,24 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { MMDB_s mmdb; char filename[256]; - if (size < kMinInputLength || size > kMaxInputLength) + if (size < kMinInputLength || size > kMaxInputLength) { return 0; + } sprintf(filename, "/tmp/libfuzzer.%d", getpid()); fp = fopen(filename, "wb"); - if (!fp) + if (!fp) { return 0; + } fwrite(data, size, sizeof(uint8_t), fp); fclose(fp); status = MMDB_open(filename, MMDB_MODE_MMAP, &mmdb); - if (status == MMDB_SUCCESS) + if (status == MMDB_SUCCESS) { MMDB_close(&mmdb); + } unlink(filename); return 0; From 12537f37f50a029c7784edf353922f7a1242c29d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 2 Sep 2026 21:57:10 +0000 Subject: [PATCH 06/17] Use snprintf for the fuzz harness file name The buffer holds the fixed prefix and any pid, so this does not change behavior. It removes the unbounded write that static analysis flags. Co-Authored-By: Claude Opus 4.8 --- t/fuzz_mmdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/fuzz_mmdb.c b/t/fuzz_mmdb.c index c6492e83..c936dc3c 100644 --- a/t/fuzz_mmdb.c +++ b/t/fuzz_mmdb.c @@ -17,7 +17,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { return 0; } - sprintf(filename, "/tmp/libfuzzer.%d", getpid()); + snprintf(filename, sizeof(filename), "/tmp/libfuzzer.%d", getpid()); fp = fopen(filename, "wb"); if (!fp) { From 1ca26127ead55ed57c07ecd19ac09803ceecbf7d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 2 Sep 2026 21:57:10 +0000 Subject: [PATCH 07/17] Fuzz MMDB_get_entry_data_list in the fuzz harness The harness only opened each input. It now looks up 1.1.1.1 and decodes the complete entry, so fuzzing reaches the decoder and its resource limits. Raise the input cap from 4 KiB to 256 KiB and update the seed corpus command to match, so the pointer DoS fixtures fit. Co-Authored-By: Claude Opus 4.8 --- README.fuzzing.md | 2 +- t/fuzz_mmdb.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/README.fuzzing.md b/README.fuzzing.md index 621061c0..706e4bef 100644 --- a/README.fuzzing.md +++ b/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/ ``` diff --git a/t/fuzz_mmdb.c b/t/fuzz_mmdb.c index c936dc3c..4d4c2e14 100644 --- a/t/fuzz_mmdb.c +++ b/t/fuzz_mmdb.c @@ -3,7 +3,7 @@ #include #define kMinInputLength 2 -#define kMaxInputLength 4048 +#define kMaxInputLength (256 * 1024) extern int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size); @@ -29,6 +29,15 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { status = MMDB_open(filename, MMDB_MODE_MMAP, &mmdb); if (status == MMDB_SUCCESS) { + int gai_error, mmdb_error; + MMDB_lookup_result_s result = + MMDB_lookup_string(&mmdb, "1.1.1.1", &gai_error, &mmdb_error); + if (gai_error == 0 && mmdb_error == MMDB_SUCCESS && + result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + MMDB_get_entry_data_list(&result.entry, &entry_data_list); + MMDB_free_entry_data_list(entry_data_list); + } MMDB_close(&mmdb); } From 8c2a60af90bafaeab3eb75685162aa82a28ed7dc Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 01:42:18 +0000 Subject: [PATCH 08/17] Refer to the merged Reader Resource Limits section The changelog and the macro comment called the guidance proposed. The MaxMind DB specification change has merged (maxmind/MaxMind-DB#282). Co-Authored-By: Claude Fable 5.1 --- Changes.md | 6 +++--- src/maxminddb.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index 50b84913..3b6d793b 100644 --- a/Changes.md +++ b/Changes.md @@ -4,9 +4,9 @@ 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 proposed Reader - Resource Limits guidance for the MaxMind DB specification and stops at 65,536 - values or 2 MiB of string and bytes payload per entry. See the + far more data than the file holds. The decoder now follows the Reader Resource + Limits section of the MaxMind DB specification and stops at 65,536 values or 2 + MiB of string and bytes payload per entry. See the `MMDB_get_entry_data_list()` documentation for details. - Exceeding a limit returns the new `MMDB_DECODER_LIMIT_ERROR` status and leaves the output list set to `NULL`. diff --git a/src/maxminddb.c b/src/maxminddb.c index c19b7d99..c752a3c6 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -38,8 +38,8 @@ typedef ADDRESS_FAMILY sa_family_t; // The maximum number of data-section values decoded for a single entry. This // bounds 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 this leaves a wide margin. See the proposed "Reader -// Resource Limits" guidance for the MaxMind DB specification. +// few hundred values, so this leaves a wide margin. See "Reader Resource +// Limits" in the MaxMind DB specification. #ifndef MAXIMUM_DATA_STRUCTURE_VALUES #define MAXIMUM_DATA_STRUCTURE_VALUES (1U << 16) #endif From f64beda2c8110391b79ae157bc05db74bbc95d2e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 03:05:24 +0000 Subject: [PATCH 09/17] Report the nesting depth limit as a decoder limit The depth limit returned MMDB_INVALID_DATA_ERROR and could not be changed, while the new value-count and payload limits return MMDB_DECODER_LIMIT_ERROR and can be raised at build time. The MaxMind DB specification groups all three as reader resource limits, and a record nested past 512 levels can be valid data just as an over-limit record can. Return MMDB_DECODER_LIMIT_ERROR from both depth checks and let MAXIMUM_DATA_STRUCTURE_DEPTH be overridden with the same range check as the other two limits. MMDB_open already maps that status to MMDB_INVALID_METADATA_ERROR, so an over-deep metadata structure is now reported the same way as an over-limit one. Co-Authored-By: Claude Fable 5.1 --- Changes.md | 10 ++++++---- doc/libmaxminddb.md | 37 ++++++++++++++++++++----------------- src/maxminddb.c | 17 ++++++++++++++--- t/decoder_limits_t.pl | 7 ++++++- t/max_depth_t.c | 10 +++++----- 5 files changed, 51 insertions(+), 30 deletions(-) diff --git a/Changes.md b/Changes.md index 3b6d793b..11189b53 100644 --- a/Changes.md +++ b/Changes.md @@ -5,15 +5,17 @@ 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 and stops at 65,536 values or 2 - MiB of string and bytes payload per entry. See the + Limits section of the MaxMind DB specification and stops at 512 nesting + levels, 65,536 values, or 2 MiB of string and bytes payload per entry. See the `MMDB_get_entry_data_list()` documentation for details. - Exceeding a limit returns the new `MMDB_DECODER_LIMIT_ERROR` status and - leaves the output list set to `NULL`. + leaves the output list set to `NULL`. Structures nested past the depth limit + previously returned `MMDB_INVALID_DATA_ERROR`. - `MMDB_open()` returns `MMDB_INVALID_METADATA_ERROR` when the `languages` or `description` metadata exceeds a limit. - The limits can be raised when building the library with - `-DMAXIMUM_DATA_STRUCTURE_VALUES` and `-DMAXIMUM_DATA_STRUCTURE_BYTES`. + `-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 diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index 26086f22..70f190dd 100644 --- a/doc/libmaxminddb.md +++ b/doc/libmaxminddb.md @@ -395,9 +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 an entry as a complete list would exceed - the configured value-count or string/bytes payload limit. The entry may still - be valid MaxMind DB data. +- `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. @@ -649,24 +649,27 @@ 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. It decodes at most 65,536 list values and -at most 2 MiB of UTF-8 string and bytes payload per call. A structure exactly at -either limit is accepted. If a structure exceeds either limit, the function -returns `MMDB_DECODER_LIMIT_ERROR` and sets `entry_data_list` to `NULL`. +work and the caller-visible payload. It decodes at most 512 nesting levels, +65,536 list values, and 2 MiB of UTF-8 string and bytes payload per call. A +structure exactly at a limit is accepted. If a structure exceeds a limit, the +function 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_VALUES` and -`MAXIMUM_DATA_STRUCTURE_BYTES`. For example, pass -`-DMAXIMUM_DATA_STRUCTURE_BYTES=3145728` in the library's compiler flags. Give -the value as a plain integer. The compiler evaluates an expression such as -`1<<31` as `int`, which overflows above 2,147,483,647. This requires rebuilding -the library itself. Defining the macro only while building an application does -not change a packaged shared library. +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. Give the value as a plain integer. The compiler evaluates an expression +such as `1<<31` as `int`, which overflows above 2,147,483,647. This requires +rebuilding the library itself. Defining the 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 these two budgets. Applications -that cannot rebuild a packaged library can use those functions to retrieve a -specific field from an otherwise over-limit record. +complete structure and therefore do not charge the value-count or payload +budgets. They apply the nesting depth limit while they follow the path and +return `MMDB_DECODER_LIMIT_ERROR` past it. 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 = diff --git a/src/maxminddb.c b/src/maxminddb.c index c752a3c6..89aa9851 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -34,7 +34,18 @@ typedef ADDRESS_FAMILY sa_family_t; #endif #define MMDB_DATA_SECTION_SEPARATOR (16) -#define MAXIMUM_DATA_STRUCTURE_DEPTH (512) +// The maximum nesting depth decoded for a single entry. Entering a map or an +// array, or following a pointer, adds one level. This stops unbounded +// recursion, including a pointer cycle. See "Reader Resource Limits" in the +// MaxMind DB specification. +#ifndef MAXIMUM_DATA_STRUCTURE_DEPTH + #define MAXIMUM_DATA_STRUCTURE_DEPTH (512) +#endif + +#if MAXIMUM_DATA_STRUCTURE_DEPTH < 1 || MAXIMUM_DATA_STRUCTURE_DEPTH > INT_MAX + #error "MAXIMUM_DATA_STRUCTURE_DEPTH must be between 1 and INT_MAX" +#endif + // The maximum number of data-section values decoded for a single entry. This // bounds a pointer fan-out, where nested pointers to shared targets would // otherwise cost 2**depth decode operations. The largest real records decode a @@ -1413,7 +1424,7 @@ static int skip_map_or_array(const MMDB_s *const mmdb, int depth) { if (depth >= MAXIMUM_DATA_STRUCTURE_DEPTH) { DEBUG_MSG("reached the maximum data structure depth"); - return MMDB_INVALID_DATA_ERROR; + return MMDB_DECODER_LIMIT_ERROR; } if (entry_data->type == MMDB_DATA_TYPE_MAP) { @@ -1790,7 +1801,7 @@ static int get_entry_data_list(const MMDB_s *const mmdb, int depth) { if (depth >= MAXIMUM_DATA_STRUCTURE_DEPTH) { DEBUG_MSG("reached the maximum data structure depth"); - return MMDB_INVALID_DATA_ERROR; + return MMDB_DECODER_LIMIT_ERROR; } depth++; CHECKED_DECODE_ONE(mmdb, offset, &entry_data_list->entry_data); diff --git a/t/decoder_limits_t.pl b/t/decoder_limits_t.pl index 65123ab1..a10ed049 100755 --- a/t/decoder_limits_t.pl +++ b/t/decoder_limits_t.pl @@ -50,7 +50,10 @@ "-I$src_dir", ); -for my $definition ('-DMAXIMUM_DATA_STRUCTURE_VALUES=1000000') { +for my $definition ( + '-DMAXIMUM_DATA_STRUCTURE_DEPTH=1000', + '-DMAXIMUM_DATA_STRUCTURE_VALUES=1000000', +) { my ( $status, $stderr ) = _run( @base, $definition, @@ -62,6 +65,8 @@ } for my $definition ( + '-DMAXIMUM_DATA_STRUCTURE_DEPTH=0', + '-DMAXIMUM_DATA_STRUCTURE_DEPTH=-1', '-DMAXIMUM_DATA_STRUCTURE_VALUES=0', '-DMAXIMUM_DATA_STRUCTURE_VALUES=-1', '-DMAXIMUM_DATA_STRUCTURE_VALUES=SIZE_MAX+1', diff --git a/t/max_depth_t.c b/t/max_depth_t.c index 7511f3e7..14ea0fc7 100644 --- a/t/max_depth_t.c +++ b/t/max_depth_t.c @@ -23,14 +23,14 @@ void test_deep_nesting_rejected(void) { if (result.found_entry) { /* Looking up non-existent key "z" forces skip_map_or_array to * recurse through all 600 nesting levels. With the depth limit, - * this should return MMDB_INVALID_DATA_ERROR instead of crashing. */ + * this should return MMDB_DECODER_LIMIT_ERROR instead of crashing. */ MMDB_entry_data_s entry_data; const char *lookup_path[] = {"z", NULL}; status = MMDB_aget_value(&result.entry, &entry_data, lookup_path); cmp_ok(status, "==", - MMDB_INVALID_DATA_ERROR, - "MMDB_aget_value returns MMDB_INVALID_DATA_ERROR for " + MMDB_DECODER_LIMIT_ERROR, + "MMDB_aget_value returns MMDB_DECODER_LIMIT_ERROR for " "deeply nested data exceeding max depth"); } @@ -98,8 +98,8 @@ void test_deep_array_nesting_rejected(void) { status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); cmp_ok(status, "==", - MMDB_INVALID_DATA_ERROR, - "MMDB_get_entry_data_list returns MMDB_INVALID_DATA_ERROR " + MMDB_DECODER_LIMIT_ERROR, + "MMDB_get_entry_data_list returns MMDB_DECODER_LIMIT_ERROR " "for deeply nested arrays exceeding max depth"); MMDB_free_entry_data_list(entry_data_list); } From 5fb2ce5667d951f39979613d954569fb5c3a5462 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 22:10:32 +0000 Subject: [PATCH 10/17] Fix data pool test under NDEBUG --- t/data-pool-t.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/data-pool-t.c b/t/data-pool-t.c index 0b2f48ca..cd9eb693 100644 --- a/t/data-pool-t.c +++ b/t/data-pool-t.c @@ -147,7 +147,9 @@ static void test_data_pool_alloc(void) { MMDB_data_pool_s *const pool = data_pool_new(64, maximum_size); ok(pool != NULL, "created a decoder-sized pool"); for (size_t i = 0; i < maximum_size; i++) { - assert(data_pool_alloc(pool) != NULL); + MMDB_entry_data_list_s *const entry = data_pool_alloc(pool); + assert(entry != NULL); + (void)entry; } cmp_ok(pool->capacity, "==", From ea62fc29661590b7ca3f8d189945214544f56024 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 22:10:32 +0000 Subject: [PATCH 11/17] Improve decoder limit override tests --- t/decoder_limits_t.pl | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/t/decoder_limits_t.pl b/t/decoder_limits_t.pl index a10ed049..e8557b69 100755 --- a/t/decoder_limits_t.pl +++ b/t/decoder_limits_t.pl @@ -69,7 +69,6 @@ '-DMAXIMUM_DATA_STRUCTURE_DEPTH=-1', '-DMAXIMUM_DATA_STRUCTURE_VALUES=0', '-DMAXIMUM_DATA_STRUCTURE_VALUES=-1', - '-DMAXIMUM_DATA_STRUCTURE_VALUES=SIZE_MAX+1', '-DMAXIMUM_DATA_STRUCTURE_BYTES=0', '-DMAXIMUM_DATA_STRUCTURE_BYTES=-1', ) { @@ -94,22 +93,24 @@ #include #include #include +#include static int fail(const char *path, const char *what, int status, int code) { fprintf(stderr, "%s: %s: %s\n", path, what, MMDB_strerror(status)); return code; } -static int lookup(const char *path, MMDB_s *mmdb, MMDB_lookup_result_s *result) { +static int lookup(const char *path, const char *address, MMDB_s *mmdb, + MMDB_lookup_result_s *result) { int status = MMDB_open(path, MMDB_MODE_MMAP, mmdb); if (status != MMDB_SUCCESS) { return fail(path, "open", status, 1); } int gai_error, mmdb_error; - *result = MMDB_lookup_string(mmdb, "1.1.1.1", &gai_error, &mmdb_error); + *result = MMDB_lookup_string(mmdb, address, &gai_error, &mmdb_error); if (gai_error != 0 || mmdb_error != MMDB_SUCCESS || !result->found_entry) { MMDB_close(mmdb); - return fail(path, "lookup of 1.1.1.1 found no entry", mmdb_error, 2); + return fail(path, "lookup found no entry", mmdb_error, 2); } return 0; } @@ -117,7 +118,7 @@ static int decode(const char *path, size_t expected_count) { MMDB_s mmdb; MMDB_lookup_result_s result; - int code = lookup(path, &mmdb, &result); + int code = lookup(path, "1.1.1.1", &mmdb, &result); if (code != 0) { return code; } @@ -141,10 +142,11 @@ return 0; } -static int reject(const char *path) { +static int +check_status(const char *path, const char *address, int expected_status) { MMDB_s mmdb; MMDB_lookup_result_s result; - int code = lookup(path, &mmdb, &result); + int code = lookup(path, address, &mmdb, &result); if (code != 0) { return code; } @@ -152,15 +154,20 @@ int status = MMDB_get_entry_data_list(&result.entry, &list); MMDB_free_entry_data_list(list); MMDB_close(&mmdb); - if (status != MMDB_DECODER_LIMIT_ERROR) { - return fail(path, "full decode was not rejected", status, 7); + if (status != expected_status) { + return fail(path, "full decode returned an unexpected status", status, + 7); } return 0; } int main(int argc, char **argv) { + if (argc == 3 && strcmp(argv[1], "--accept") == 0) { + return check_status(argv[2], "1.2.3.4", MMDB_SUCCESS); + } if (argc == 2) { - return reject(argv[1]); + return check_status( + argv[1], "1.1.1.1", MMDB_DECODER_LIMIT_ERROR); } if (argc != 3) { return 5; @@ -199,6 +206,14 @@ definitions => ['-DMAXIMUM_DATA_STRUCTURE_BYTES=65534'], fixtures => ['MaxMind-DB-test-payload-amplification-dos.mmdb'], }, + { + desc => 'a raised nesting depth limit', + definitions => ['-DMAXIMUM_DATA_STRUCTURE_DEPTH=1000'], + accept => 1, + fixtures => [ + '../bad-data/libmaxminddb/libmaxminddb-deep-array-nesting.mmdb' + ], + }, ); my $count = 0; @@ -218,10 +233,10 @@ or diag($compile_stderr); next if $compile_status != 0; - my ( $status, $stderr ) = _run( - $executable, - map { "$Bin/maxmind-db/test-data/$_" } @{ $override->{fixtures} }, - ); + my @arguments = + map { "$Bin/maxmind-db/test-data/$_" } @{ $override->{fixtures} }; + unshift @arguments, '--accept' if $override->{accept}; + my ( $status, $stderr ) = _run( $executable, @arguments ); is( $status, 0, "$override->{desc} takes effect at runtime" ) or diag($stderr); } From 5509ed3e6ada5df75f4213b4f21d49a9009fc6ba Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 22:10:32 +0000 Subject: [PATCH 12/17] Harden fuzz harness file handling --- t/fuzz_mmdb.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/t/fuzz_mmdb.c b/t/fuzz_mmdb.c index 4d4c2e14..b5795cc4 100644 --- a/t/fuzz_mmdb.c +++ b/t/fuzz_mmdb.c @@ -1,5 +1,10 @@ +#ifndef _POSIX_C_SOURCE + #define _POSIX_C_SOURCE 200809L +#endif + #include "maxminddb-compat-util.h" #include "maxminddb.h" +#include #include #define kMinInputLength 2 @@ -9,23 +14,30 @@ extern int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size); int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { int status; - FILE *fp; MMDB_s mmdb; - char filename[256]; + char filename[] = "/tmp/libfuzzer.XXXXXX"; if (size < kMinInputLength || size > kMaxInputLength) { return 0; } - snprintf(filename, sizeof(filename), "/tmp/libfuzzer.%d", getpid()); - - fp = fopen(filename, "wb"); + int const fd = mkstemp(filename); + if (fd == -1) { + abort(); + } + FILE *const fp = fdopen(fd, "wb"); if (!fp) { - return 0; + close(fd); + unlink(filename); + abort(); } - fwrite(data, size, sizeof(uint8_t), fp); - fclose(fp); + size_t const written = fwrite(data, sizeof(uint8_t), size, fp); + int const close_status = fclose(fp); + if (written != size || close_status != 0) { + unlink(filename); + abort(); + } status = MMDB_open(filename, MMDB_MODE_MMAP, &mmdb); if (status == MMDB_SUCCESS) { @@ -41,6 +53,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { MMDB_close(&mmdb); } - unlink(filename); + if (unlink(filename) != 0) { + abort(); + } return 0; } From 5017610115edf2897d9326481068b77814770f73 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 21:51:19 +0000 Subject: [PATCH 13/17] Clarify decoder limit documentation --- Changes.md | 19 +++++++++++-------- doc/libmaxminddb.md | 35 ++++++++++++++++++----------------- src/data-pool.c | 7 ++++--- src/data-pool.h | 12 +++++------- src/maxminddb.c | 8 ++++---- 5 files changed, 42 insertions(+), 39 deletions(-) diff --git a/Changes.md b/Changes.md index 11189b53..2ed68428 100644 --- a/Changes.md +++ b/Changes.md @@ -5,14 +5,17 @@ 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 and stops at 512 nesting - levels, 65,536 values, or 2 MiB of string and bytes payload per entry. See the - `MMDB_get_entry_data_list()` documentation for details. - - Exceeding a limit returns the new `MMDB_DECODER_LIMIT_ERROR` status and - leaves the output list set to `NULL`. Structures nested past the depth limit - previously returned `MMDB_INVALID_DATA_ERROR`. - - `MMDB_open()` returns `MMDB_INVALID_METADATA_ERROR` when the `languages` or - `description` metadata exceeds a limit. + 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`. diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index 70f190dd..7afb671a 100644 --- a/doc/libmaxminddb.md +++ b/doc/libmaxminddb.md @@ -455,9 +455,8 @@ 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`. -Opening a database decodes its `languages` and `description` metadata. If one of -these structures exceeds the decoder resource limits described under -`MMDB_get_entry_data_list()`, this function returns +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()` @@ -649,27 +648,29 @@ 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. It decodes at most 512 nesting levels, -65,536 list values, and 2 MiB of UTF-8 string and bytes payload per call. A -structure exactly at a limit is accepted. If a structure exceeds a limit, the -function returns `MMDB_DECODER_LIMIT_ERROR` and sets `entry_data_list` to -`NULL`. +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 existing 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. Give the value as a plain integer. The compiler evaluates an expression -such as `1<<31` as `int`, which overflows above 2,147,483,647. This requires -rebuilding the library itself. Defining the macro only while building an -application does not change a packaged shared library. +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. They apply the nesting depth limit while they follow the path and -return `MMDB_DECODER_LIMIT_ERROR` past it. Applications that cannot rebuild a -packaged library can use those functions to retrieve a specific field from an -otherwise over-limit record. +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 = @@ -750,7 +751,7 @@ 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. 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 either one. +`MMDB_DECODER_LIMIT_ERROR` if the complete metadata list exceeds any of them. ```c MMDB_entry_data_list_s *entry_data_list, *first; diff --git a/src/data-pool.c b/src/data-pool.c index 321c5d18..0280508f 100644 --- a/src/data-pool.c +++ b/src/data-pool.c @@ -10,7 +10,7 @@ #include // Allocate an MMDB_data_pool_s. It initially has space for up to size -// MMDB_entry_data_list_s structs and will never reserve more than max_size. +// MMDB_entry_data_list_s structs. Its total capacity will not exceed max_size. MMDB_data_pool_s *data_pool_new(size_t size, size_t const max_size) { MMDB_data_pool_s *const pool = calloc(1, sizeof(MMDB_data_pool_s)); if (!pool) { @@ -70,8 +70,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; diff --git a/src/data-pool.h b/src/data-pool.h index 7e779f04..23581577 100644 --- a/src/data-pool.h +++ b/src/data-pool.h @@ -6,12 +6,10 @@ #include #include -// 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 @@ -36,7 +34,7 @@ typedef struct MMDB_data_pool_s { // Total number of structs reserved across all blocks. size_t capacity; - // Maximum number of structs this pool may reserve. + // Maximum total number of structs this pool may reserve. size_t max_size; // The current block we're allocating out of. diff --git a/src/maxminddb.c b/src/maxminddb.c index 89aa9851..2263cf7e 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -34,8 +34,7 @@ typedef ADDRESS_FAMILY sa_family_t; #endif #define MMDB_DATA_SECTION_SEPARATOR (16) -// The maximum nesting depth decoded for a single entry. Entering a map or an -// array, or following a pointer, adds one level. This stops unbounded +// The maximum recursive decoder depth for a single entry. This stops unbounded // recursion, including a pointer cycle. See "Reader Resource Limits" in the // MaxMind DB specification. #ifndef MAXIMUM_DATA_STRUCTURE_DEPTH @@ -55,6 +54,8 @@ typedef ADDRESS_FAMILY sa_family_t; #define MAXIMUM_DATA_STRUCTURE_VALUES (1U << 16) #endif +// The upper bound matters on platforms where size_t is narrower than the +// preprocessor's integer arithmetic. #if MAXIMUM_DATA_STRUCTURE_VALUES < 1 || \ MAXIMUM_DATA_STRUCTURE_VALUES > SIZE_MAX #error "MAXIMUM_DATA_STRUCTURE_VALUES must be between 1 and SIZE_MAX" @@ -336,8 +337,7 @@ int MMDB_open(const char *const filename, uint32_t flags, MMDB_s *const mmdb) { status = read_metadata(mmdb); if (MMDB_DECODER_LIMIT_ERROR == status) { - // The languages and description structures are decoded as complete - // lists. Metadata that exceeds the decoder limits is invalid metadata. + // Metadata that exceeds a decoder limit is invalid metadata. status = MMDB_INVALID_METADATA_ERROR; } if (MMDB_SUCCESS != status) { From 3bb02c05fa7128c193660ed2df1d05a5dec627bf Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 22:17:29 +0000 Subject: [PATCH 14/17] Rename the private decoder state type --- src/maxminddb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/maxminddb.c b/src/maxminddb.c index 2263cf7e..634635b4 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -173,10 +173,10 @@ typedef struct record_info_s { uint8_t right_record_offset; } record_info_s; -typedef struct MMDB_decode_state_s { +typedef struct decode_state_s { size_t values; uint64_t bytes; -} MMDB_decode_state_s; +} decode_state_s; #define METADATA_MARKER "\xab\xcd\xefMaxMind.com" /* This is 128kb */ @@ -240,11 +240,11 @@ static int get_entry_data_list(const MMDB_s *const mmdb, uint32_t offset, MMDB_entry_data_list_s *const entry_data_list, MMDB_data_pool_s *const pool, - MMDB_decode_state_s *const decode_state, + decode_state_s *const decode_state, int depth); static int alloc_entry_data_list(MMDB_data_pool_s *const pool, - MMDB_decode_state_s *const decode_state, + decode_state_s *const decode_state, MMDB_entry_data_list_s **const entry_data_list); static float get_ieee754_float(const uint8_t *restrict p); static double get_ieee754_double(const uint8_t *restrict p); @@ -1751,7 +1751,7 @@ int MMDB_get_entry_data_list(MMDB_entry_s *start, return MMDB_OUT_OF_MEMORY_ERROR; } - MMDB_decode_state_s decode_state = {0}; + decode_state_s decode_state = {0}; MMDB_entry_data_list_s *list = NULL; int status = alloc_entry_data_list(pool, &decode_state, &list); if (MMDB_SUCCESS != status) { @@ -1777,7 +1777,7 @@ int MMDB_get_entry_data_list(MMDB_entry_s *start, static int alloc_entry_data_list(MMDB_data_pool_s *const pool, - MMDB_decode_state_s *const decode_state, + decode_state_s *const decode_state, MMDB_entry_data_list_s **const entry_data_list) { size_t const maximum_values = (size_t)(MAXIMUM_DATA_STRUCTURE_VALUES); if (decode_state->values >= maximum_values) { @@ -1797,7 +1797,7 @@ static int get_entry_data_list(const MMDB_s *const mmdb, uint32_t offset, MMDB_entry_data_list_s *const entry_data_list, MMDB_data_pool_s *const pool, - MMDB_decode_state_s *const decode_state, + decode_state_s *const decode_state, int depth) { if (depth >= MAXIMUM_DATA_STRUCTURE_DEPTH) { DEBUG_MSG("reached the maximum data structure depth"); From ed6a89304ce1d1ab3e9e72a9c497c6efeff5696d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 22:18:25 +0000 Subject: [PATCH 15/17] Keep the data pool size parameter const --- src/data-pool.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/data-pool.c b/src/data-pool.c index 0280508f..e4b00f3b 100644 --- a/src/data-pool.c +++ b/src/data-pool.c @@ -11,7 +11,7 @@ // 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 size, size_t const 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; @@ -21,14 +21,15 @@ MMDB_data_pool_s *data_pool_new(size_t size, size_t const max_size) { data_pool_destroy(pool); return NULL; } - if (size > max_size) { - size = max_size; + size_t initial_size = size; + if (initial_size > max_size) { + initial_size = max_size; } - if (!can_multiply(SIZE_MAX, size, sizeof(MMDB_entry_data_list_s))) { + if (!can_multiply(SIZE_MAX, initial_size, sizeof(MMDB_entry_data_list_s))) { data_pool_destroy(pool); return NULL; } - pool->size = size; + pool->size = initial_size; pool->blocks[0] = calloc(pool->size, sizeof(MMDB_entry_data_list_s)); if (!pool->blocks[0]) { data_pool_destroy(pool); @@ -36,8 +37,8 @@ MMDB_data_pool_s *data_pool_new(size_t size, size_t const max_size) { } pool->blocks[0]->pool = pool; - pool->sizes[0] = size; - pool->capacity = size; + pool->sizes[0] = initial_size; + pool->capacity = initial_size; pool->max_size = max_size; pool->block = pool->blocks[0]; From f5fd27d360edbba3d50647be1055a4326936743a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 3 Sep 2026 22:28:51 +0000 Subject: [PATCH 16/17] Avoid a shell in compiler detection --- t/decoder_limits_t.pl | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/t/decoder_limits_t.pl b/t/decoder_limits_t.pl index e8557b69..3284565e 100755 --- a/t/decoder_limits_t.pl +++ b/t/decoder_limits_t.pl @@ -26,8 +26,15 @@ # The checks below rebuild the library with -Werror. Only gcc and clang are # known to compile it cleanly with the flags used here, so skip elsewhere # instead of failing on a missing compiler or an unrelated warning. -my $cc_version = `$cc --version 2>&1`; -if ( $? != 0 || $cc_version !~ /gcc|clang|Free Software Foundation/ ) { +my ( $cc_version, $cc_stderr ) = ( q{}, q{} ); +my $cc_status = eval { + run3( [ $cc, '--version' ], \undef, \$cc_version, \$cc_stderr ); + $?; +}; +$cc_version .= $cc_stderr; +if ( !defined $cc_status + || $cc_status != 0 + || $cc_version !~ /gcc|clang|Free Software Foundation/ ) { plan( skip_all => "decoder limit override tests need gcc or clang" ); } From 5741fff39d3274f405345509d6b004333b4f7f82 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 14:37:41 +0000 Subject: [PATCH 17/17] Clarify the decoder depth limit wording --- doc/libmaxminddb.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index 7afb671a..092bcff7 100644 --- a/doc/libmaxminddb.md +++ b/doc/libmaxminddb.md @@ -650,8 +650,8 @@ once, rather than looking up each piece using repeated calls to 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 existing recursive-decoder depth limit of 512 also -applies. Exceeding any limit returns `MMDB_DECODER_LIMIT_ERROR` and sets +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