From 89ffacae6a062d4c3ee8ce110a370a7ff273a5b6 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 11:32:46 -0800 Subject: [PATCH 01/27] Fix off-by-one in MMDB_read_node() bounds check MMDB_read_node() used > instead of >= when checking node_number against node_count, allowing node_number == node_count through the bounds check. Since nodes are 0-indexed, the last valid node is node_count - 1, so node_count itself is one past the end of the search tree. Reading at that offset hits the 16-byte null separator between the search tree and data section, producing records that decode as 0. When converted to a data offset via data_section_offset_for_record(), this underflows (0 - node_count - 16), resulting in an invalid entry offset. The fix changes > to >= so that node_number == node_count is correctly rejected with MMDB_INVALID_NODE_NUMBER_ERROR. A test has been added to verify this boundary condition. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 5 +++++ src/maxminddb.c | 2 +- t/read_node_t.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/Changes.md b/Changes.md index a93afe93..9a5fee02 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,10 @@ ## 1.13.0 +* Fixed an off-by-one error in `MMDB_read_node()` that allowed reading one + node past the end of the search tree when called with + `node_number == node_count`. This caused the function to read from the + data section separator and return an invalid record with an underflowed + data offset. The check now correctly rejects `node_number >= node_count`. * The handling of float and double types was rewritten to fix compiler errors and to eliminate the use of volatile. * Improved endian preprocessor check if `MMDB_LITTLE_ENDIAN` is not set. diff --git a/src/maxminddb.c b/src/maxminddb.c index 838a2669..e5e5efd1 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1097,7 +1097,7 @@ int MMDB_read_node(const MMDB_s *const mmdb, return MMDB_UNKNOWN_DATABASE_FORMAT_ERROR; } - if (node_number > mmdb->metadata.node_count) { + if (node_number >= mmdb->metadata.node_count) { return MMDB_INVALID_NODE_NUMBER_ERROR; } diff --git a/t/read_node_t.c b/t/read_node_t.c index da4ec849..b6ab6d83 100644 --- a/t/read_node_t.c +++ b/t/read_node_t.c @@ -252,10 +252,43 @@ void run_32_bit_record_tests(int mode, const char *mode_desc) { free(mmdb); } +void run_read_node_invalid_node_number_tests(int mode, const char *mode_desc) { + const char *filename = "MaxMind-DB-test-mixed-24.mmdb"; + char *path = test_database_path(filename); + MMDB_s *mmdb = open_ok(path, mode, mode_desc); + free(path); + + uint32_t node_count = mmdb->metadata.node_count; + + MMDB_search_node_s node; + int status; + + /* node_count is one past the last valid node (nodes are 0-indexed). + * This must be rejected. */ + status = MMDB_read_node(mmdb, node_count, &node); + cmp_ok(status, + "==", + MMDB_INVALID_NODE_NUMBER_ERROR, + "MMDB_read_node with node_number == node_count returns " + "MMDB_INVALID_NODE_NUMBER_ERROR"); + + /* node_count + 1 should also be rejected. */ + status = MMDB_read_node(mmdb, node_count + 1, &node); + cmp_ok(status, + "==", + MMDB_INVALID_NODE_NUMBER_ERROR, + "MMDB_read_node with node_number > node_count returns " + "MMDB_INVALID_NODE_NUMBER_ERROR"); + + MMDB_close(mmdb); + free(mmdb); +} + void run_tests(int mode, const char *mode_desc) { run_24_bit_record_tests(mode, mode_desc); run_28_bit_record_tests(mode, mode_desc); run_32_bit_record_tests(mode, mode_desc); + run_read_node_invalid_node_number_tests(mode, mode_desc); } int main(void) { From f29642972498d18227bdb6cf6dc8aa218c49ccac Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 11:36:29 -0800 Subject: [PATCH 02/27] Add recursion depth limit to skip_map_or_array() skip_map_or_array(), used internally by MMDB_aget_value and MMDB_get_value to skip past map/array values during path lookup, had no recursion depth limit. A crafted MMDB file with deeply nested maps or arrays (e.g. 200,000 levels) could cause unbounded recursion, exhausting the stack and crashing the process. The fix adds a depth parameter matching the existing MAXIMUM_DATA_STRUCTURE_DEPTH (512) limit already enforced by get_entry_data_list(). Callers pass an initial depth of 0, and each recursive call increments it. When the limit is reached, MMDB_INVALID_DATA_ERROR is returned. A new test (max_depth_t) verifies that: - A 600-level nested MMDB returns MMDB_INVALID_DATA_ERROR - A 10-level nested MMDB works normally Co-Authored-By: Claude Opus 4.6 --- Changes.md | 5 ++ src/maxminddb.c | 19 ++-- t/Makefile.am | 4 +- t/max_depth_t.c | 228 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 248 insertions(+), 8 deletions(-) create mode 100644 t/max_depth_t.c diff --git a/Changes.md b/Changes.md index 9a5fee02..1dcb144f 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,10 @@ ## 1.13.0 +* Added a recursion depth limit to `skip_map_or_array()`, matching the + existing `MAXIMUM_DATA_STRUCTURE_DEPTH` (512) limit already used by + `get_entry_data_list()`. A crafted MMDB file with deeply nested maps + or arrays could previously cause a stack overflow via unbounded + recursion in the `MMDB_aget_value` / `MMDB_get_value` code path. * Fixed an off-by-one error in `MMDB_read_node()` that allowed reading one node past the end of the search tree when called with `node_number == node_count`. This caused the function to read from the diff --git a/src/maxminddb.c b/src/maxminddb.c index e5e5efd1..a19e5a8b 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -178,7 +178,8 @@ static int lookup_path_in_map(const char *path_elem, const MMDB_s *const mmdb, MMDB_entry_data_s *entry_data); static int skip_map_or_array(const MMDB_s *const mmdb, - MMDB_entry_data_s *entry_data); + MMDB_entry_data_s *entry_data, + int depth); static int decode_one_follow(const MMDB_s *const mmdb, uint32_t offset, MMDB_entry_data_s *entry_data); @@ -1272,7 +1273,7 @@ static int lookup_path_in_array(const char *path_elem, /* We don't want to follow a pointer here. If the next element is a * pointer we simply skip it and keep going */ CHECKED_DECODE_ONE(mmdb, entry_data->offset_to_next, entry_data); - int status = skip_map_or_array(mmdb, entry_data); + int status = skip_map_or_array(mmdb, entry_data, 0); if (MMDB_SUCCESS != status) { return status; } @@ -1314,7 +1315,7 @@ static int lookup_path_in_map(const char *path_elem, /* We don't want to follow a pointer here. If the next element is * a pointer we simply skip it and keep going */ CHECKED_DECODE_ONE(mmdb, offset_to_value, &value); - int status = skip_map_or_array(mmdb, &value); + int status = skip_map_or_array(mmdb, &value, 0); if (MMDB_SUCCESS != status) { return status; } @@ -1327,7 +1328,13 @@ static int lookup_path_in_map(const char *path_elem, } static int skip_map_or_array(const MMDB_s *const mmdb, - MMDB_entry_data_s *entry_data) { + MMDB_entry_data_s *entry_data, + int depth) { + if (depth >= MAXIMUM_DATA_STRUCTURE_DEPTH) { + DEBUG_MSG("reached the maximum data structure depth"); + return MMDB_INVALID_DATA_ERROR; + } + if (entry_data->type == MMDB_DATA_TYPE_MAP) { uint32_t size = entry_data->data_size; while (size-- > 0) { @@ -1335,7 +1342,7 @@ static int skip_map_or_array(const MMDB_s *const mmdb, mmdb, entry_data->offset_to_next, entry_data); // key CHECKED_DECODE_ONE( mmdb, entry_data->offset_to_next, entry_data); // value - int status = skip_map_or_array(mmdb, entry_data); + int status = skip_map_or_array(mmdb, entry_data, depth + 1); if (MMDB_SUCCESS != status) { return status; } @@ -1345,7 +1352,7 @@ static int skip_map_or_array(const MMDB_s *const mmdb, while (size-- > 0) { CHECKED_DECODE_ONE( mmdb, entry_data->offset_to_next, entry_data); // value - int status = skip_map_or_array(mmdb, entry_data); + int status = skip_map_or_array(mmdb, entry_data, depth + 1); if (MMDB_SUCCESS != status) { return status; } diff --git a/t/Makefile.am b/t/Makefile.am index d8d416f1..bb739dab 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -18,8 +18,8 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ check_PROGRAMS = \ bad_pointers_t bad_databases_t basic_lookup_t data_entry_list_t \ data-pool-t data_types_t dump_t get_value_t get_value_pointer_bug_t \ - ipv4_start_cache_t ipv6_lookup_in_ipv4_t metadata_t metadata_pointers_t \ - no_map_get_value_t read_node_t threads_t version_t + ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ + metadata_pointers_t no_map_get_value_t read_node_t threads_t version_t data_pool_t_LDFLAGS = $(AM_LDFLAGS) -lm data_pool_t_SOURCES = data-pool-t.c ../src/data-pool.c diff --git a/t/max_depth_t.c b/t/max_depth_t.c new file mode 100644 index 00000000..128be6f1 --- /dev/null +++ b/t/max_depth_t.c @@ -0,0 +1,228 @@ +#include "maxminddb_test_helper.h" +#include + +/* MMDB binary format constants */ +#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" +#define METADATA_MARKER_LEN 14 +#define DATA_SEPARATOR_SIZE 16 + +/* Write a map with size entries (size must be < 29) */ +static int write_map(uint8_t *buf, uint32_t size) { + buf[0] = (7 << 5) | (size & 0x1f); + return 1; +} + +/* Write a utf8_string (type 2). For short strings (len < 29). */ +static int write_string(uint8_t *buf, const char *str, uint32_t len) { + buf[0] = (2 << 5) | (len & 0x1f); + memcpy(buf + 1, str, len); + return 1 + len; +} + +static int write_uint16(uint8_t *buf, uint16_t value) { + buf[0] = (5 << 5) | 2; + buf[1] = (value >> 8) & 0xff; + buf[2] = value & 0xff; + return 3; +} + +static int write_uint32(uint8_t *buf, uint32_t value) { + buf[0] = (6 << 5) | 4; + buf[1] = (value >> 24) & 0xff; + buf[2] = (value >> 16) & 0xff; + buf[3] = (value >> 8) & 0xff; + buf[4] = value & 0xff; + return 5; +} + +static int write_uint64(uint8_t *buf, uint64_t value) { + buf[0] = (0 << 5) | 8; + buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ + buf[2] = (value >> 56) & 0xff; + buf[3] = (value >> 48) & 0xff; + buf[4] = (value >> 40) & 0xff; + buf[5] = (value >> 32) & 0xff; + buf[6] = (value >> 24) & 0xff; + buf[7] = (value >> 16) & 0xff; + buf[8] = (value >> 8) & 0xff; + buf[9] = value & 0xff; + return 10; +} + +static int write_meta_key(uint8_t *buf, const char *key) { + return write_string(buf, key, strlen(key)); +} + +/* + * Create a crafted MMDB file with deeply nested maps and write it to path. + * The data section contains: {"a": {"a": {"a": ... {"a": "x"} ...}}} + * nested `depth` levels deep. All IP lookups resolve to this data. + */ +static void create_deep_nesting_db(const char *path, int depth) { + uint32_t node_count = 1; + uint32_t record_size = 24; + uint32_t record_value = node_count + 16; + + size_t data_size = (size_t)depth * 3 + 2; + size_t metadata_buf_size = 512; + size_t search_tree_size = 6; + size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_size + + METADATA_MARKER_LEN + metadata_buf_size; + + uint8_t *file = calloc(1, total_size); + if (!file) { + BAIL_OUT("calloc failed"); + } + + size_t pos = 0; + + /* Search tree: 1 node, 24-bit records */ + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + + /* 16-byte null separator */ + memset(file + pos, 0, DATA_SEPARATOR_SIZE); + pos += DATA_SEPARATOR_SIZE; + + /* Data section: deeply nested maps */ + for (int i = 0; i < depth; i++) { + pos += write_map(file + pos, 1); + pos += write_string(file + pos, "a", 1); + } + pos += write_string(file + pos, "x", 1); + + /* Metadata marker */ + memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); + pos += METADATA_MARKER_LEN; + + /* Metadata map */ + pos += write_map(file + pos, 9); + + pos += write_meta_key(file + pos, "binary_format_major_version"); + pos += write_uint16(file + pos, 2); + + pos += write_meta_key(file + pos, "binary_format_minor_version"); + pos += write_uint16(file + pos, 0); + + pos += write_meta_key(file + pos, "build_epoch"); + pos += write_uint64(file + pos, 1000000000ULL); + + pos += write_meta_key(file + pos, "database_type"); + pos += write_string(file + pos, "Test", 4); + + pos += write_meta_key(file + pos, "description"); + pos += write_map(file + pos, 0); + + pos += write_meta_key(file + pos, "ip_version"); + pos += write_uint16(file + pos, 4); + + pos += write_meta_key(file + pos, "languages"); + file[pos++] = (0 << 5) | 0; /* type 0 (extended), size 0 */ + file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ + + pos += write_meta_key(file + pos, "node_count"); + pos += write_uint32(file + pos, node_count); + + pos += write_meta_key(file + pos, "record_size"); + pos += write_uint16(file + pos, record_size); + + FILE *f = fopen(path, "wb"); + if (!f) { + free(file); + BAIL_OUT("fopen failed"); + } + fwrite(file, 1, pos, f); + fclose(f); + free(file); +} + +void test_deep_nesting_rejected(void) { + const char *path = "/tmp/test_max_depth.mmdb"; + /* 600 levels is above MAXIMUM_DATA_STRUCTURE_DEPTH (512) */ + create_deep_nesting_db(path, 600); + + MMDB_s mmdb; + int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "opened deeply nested MMDB"); + + if (status != MMDB_SUCCESS) { + diag("MMDB_open failed: %s", MMDB_strerror(status)); + remove(path); + return; + } + + int gai_error, mmdb_error; + MMDB_lookup_result_s result = + MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); + ok(result.found_entry, "entry found"); + + 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. */ + 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 " + "deeply nested data exceeding max depth"); + } + + MMDB_close(&mmdb); + remove(path); +} + +void test_valid_nesting_allowed(void) { + const char *path = "/tmp/test_valid_depth.mmdb"; + /* 10 levels is well within MAXIMUM_DATA_STRUCTURE_DEPTH (512) */ + create_deep_nesting_db(path, 10); + + MMDB_s mmdb; + int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "opened moderately nested MMDB"); + + if (status != MMDB_SUCCESS) { + diag("MMDB_open failed: %s", MMDB_strerror(status)); + remove(path); + return; + } + + int gai_error, mmdb_error; + MMDB_lookup_result_s result = + MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); + ok(result.found_entry, "entry found"); + + if (result.found_entry) { + /* Looking up non-existent key "z" at depth 10 should NOT + * trigger the depth limit — it should return the normal + * "path does not match" error. */ + 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_LOOKUP_PATH_DOES_NOT_MATCH_DATA_ERROR, + "MMDB_aget_value returns PATH_DOES_NOT_MATCH for " + "valid nesting depth"); + } + + MMDB_close(&mmdb); + remove(path); +} + +int main(void) { + plan(NO_PLAN); + test_deep_nesting_rejected(); + test_valid_nesting_allowed(); + done_testing(); +} From 01281277a0bab64ea20c6d91cdf316efb80703ee Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 11:37:11 -0800 Subject: [PATCH 03/27] Fix alloca off-by-one in mmdblookup on Windows On Windows, mmdblookup allocated strlen(argv[0]) bytes for the program name buffer, but _splitpath writes fname + ext + '\0', which needs strlen(argv[0]) + 1 bytes. This caused a one-byte stack buffer overflow when the null terminator was written past the end of the alloca'd buffer. The fix adds + 1 to the alloca size. This code path is Windows-only (#ifdef _WIN32) and only affects the CLI tool, not the library. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 3 +++ bin/mmdblookup.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Changes.md b/Changes.md index 1dcb144f..457b751b 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,8 @@ ## 1.13.0 +* Fixed an off-by-one in `mmdblookup` on Windows where `alloca` allocated + one byte too few for the program name buffer, causing `_splitpath` to + write one byte past the end when appending the null terminator. * Added a recursion depth limit to `skip_map_or_array()`, matching the existing `MAXIMUM_DATA_STRUCTURE_DEPTH` (512) limit already used by `get_entry_data_list()`. A crafted MMDB file with deeply nested maps diff --git a/bin/mmdblookup.c b/bin/mmdblookup.c index 9a37c09d..801837bd 100644 --- a/bin/mmdblookup.c +++ b/bin/mmdblookup.c @@ -229,7 +229,7 @@ static const char **get_options(int argc, static int version = 0; #ifdef _WIN32 - char *program = alloca(strlen(argv[0])); + char *program = alloca(strlen(argv[0]) + 1); _splitpath(argv[0], NULL, NULL, program, NULL); _splitpath(argv[0], NULL, NULL, NULL, program + strlen(program)); #else From 467b085225f238ad273e1ce66c72fc2fcc6da837 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 11:55:41 -0800 Subject: [PATCH 04/27] Fix MMDB_lookup_string leaving *mmdb_error uninitialized on GAI failure When getaddrinfo fails, *mmdb_error was never written, leaving callers with an indeterminate value if they checked it without first checking *gai_error. Now explicitly set to MMDB_SUCCESS in the GAI failure path. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 4 ++++ src/maxminddb.c | 5 +++++ t/Makefile.am | 3 ++- t/gai_error_t.c | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 t/gai_error_t.c diff --git a/Changes.md b/Changes.md index 457b751b..166e3d03 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,9 @@ ## 1.13.0 +* `MMDB_lookup_string()` now sets `*mmdb_error` to `MMDB_SUCCESS` when + `getaddrinfo` fails (non-zero `*gai_error`). Previously, `*mmdb_error` + was left uninitialized in this case, which could cause callers to read + an indeterminate value. * Fixed an off-by-one in `mmdblookup` on Windows where `alloca` allocated one byte too few for the program name buffer, causing `_splitpath` to write one byte past the end when appending the null terminator. diff --git a/src/maxminddb.c b/src/maxminddb.c index a19e5a8b..5ceff994 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -883,6 +883,11 @@ MMDB_lookup_result_s MMDB_lookup_string(const MMDB_s *const mmdb, if (!*gai_error) { result = MMDB_lookup_sockaddr(mmdb, addresses->ai_addr, mmdb_error); + } else { + /* No MMDB error occurred; the GAI failure is reported via + * *gai_error. Set *mmdb_error to a defined value so callers + * don't read indeterminate memory. */ + *mmdb_error = MMDB_SUCCESS; } if (NULL != addresses) { diff --git a/t/Makefile.am b/t/Makefile.am index bb739dab..6c5c2cba 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -17,7 +17,8 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ check_PROGRAMS = \ bad_pointers_t bad_databases_t basic_lookup_t data_entry_list_t \ - data-pool-t data_types_t dump_t get_value_t get_value_pointer_bug_t \ + data-pool-t data_types_t dump_t gai_error_t get_value_t \ + get_value_pointer_bug_t \ ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ metadata_pointers_t no_map_get_value_t read_node_t threads_t version_t diff --git a/t/gai_error_t.c b/t/gai_error_t.c new file mode 100644 index 00000000..9e9f0304 --- /dev/null +++ b/t/gai_error_t.c @@ -0,0 +1,33 @@ +#include "maxminddb_test_helper.h" + +void test_mmdb_error_set_on_gai_failure(void) { + char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); + MMDB_s *mmdb = open_ok(db_file, MMDB_MODE_MMAP, "mmap mode"); + free(db_file); + + if (!mmdb) { + return; + } + + /* Set mmdb_error to a known non-zero value to detect if it gets written */ + int gai_error = 0; + int mmdb_error = 0xDEAD; + + /* ".." is not a valid IP address - getaddrinfo will fail */ + MMDB_lookup_string(mmdb, "..", &gai_error, &mmdb_error); + + ok(gai_error != 0, "gai_error is non-zero for invalid IP '..'"); + cmp_ok(mmdb_error, + "==", + MMDB_SUCCESS, + "mmdb_error is set to MMDB_SUCCESS when gai_error is non-zero"); + + MMDB_close(mmdb); + free(mmdb); +} + +int main(void) { + plan(NO_PLAN); + test_mmdb_error_set_on_gai_failure(); + done_testing(); +} From ffa29c1824336e1c4256a7b9696ea4e0800e9968 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 11:56:25 -0800 Subject: [PATCH 05/27] Fix print_indentation stack overflow on negative indent When MMDB_dump_entry_data_list() was called with a negative indent, the value was cast to size_t producing a massive memset length. Clamp negative values to 0. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 5 ++++ src/maxminddb.c | 2 +- t/Makefile.am | 3 ++- t/bad_indent_t.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 t/bad_indent_t.c diff --git a/Changes.md b/Changes.md index 166e3d03..27090a23 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,10 @@ ## 1.13.0 +* Fixed a stack buffer overflow in `print_indentation()` when + `MMDB_dump_entry_data_list()` was called with a negative `indent` + value. The negative integer was cast to `size_t`, producing a massive + value passed to `memset()`. Negative indent values are now clamped + to 0. * `MMDB_lookup_string()` now sets `*mmdb_error` to `MMDB_SUCCESS` when `getaddrinfo` fails (non-zero `*gai_error`). Previously, `*mmdb_error` was left uninitialized in this case, which could cause callers to read diff --git a/src/maxminddb.c b/src/maxminddb.c index 5ceff994..d3ede07b 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -2170,7 +2170,7 @@ dump_entry_data_list(FILE *stream, static void print_indentation(FILE *stream, int i) { char buffer[1024]; - int size = i >= 1024 ? 1023 : i; + int size = i < 0 ? 0 : (i >= 1024 ? 1023 : i); memset(buffer, 32, (size_t)size); buffer[size] = '\0'; fputs(buffer, stream); diff --git a/t/Makefile.am b/t/Makefile.am index 6c5c2cba..80f7db5c 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -16,7 +16,8 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ libtap/tap.c libtap/tap.h maxmind-db check_PROGRAMS = \ - bad_pointers_t bad_databases_t basic_lookup_t data_entry_list_t \ + bad_pointers_t bad_databases_t bad_indent_t basic_lookup_t \ + data_entry_list_t \ data-pool-t data_types_t dump_t gai_error_t get_value_t \ get_value_pointer_bug_t \ ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ diff --git a/t/bad_indent_t.c b/t/bad_indent_t.c new file mode 100644 index 00000000..21075ce9 --- /dev/null +++ b/t/bad_indent_t.c @@ -0,0 +1,64 @@ +#include "maxminddb_test_helper.h" +#include + +void test_negative_indent(void) { + char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); + MMDB_s *mmdb = open_ok(db_file, MMDB_MODE_MMAP, "mmap mode"); + free(db_file); + + if (!mmdb) { + return; + } + + int gai_error, mmdb_error; + MMDB_lookup_result_s result = + MMDB_lookup_string(mmdb, "1.1.1.1", &gai_error, &mmdb_error); + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); + + if (result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + int status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); + cmp_ok(status, "==", MMDB_SUCCESS, "get_entry_data_list succeeded"); + + if (MMDB_SUCCESS == status && entry_data_list) { + FILE *devnull = fopen("/dev/null", "w"); + if (!devnull) { + BAIL_OUT("could not open /dev/null"); + } + + /* Negative indent should not crash — it should be clamped to 0 */ + status = MMDB_dump_entry_data_list(devnull, entry_data_list, -1); + cmp_ok(status, + "==", + MMDB_SUCCESS, + "MMDB_dump_entry_data_list with indent=-1 returns success"); + + status = MMDB_dump_entry_data_list(devnull, entry_data_list, -100); + cmp_ok( + status, + "==", + MMDB_SUCCESS, + "MMDB_dump_entry_data_list with indent=-100 returns success"); + + status = + MMDB_dump_entry_data_list(devnull, entry_data_list, INT_MIN); + cmp_ok(status, + "==", + MMDB_SUCCESS, + "MMDB_dump_entry_data_list with indent=INT_MIN returns " + "success"); + + fclose(devnull); + } + MMDB_free_entry_data_list(entry_data_list); + } + + MMDB_close(mmdb); + free(mmdb); +} + +int main(void) { + plan(NO_PLAN); + test_negative_indent(); + done_testing(); +} From 6b2a64ba8c1857a74041a846b857c6b22a200693 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 11:57:59 -0800 Subject: [PATCH 06/27] NULL file_content after munmap to prevent double-unmap free_mmdb_struct did not NULL file_content after unmapping, so a second call to MMDB_close would double-munmap the same address (UB). Also zero description.count so free_descriptions_metadata is safe on re-entry. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 4 ++++ src/maxminddb.c | 4 ++++ t/Makefile.am | 2 +- t/double_close_t.c | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 t/double_close_t.c diff --git a/Changes.md b/Changes.md index 27090a23..6e09328e 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,9 @@ ## 1.13.0 +* `MMDB_close()` now NULLs the `file_content` pointer after unmapping. + Previously, calling `MMDB_close()` twice on the same struct (or calling + it after a failed `MMDB_open()` that succeeded at mapping) would + double-munmap the file content, which is undefined behavior. * Fixed a stack buffer overflow in `print_indentation()` when `MMDB_dump_entry_data_list()` was called with a negative `indent` value. The negative integer was cast to `size_t`, producing a massive diff --git a/src/maxminddb.c b/src/maxminddb.c index d3ede07b..4a2f64e2 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1906,6 +1906,8 @@ static void free_mmdb_struct(MMDB_s *const mmdb) { #pragma clang diagnostic pop #endif #endif + mmdb->file_content = NULL; + mmdb->file_size = 0; } if (NULL != mmdb->metadata.database_type) { @@ -1943,6 +1945,7 @@ static void free_languages_metadata(MMDB_s *mmdb) { #endif } FREE_AND_SET_NULL(mmdb->metadata.languages.names); + mmdb->metadata.languages.count = 0; } static void free_descriptions_metadata(MMDB_s *mmdb) { @@ -1985,6 +1988,7 @@ static void free_descriptions_metadata(MMDB_s *mmdb) { } FREE_AND_SET_NULL(mmdb->metadata.description.descriptions); + mmdb->metadata.description.count = 0; } const char *MMDB_lib_version(void) { return PACKAGE_VERSION; } diff --git a/t/Makefile.am b/t/Makefile.am index 80f7db5c..d2ca8c0c 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -18,7 +18,7 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ check_PROGRAMS = \ bad_pointers_t bad_databases_t bad_indent_t basic_lookup_t \ data_entry_list_t \ - data-pool-t data_types_t dump_t gai_error_t get_value_t \ + data-pool-t data_types_t double_close_t dump_t gai_error_t get_value_t \ get_value_pointer_bug_t \ ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ metadata_pointers_t no_map_get_value_t read_node_t threads_t version_t diff --git a/t/double_close_t.c b/t/double_close_t.c new file mode 100644 index 00000000..cf5c9d3c --- /dev/null +++ b/t/double_close_t.c @@ -0,0 +1,39 @@ +#include "maxminddb_test_helper.h" + +void test_double_close(void) { + char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); + + MMDB_s mmdb; + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); + free(db_file); + cmp_ok(status, "==", MMDB_SUCCESS, "MMDB_open succeeded"); + + if (status != MMDB_SUCCESS) { + return; + } + + /* First close should work normally */ + MMDB_close(&mmdb); + + ok(mmdb.file_content == NULL, "file_content is NULL after first close"); + cmp_ok(mmdb.metadata.languages.count, + "==", + 0, + "languages.count is 0 after close"); + cmp_ok(mmdb.metadata.description.count, + "==", + 0, + "description.count is 0 after close"); + cmp_ok(mmdb.file_size, "==", 0, "file_size is 0 after close"); + + /* Second close should be a safe no-op (file_content was NULLed) */ + MMDB_close(&mmdb); + + ok(1, "calling MMDB_close twice does not crash"); +} + +int main(void) { + plan(NO_PLAN); + test_double_close(); + done_testing(); +} From 233e312a95f7c3e98afbf98bd1893c932b912415 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 11:59:38 -0800 Subject: [PATCH 07/27] Fix gmtime NULL dereference in mmdblookup dump_meta gmtime() returns NULL for out-of-range time_t values. A crafted database with build_epoch=UINT64_MAX would cause strftime to dereference NULL. Now check the return value and display "unknown" for invalid epochs. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 3 + bin/mmdblookup.c | 7 +- t/Makefile.am | 2 +- t/bad_epoch_t.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 t/bad_epoch_t.c diff --git a/Changes.md b/Changes.md index 6e09328e..8b0290fd 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,8 @@ ## 1.13.0 +* Fixed a NULL pointer dereference in `mmdblookup` when displaying + metadata for a database with an out-of-range `build_epoch`. The + `gmtime()` return value is now checked before passing to `strftime()`. * `MMDB_close()` now NULLs the `file_content` pointer after unmapping. Previously, calling `MMDB_close()` twice on the same struct (or calling it after a failed `MMDB_open()` that succeeded at mapping) would diff --git a/bin/mmdblookup.c b/bin/mmdblookup.c index 801837bd..07034cfb 100644 --- a/bin/mmdblookup.c +++ b/bin/mmdblookup.c @@ -356,7 +356,12 @@ static void dump_meta(MMDB_s *mmdb) { char date[40]; const time_t epoch = (const time_t)mmdb->metadata.build_epoch; - strftime(date, 40, "%F %T UTC", gmtime(&epoch)); + struct tm *tm = gmtime(&epoch); + if (tm != NULL) { + strftime(date, sizeof(date), "%F %T UTC", tm); + } else { + snprintf(date, sizeof(date), "unknown"); + } fprintf(stdout, meta_dump, diff --git a/t/Makefile.am b/t/Makefile.am index d2ca8c0c..a2e994ba 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -16,7 +16,7 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ libtap/tap.c libtap/tap.h maxmind-db check_PROGRAMS = \ - bad_pointers_t bad_databases_t bad_indent_t basic_lookup_t \ + bad_pointers_t bad_databases_t bad_epoch_t bad_indent_t basic_lookup_t \ data_entry_list_t \ data-pool-t data_types_t double_close_t dump_t gai_error_t get_value_t \ get_value_pointer_bug_t \ diff --git a/t/bad_epoch_t.c b/t/bad_epoch_t.c new file mode 100644 index 00000000..e92f4324 --- /dev/null +++ b/t/bad_epoch_t.c @@ -0,0 +1,185 @@ +#include "maxminddb_test_helper.h" +#include + +/* MMDB binary format constants */ +#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" +#define METADATA_MARKER_LEN 14 +#define DATA_SEPARATOR_SIZE 16 + +static int write_map(uint8_t *buf, uint32_t size) { + buf[0] = (7 << 5) | (size & 0x1f); + return 1; +} + +static int write_string(uint8_t *buf, const char *str, uint32_t len) { + buf[0] = (2 << 5) | (len & 0x1f); + memcpy(buf + 1, str, len); + return 1 + len; +} + +static int write_uint16(uint8_t *buf, uint16_t value) { + buf[0] = (5 << 5) | 2; + buf[1] = (value >> 8) & 0xff; + buf[2] = value & 0xff; + return 3; +} + +static int write_uint32(uint8_t *buf, uint32_t value) { + buf[0] = (6 << 5) | 4; + buf[1] = (value >> 24) & 0xff; + buf[2] = (value >> 16) & 0xff; + buf[3] = (value >> 8) & 0xff; + buf[4] = value & 0xff; + return 5; +} + +static int write_uint64(uint8_t *buf, uint64_t value) { + buf[0] = (0 << 5) | 8; + buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ + buf[2] = (value >> 56) & 0xff; + buf[3] = (value >> 48) & 0xff; + buf[4] = (value >> 40) & 0xff; + buf[5] = (value >> 32) & 0xff; + buf[6] = (value >> 24) & 0xff; + buf[7] = (value >> 16) & 0xff; + buf[8] = (value >> 8) & 0xff; + buf[9] = value & 0xff; + return 10; +} + +static int write_meta_key(uint8_t *buf, const char *key) { + return write_string(buf, key, strlen(key)); +} + +static void create_bad_epoch_db(const char *path) { + uint32_t node_count = 1; + uint32_t record_size = 24; + uint32_t record_value = node_count + 16; + + size_t metadata_buf_size = 512; + size_t data_size = 10; + size_t search_tree_size = 6; + size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_size + + METADATA_MARKER_LEN + metadata_buf_size; + + uint8_t *file = calloc(1, total_size); + if (!file) { + BAIL_OUT("calloc failed"); + } + + size_t pos = 0; + + /* Search tree: 1 node, 24-bit records */ + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + + /* 16-byte null separator */ + memset(file + pos, 0, DATA_SEPARATOR_SIZE); + pos += DATA_SEPARATOR_SIZE; + + /* Data section: a simple map with one string entry */ + pos += write_map(file + pos, 1); + pos += write_string(file + pos, "ip", 2); + pos += write_string(file + pos, "test", 4); + + /* Metadata marker */ + memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); + pos += METADATA_MARKER_LEN; + + /* Metadata map */ + pos += write_map(file + pos, 9); + + pos += write_meta_key(file + pos, "binary_format_major_version"); + pos += write_uint16(file + pos, 2); + + pos += write_meta_key(file + pos, "binary_format_minor_version"); + pos += write_uint16(file + pos, 0); + + pos += write_meta_key(file + pos, "build_epoch"); + pos += write_uint64(file + pos, UINT64_MAX); + + pos += write_meta_key(file + pos, "database_type"); + pos += write_string(file + pos, "Test", 4); + + pos += write_meta_key(file + pos, "description"); + pos += write_map(file + pos, 0); + + pos += write_meta_key(file + pos, "ip_version"); + pos += write_uint16(file + pos, 4); + + pos += write_meta_key(file + pos, "languages"); + file[pos++] = (0 << 5) | 0; + file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ + + pos += write_meta_key(file + pos, "node_count"); + pos += write_uint32(file + pos, node_count); + + pos += write_meta_key(file + pos, "record_size"); + pos += write_uint16(file + pos, record_size); + + FILE *f = fopen(path, "wb"); + if (!f) { + free(file); + BAIL_OUT("fopen failed"); + } + fwrite(file, 1, pos, f); + fclose(f); + free(file); +} + +void test_bad_epoch(void) { + const char *path = "/tmp/test_bad_epoch.mmdb"; + create_bad_epoch_db(path); + + /* Verify we can at least open the DB without crashing */ + MMDB_s mmdb; + int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-epoch MMDB"); + + if (status != MMDB_SUCCESS) { + diag("MMDB_open failed: %s", MMDB_strerror(status)); + remove(path); + return; + } + + /* Run mmdblookup --verbose via system() and check it doesn't crash. + * We redirect output to /dev/null; the return code tells us + * whether the process survived. Try both possible paths since tests + * may run from either the project root or the t/ directory. */ + char cmd[512]; + const char *binary = "../bin/mmdblookup"; + FILE *test_bin = fopen(binary, "r"); + if (!test_bin) { + binary = "./bin/mmdblookup"; + test_bin = fopen(binary, "r"); + } + if (test_bin) { + fclose(test_bin); + } + + skip(!test_bin, 1, "mmdblookup binary not found"); + snprintf(cmd, + sizeof(cmd), + "%s -f %s -i 1.2.3.4 -v > /dev/null 2>&1", + binary, + path); + int ret = system(cmd); + /* system() returns the exit status; a signal-killed process gives + * a non-zero status. WIFEXITED checks for normal exit. */ + ok(WIFEXITED(ret) && WEXITSTATUS(ret) == 0, + "mmdblookup --verbose with UINT64_MAX build_epoch does not crash"); + end_skip; + + MMDB_close(&mmdb); + remove(path); +} + +int main(void) { + plan(NO_PLAN); + test_bad_epoch(); + done_testing(); +} From 13cb45bce96cf73042daf0915abf2cca416ea7d7 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 12:00:41 -0800 Subject: [PATCH 08/27] Fix integer overflow in find_address_in_search_tree bounds check The addition of node_count and data_section_size was performed in uint32_t arithmetic. On very large databases where the sum exceeds UINT32_MAX, this wraps and causes valid records to be rejected as corrupt. Cast to uint64_t before addition. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 5 ++ src/maxminddb.c | 2 +- t/Makefile.am | 3 +- t/overflow_bounds_t.c | 118 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 t/overflow_bounds_t.c diff --git a/Changes.md b/Changes.md index 8b0290fd..f5f0fa37 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,10 @@ ## 1.13.0 +* Fixed an integer overflow in the search tree bounds check in + `find_address_in_search_tree()`. The addition of `node_count` and + `data_section_size` was performed in `uint32_t` arithmetic, which + could wrap on very large databases, causing valid lookups to be + incorrectly rejected as corrupt. * Fixed a NULL pointer dereference in `mmdblookup` when displaying metadata for a database with an out-of-range `build_epoch`. The `gmtime()` return value is now checked before passing to `strftime()`. diff --git a/src/maxminddb.c b/src/maxminddb.c index 4a2f64e2..69a1a295 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -985,7 +985,7 @@ static int find_address_in_search_tree(const MMDB_s *const mmdb, result->netmask = current_bit; - if (value >= node_count + mmdb->data_section_size) { + if (value >= (uint64_t)node_count + mmdb->data_section_size) { // The pointer points off the end of the database. return MMDB_CORRUPT_SEARCH_TREE_ERROR; } diff --git a/t/Makefile.am b/t/Makefile.am index a2e994ba..eb7f1ad5 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -21,7 +21,8 @@ check_PROGRAMS = \ data-pool-t data_types_t double_close_t dump_t gai_error_t get_value_t \ get_value_pointer_bug_t \ ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ - metadata_pointers_t no_map_get_value_t read_node_t threads_t version_t + metadata_pointers_t no_map_get_value_t overflow_bounds_t read_node_t \ + threads_t version_t data_pool_t_LDFLAGS = $(AM_LDFLAGS) -lm data_pool_t_SOURCES = data-pool-t.c ../src/data-pool.c diff --git a/t/overflow_bounds_t.c b/t/overflow_bounds_t.c new file mode 100644 index 00000000..e12b6eb9 --- /dev/null +++ b/t/overflow_bounds_t.c @@ -0,0 +1,118 @@ +#include "maxminddb_test_helper.h" +#include + +/* MMDB binary format constants */ +#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" +#define METADATA_MARKER_LEN 14 +#define DATA_SEPARATOR_SIZE 16 + +static int write_map(uint8_t *buf, uint32_t size) { + buf[0] = (7 << 5) | (size & 0x1f); + return 1; +} + +static int write_string(uint8_t *buf, const char *str, uint32_t len) { + buf[0] = (2 << 5) | (len & 0x1f); + memcpy(buf + 1, str, len); + return 1 + len; +} + +static int write_uint16(uint8_t *buf, uint16_t value) { + buf[0] = (5 << 5) | 2; + buf[1] = (value >> 8) & 0xff; + buf[2] = value & 0xff; + return 3; +} + +static int write_uint32(uint8_t *buf, uint32_t value) { + buf[0] = (6 << 5) | 4; + buf[1] = (value >> 24) & 0xff; + buf[2] = (value >> 16) & 0xff; + buf[3] = (value >> 8) & 0xff; + buf[4] = value & 0xff; + return 5; +} + +static int write_uint64(uint8_t *buf, uint64_t value) { + buf[0] = (0 << 5) | 8; + buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ + buf[2] = (value >> 56) & 0xff; + buf[3] = (value >> 48) & 0xff; + buf[4] = (value >> 40) & 0xff; + buf[5] = (value >> 32) & 0xff; + buf[6] = (value >> 24) & 0xff; + buf[7] = (value >> 16) & 0xff; + buf[8] = (value >> 8) & 0xff; + buf[9] = value & 0xff; + return 10; +} + +static int write_meta_key(uint8_t *buf, const char *key) { + return write_string(buf, key, strlen(key)); +} + +/* + * Test that the bounds check in find_address_in_search_tree uses 64-bit + * arithmetic. We can't realistically create a database large enough to + * trigger the uint32 overflow (it would require billions of nodes), but + * we can verify the fix doesn't regress normal lookups. + * + * The real protection is the cast to uint64_t before addition, + * matching the pattern used elsewhere in the codebase. + */ +void test_normal_lookup_still_works(void) { + char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); + MMDB_s *mmdb = open_ok(db_file, MMDB_MODE_MMAP, "mmap mode"); + free(db_file); + + if (!mmdb) { + return; + } + + int gai_error, mmdb_error; + (void)MMDB_lookup_string(mmdb, "1.1.1.1", &gai_error, &mmdb_error); + + cmp_ok(gai_error, "==", 0, "no gai error"); + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "no mmdb error"); + + MMDB_close(mmdb); + free(mmdb); +} + +void test_record_type_bounds(void) { + /* Test that record_type correctly handles values near the boundary. + * With the uint64_t cast fix, valid records should still be classified + * correctly. */ + char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); + MMDB_s *mmdb = open_ok(db_file, MMDB_MODE_MMAP, "mmap mode"); + free(db_file); + + if (!mmdb) { + return; + } + + /* Read node 0 and verify records are valid types */ + MMDB_search_node_s node; + int status = MMDB_read_node(mmdb, 0, &node); + cmp_ok(status, "==", MMDB_SUCCESS, "MMDB_read_node succeeded"); + + ok(node.left_record_type == MMDB_RECORD_TYPE_SEARCH_NODE || + node.left_record_type == MMDB_RECORD_TYPE_EMPTY || + node.left_record_type == MMDB_RECORD_TYPE_DATA, + "left record type is valid"); + + ok(node.right_record_type == MMDB_RECORD_TYPE_SEARCH_NODE || + node.right_record_type == MMDB_RECORD_TYPE_EMPTY || + node.right_record_type == MMDB_RECORD_TYPE_DATA, + "right record type is valid"); + + MMDB_close(mmdb); + free(mmdb); +} + +int main(void) { + plan(NO_PLAN); + test_normal_lookup_still_works(); + test_record_type_bounds(); + done_testing(); +} From c3bccc08218a94d4d182809cfe7751bf3a032927 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 12:01:21 -0800 Subject: [PATCH 09/27] Fix printf format specifiers in mmdblookup dump_meta Use PRIu32/PRIu16/PRIu64 from instead of %i and %llu for metadata fields. The previous specifiers were technically undefined behavior and could corrupt subsequent variadic arguments on platforms where uint64_t and unsigned long long have different representations. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 4 ++++ bin/mmdblookup.c | 11 ++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index f5f0fa37..3b96712d 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,9 @@ ## 1.13.0 +* Fixed printf format specifier mismatches in `mmdblookup`'s metadata + dump. `%i` was used for unsigned types and `%llu` for `uint64_t`, + which is technically undefined behavior. Now uses the portable + `PRIu32`, `PRIu16`, and `PRIu64` macros from ``. * Fixed an integer overflow in the search tree bounds check in `find_address_in_search_tree()`. The addition of `node_count` and `data_section_size` was performed in `uint32_t` arithmetic, which diff --git a/bin/mmdblookup.c b/bin/mmdblookup.c index 07034cfb..fbae4206 100644 --- a/bin/mmdblookup.c +++ b/bin/mmdblookup.c @@ -8,6 +8,7 @@ #include "maxminddb.h" #include #include +#include #ifndef _WIN32 #include #endif @@ -346,11 +347,11 @@ static MMDB_s open_or_die(const char *fname) { static void dump_meta(MMDB_s *mmdb) { const char *meta_dump = "\n" " Database metadata\n" - " Node count: %i\n" - " Record size: %i bits\n" - " IP version: IPv%i\n" - " Binary format: %i.%i\n" - " Build epoch: %llu (%s)\n" + " Node count: %" PRIu32 "\n" + " Record size: %" PRIu16 " bits\n" + " IP version: IPv%" PRIu16 "\n" + " Binary format: %" PRIu16 ".%" PRIu16 "\n" + " Build epoch: %" PRIu64 " (%s)\n" " Type: %s\n" " Languages: "; From 4f418a986a414441d54a4432e23a22fbaa3b2133 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 12:02:01 -0800 Subject: [PATCH 10/27] Fix integer overflow in MMDB_read_node/find_ipv4_start_node pointer arithmetic Cast node_number/node_value to uint64_t before multiplying by record_length to prevent uint32 overflow on very large databases. This matches the pattern already used in find_address_in_search_tree. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 5 +++++ src/maxminddb.c | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Changes.md b/Changes.md index 3b96712d..44b344d5 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,10 @@ ## 1.13.0 +* Fixed integer overflow in `MMDB_read_node()` and `find_ipv4_start_node()` + pointer arithmetic. The `node_number * record_length` multiplication + was performed in `uint32_t`, which could overflow for very large + databases. Now cast to `uint64_t` before multiplying, matching the + pattern already used in `find_address_in_search_tree()`. * Fixed printf format specifier mismatches in `mmdblookup`'s metadata dump. `%i` was used for unsigned types and `%llu` for `uint64_t`, which is technically undefined behavior. Now uses the portable diff --git a/src/maxminddb.c b/src/maxminddb.c index 69a1a295..3104caf8 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1045,7 +1045,8 @@ static int find_ipv4_start_node(MMDB_s *const mmdb) { uint32_t node_count = mmdb->metadata.node_count; for (netmask = 0; netmask < 96 && node_value < node_count; netmask++) { - record_pointer = &search_tree[node_value * record_info.record_length]; + record_pointer = + &search_tree[(uint64_t)node_value * record_info.record_length]; if (record_pointer + record_info.record_length > mmdb->data_section) { return MMDB_CORRUPT_SEARCH_TREE_ERROR; } @@ -1109,7 +1110,7 @@ int MMDB_read_node(const MMDB_s *const mmdb, const uint8_t *search_tree = mmdb->file_content; const uint8_t *record_pointer = - &search_tree[node_number * record_info.record_length]; + &search_tree[(uint64_t)node_number * record_info.record_length]; node->left_record = record_info.left_record_getter(record_pointer); record_pointer += record_info.right_record_offset; node->right_record = record_info.right_record_getter(record_pointer); From c185bb8b54ff57e963155d77ea3068d9e46536b4 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 12:02:40 -0800 Subject: [PATCH 11/27] Use GetFileSizeEx for Windows file size GetFileSize(fd, NULL) discards the upper 32 bits of the file size. Replace with GetFileSizeEx which returns the full 64-bit size, and add an SSIZE_MAX bounds check matching the POSIX code path. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 4 ++++ src/maxminddb.c | 15 ++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index 44b344d5..6a6778bc 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,9 @@ ## 1.13.0 +* On Windows, `GetFileSize()` was replaced with `GetFileSizeEx()` to + correctly handle files larger than 4GB. The previous code passed + `NULL` for the high DWORD, discarding the upper 32 bits of the file + size. * Fixed integer overflow in `MMDB_read_node()` and `find_ipv4_start_node()` pointer arithmetic. The `node_number * record_length` multiplication was performed in `uint32_t`, which could overflow for very large diff --git a/src/maxminddb.c b/src/maxminddb.c index 3104caf8..76427b97 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -367,7 +367,7 @@ static LPWSTR utf8_to_utf16(const char *utf8_str) { } static int map_file(MMDB_s *const mmdb) { - DWORD size; + ssize_t size; int status = MMDB_SUCCESS; HANDLE mmh = NULL; HANDLE fd = INVALID_HANDLE_VALUE; @@ -387,12 +387,17 @@ static int map_file(MMDB_s *const mmdb) { status = MMDB_FILE_OPEN_ERROR; goto cleanup; } - size = GetFileSize(fd, NULL); - if (size == INVALID_FILE_SIZE) { - status = MMDB_FILE_OPEN_ERROR; + LARGE_INTEGER file_size; + if (!GetFileSizeEx(fd, &file_size)) { + status = MMDB_IO_ERROR; + goto cleanup; + } + if (file_size.QuadPart < 0 || file_size.QuadPart > SSIZE_MAX) { + status = MMDB_IO_ERROR; goto cleanup; } - mmh = CreateFileMapping(fd, NULL, PAGE_READONLY, 0, size, NULL); + size = (ssize_t)file_size.QuadPart; + mmh = CreateFileMapping(fd, NULL, PAGE_READONLY, 0, 0, NULL); /* Microsoft documentation for CreateFileMapping indicates this returns NULL not INVALID_HANDLE_VALUE on error */ if (NULL == mmh) { From c1d353a260a102983e2179d11a323a22d1d5250b Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 12:04:06 -0800 Subject: [PATCH 12/27] Validate array/map size in get_entry_data_list against remaining data A crafted database could claim millions of array/map elements while only having a few bytes of data, causing disproportionate memory allocation (~40x amplification). Now validate that the claimed element count does not exceed the remaining data section bytes before entering the allocation loop. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 5 + src/maxminddb.c | 11 ++ t/Makefile.am | 4 +- t/bad_data_size_t.c | 349 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 367 insertions(+), 2 deletions(-) create mode 100644 t/bad_data_size_t.c diff --git a/Changes.md b/Changes.md index 6a6778bc..21484fbd 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,10 @@ ## 1.13.0 +* `MMDB_get_entry_data_list()` now validates that the claimed array/map + size is plausible given the remaining bytes in the data section. A + crafted database could previously claim millions of array elements + while only having a few bytes of data, causing disproportionate memory + allocation (memory amplification DoS). * On Windows, `GetFileSize()` was replaced with `GetFileSizeEx()` to correctly handle files larger than 4GB. The previous code passed `NULL` for the high DWORD, discarding the upper 32 bits of the file diff --git a/src/maxminddb.c b/src/maxminddb.c index 76427b97..11daa27d 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1725,6 +1725,11 @@ static int get_entry_data_list(const MMDB_s *const mmdb, case MMDB_DATA_TYPE_ARRAY: { uint32_t array_size = entry_data_list->entry_data.data_size; uint32_t array_offset = entry_data_list->entry_data.offset_to_next; + if (array_offset >= mmdb->data_section_size || + array_size > mmdb->data_section_size - array_offset) { + DEBUG_MSG("array size exceeds remaining data section"); + return MMDB_INVALID_DATA_ERROR; + } while (array_size-- > 0) { MMDB_entry_data_list_s *entry_data_list_to = data_pool_alloc(pool); @@ -1748,6 +1753,12 @@ static int get_entry_data_list(const MMDB_s *const mmdb, uint32_t size = entry_data_list->entry_data.data_size; offset = entry_data_list->entry_data.offset_to_next; + /* Each map entry needs at least a key and a value (1 byte each). */ + if (offset >= mmdb->data_section_size || + size > (mmdb->data_section_size - offset) / 2) { + DEBUG_MSG("map size exceeds remaining data section"); + return MMDB_INVALID_DATA_ERROR; + } while (size-- > 0) { MMDB_entry_data_list_s *list_key = data_pool_alloc(pool); if (!list_key) { diff --git a/t/Makefile.am b/t/Makefile.am index eb7f1ad5..b04abcaa 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -16,8 +16,8 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ libtap/tap.c libtap/tap.h maxmind-db check_PROGRAMS = \ - bad_pointers_t bad_databases_t bad_epoch_t bad_indent_t basic_lookup_t \ - data_entry_list_t \ + bad_pointers_t bad_databases_t bad_data_size_t bad_epoch_t bad_indent_t \ + basic_lookup_t data_entry_list_t \ data-pool-t data_types_t double_close_t dump_t gai_error_t get_value_t \ get_value_pointer_bug_t \ ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ diff --git a/t/bad_data_size_t.c b/t/bad_data_size_t.c new file mode 100644 index 00000000..c5eac564 --- /dev/null +++ b/t/bad_data_size_t.c @@ -0,0 +1,349 @@ +#include "maxminddb_test_helper.h" +#include + +/* MMDB binary format constants */ +#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" +#define METADATA_MARKER_LEN 14 +#define DATA_SEPARATOR_SIZE 16 + +static int write_map(uint8_t *buf, uint32_t size) { + buf[0] = (7 << 5) | (size & 0x1f); + return 1; +} + +static int write_string(uint8_t *buf, const char *str, uint32_t len) { + buf[0] = (2 << 5) | (len & 0x1f); + memcpy(buf + 1, str, len); + return 1 + len; +} + +static int write_uint16(uint8_t *buf, uint16_t value) { + buf[0] = (5 << 5) | 2; + buf[1] = (value >> 8) & 0xff; + buf[2] = value & 0xff; + return 3; +} + +static int write_uint32(uint8_t *buf, uint32_t value) { + buf[0] = (6 << 5) | 4; + buf[1] = (value >> 24) & 0xff; + buf[2] = (value >> 16) & 0xff; + buf[3] = (value >> 8) & 0xff; + buf[4] = value & 0xff; + return 5; +} + +static int write_uint64(uint8_t *buf, uint64_t value) { + buf[0] = (0 << 5) | 8; + buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ + buf[2] = (value >> 56) & 0xff; + buf[3] = (value >> 48) & 0xff; + buf[4] = (value >> 40) & 0xff; + buf[5] = (value >> 32) & 0xff; + buf[6] = (value >> 24) & 0xff; + buf[7] = (value >> 16) & 0xff; + buf[8] = (value >> 8) & 0xff; + buf[9] = value & 0xff; + return 10; +} + +static int write_meta_key(uint8_t *buf, const char *key) { + return write_string(buf, key, strlen(key)); +} + +/* + * Write a map control byte with a large size using case-31 encoding. + * Type 7 (map) fits in the base types: control byte = (7 << 5) | size_marker. + * For case-31 size encoding: control byte size bits = 31, + * then 3 bytes for (size - 65821). + */ +static int write_large_map(uint8_t *buf, uint32_t size) { + uint32_t adjusted = size - 65821; + buf[0] = (7 << 5) | 31; /* type 7 (map), size = case 31 */ + buf[1] = (adjusted >> 16) & 0xff; + buf[2] = (adjusted >> 8) & 0xff; + buf[3] = adjusted & 0xff; + return 4; +} + +/* + * Write an array control byte with a large size using case-31 encoding. + * Type 11 (array) is extended: control byte = (0 << 5) | size_marker, + * followed by extended type byte 4. + * For case-31 size encoding: control byte size bits = 31, + * then 3 bytes for (size - 65821). + */ +static int write_large_array(uint8_t *buf, uint32_t size) { + uint32_t adjusted = size - 65821; + buf[0] = (0 << 5) | 31; /* extended type, size = case 31 */ + buf[1] = 4; /* extended type: 7 + 4 = 11 (array) */ + buf[2] = (adjusted >> 16) & 0xff; + buf[3] = (adjusted >> 8) & 0xff; + buf[4] = adjusted & 0xff; + return 5; +} + +/* + * Create a crafted MMDB with an array claiming 1,000,000 elements but + * only a few bytes of actual data. + */ +static void create_bad_data_size_db(const char *path) { + uint32_t node_count = 1; + uint32_t record_size = 24; + uint32_t record_value = node_count + 16; + + /* The data section needs enough bytes for the array header + a few + * elements but NOT enough for 1M elements. */ + size_t data_buf_size = 64; + size_t metadata_buf_size = 512; + size_t search_tree_size = 6; + size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size + + METADATA_MARKER_LEN + metadata_buf_size; + + uint8_t *file = calloc(1, total_size); + if (!file) { + BAIL_OUT("calloc failed"); + } + + size_t pos = 0; + + /* Search tree: 1 node, 24-bit records, both pointing to data */ + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + + /* 16-byte null separator */ + memset(file + pos, 0, DATA_SEPARATOR_SIZE); + pos += DATA_SEPARATOR_SIZE; + + /* Data section: array claiming 1,000,000 elements */ + pos += write_large_array(file + pos, 1000000); + + /* Only write a few bytes of actual data (way less than 1M entries) */ + pos += write_string(file + pos, "x", 1); + pos += write_string(file + pos, "y", 1); + + /* Pad to data_buf_size */ + size_t data_end = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size; + if (pos < data_end) { + pos = data_end; + } + + /* Metadata marker */ + memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); + pos += METADATA_MARKER_LEN; + + /* Metadata map */ + pos += write_map(file + pos, 9); + + pos += write_meta_key(file + pos, "binary_format_major_version"); + pos += write_uint16(file + pos, 2); + + pos += write_meta_key(file + pos, "binary_format_minor_version"); + pos += write_uint16(file + pos, 0); + + pos += write_meta_key(file + pos, "build_epoch"); + pos += write_uint64(file + pos, 1000000000ULL); + + pos += write_meta_key(file + pos, "database_type"); + pos += write_string(file + pos, "Test", 4); + + pos += write_meta_key(file + pos, "description"); + pos += write_map(file + pos, 0); + + pos += write_meta_key(file + pos, "ip_version"); + pos += write_uint16(file + pos, 4); + + pos += write_meta_key(file + pos, "languages"); + file[pos++] = (0 << 5) | 0; + file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ + + pos += write_meta_key(file + pos, "node_count"); + pos += write_uint32(file + pos, node_count); + + pos += write_meta_key(file + pos, "record_size"); + pos += write_uint16(file + pos, record_size); + + FILE *f = fopen(path, "wb"); + if (!f) { + free(file); + BAIL_OUT("fopen failed"); + } + fwrite(file, 1, pos, f); + fclose(f); + free(file); +} + +void test_bad_data_size_rejected(void) { + const char *path = "/tmp/test_bad_data_size.mmdb"; + create_bad_data_size_db(path); + + MMDB_s mmdb; + int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-data-size MMDB"); + + if (status != MMDB_SUCCESS) { + diag("MMDB_open failed: %s", MMDB_strerror(status)); + remove(path); + return; + } + + int gai_error, mmdb_error; + MMDB_lookup_result_s result = + MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); + ok(result.found_entry, "entry found"); + + if (result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); + cmp_ok(status, + "==", + MMDB_INVALID_DATA_ERROR, + "MMDB_get_entry_data_list returns INVALID_DATA_ERROR " + "for array with size exceeding remaining data"); + MMDB_free_entry_data_list(entry_data_list); + } + + MMDB_close(&mmdb); + remove(path); +} + +/* + * Create a crafted MMDB with a map claiming 1,000,000 entries but + * only a few bytes of actual data. + */ +static void create_bad_map_size_db(const char *path) { + uint32_t node_count = 1; + uint32_t record_size = 24; + uint32_t record_value = node_count + 16; + + size_t data_buf_size = 64; + size_t metadata_buf_size = 512; + size_t search_tree_size = 6; + size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size + + METADATA_MARKER_LEN + metadata_buf_size; + + uint8_t *file = calloc(1, total_size); + if (!file) { + BAIL_OUT("calloc failed"); + } + + size_t pos = 0; + + /* Search tree: 1 node, 24-bit records, both pointing to data */ + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + file[pos++] = (record_value >> 16) & 0xff; + file[pos++] = (record_value >> 8) & 0xff; + file[pos++] = record_value & 0xff; + + /* 16-byte null separator */ + memset(file + pos, 0, DATA_SEPARATOR_SIZE); + pos += DATA_SEPARATOR_SIZE; + + /* Data section: map claiming 1,000,000 entries */ + pos += write_large_map(file + pos, 1000000); + + /* Only write a couple of key-value pairs (way less than 1M entries) */ + pos += write_string(file + pos, "k", 1); + pos += write_string(file + pos, "v", 1); + + /* Pad to data_buf_size */ + size_t data_end = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size; + if (pos < data_end) { + pos = data_end; + } + + /* Metadata marker */ + memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); + pos += METADATA_MARKER_LEN; + + /* Metadata map */ + pos += write_map(file + pos, 9); + + pos += write_meta_key(file + pos, "binary_format_major_version"); + pos += write_uint16(file + pos, 2); + + pos += write_meta_key(file + pos, "binary_format_minor_version"); + pos += write_uint16(file + pos, 0); + + pos += write_meta_key(file + pos, "build_epoch"); + pos += write_uint64(file + pos, 1000000000ULL); + + pos += write_meta_key(file + pos, "database_type"); + pos += write_string(file + pos, "Test", 4); + + pos += write_meta_key(file + pos, "description"); + pos += write_map(file + pos, 0); + + pos += write_meta_key(file + pos, "ip_version"); + pos += write_uint16(file + pos, 4); + + pos += write_meta_key(file + pos, "languages"); + file[pos++] = (0 << 5) | 0; + file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ + + pos += write_meta_key(file + pos, "node_count"); + pos += write_uint32(file + pos, node_count); + + pos += write_meta_key(file + pos, "record_size"); + pos += write_uint16(file + pos, record_size); + + FILE *f = fopen(path, "wb"); + if (!f) { + free(file); + BAIL_OUT("fopen failed"); + } + fwrite(file, 1, pos, f); + fclose(f); + free(file); +} + +void test_bad_map_size_rejected(void) { + const char *path = "/tmp/test_bad_map_size.mmdb"; + create_bad_map_size_db(path); + + MMDB_s mmdb; + int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-map-size MMDB"); + + if (status != MMDB_SUCCESS) { + diag("MMDB_open failed: %s", MMDB_strerror(status)); + remove(path); + return; + } + + int gai_error, mmdb_error; + MMDB_lookup_result_s result = + MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); + ok(result.found_entry, "entry found"); + + if (result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); + cmp_ok(status, + "==", + MMDB_INVALID_DATA_ERROR, + "MMDB_get_entry_data_list returns INVALID_DATA_ERROR " + "for map with size exceeding remaining data"); + MMDB_free_entry_data_list(entry_data_list); + } + + MMDB_close(&mmdb); + remove(path); +} + +int main(void) { + plan(NO_PLAN); + test_bad_data_size_rejected(); + test_bad_map_size_rejected(); + done_testing(); +} From dbb6558a1dee836fe509ae28435881e40291a44f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 20 Feb 2026 13:16:48 -0800 Subject: [PATCH 13/27] Extract shared MMDB test writer helpers into mmdb_test_writer.h bad_epoch_t.c, overflow_bounds_t.c, and bad_data_size_t.c all had identical copies of write_map, write_string, write_uint16, write_uint32, write_uint64, write_meta_key, and the METADATA_MARKER defines. Move these into a shared static-inline header. Co-Authored-By: Claude Opus 4.6 --- t/Makefile.am | 2 +- t/bad_data_size_t.c | 51 +---------------------------------- t/bad_epoch_t.c | 51 +---------------------------------- t/max_depth_t.c | 53 +----------------------------------- t/mmdb_test_writer.h | 63 +++++++++++++++++++++++++++++++++++++++++++ t/overflow_bounds_t.c | 51 +---------------------------------- 6 files changed, 68 insertions(+), 203 deletions(-) create mode 100644 t/mmdb_test_writer.h diff --git a/t/Makefile.am b/t/Makefile.am index b04abcaa..f38a2cc2 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -13,7 +13,7 @@ libmmdbtest_la_SOURCES = maxminddb_test_helper.c maxminddb_test_helper.h EXTRA_DIST = compile_c++_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 + libtap/tap.c libtap/tap.h maxmind-db mmdb_test_writer.h check_PROGRAMS = \ bad_pointers_t bad_databases_t bad_data_size_t bad_epoch_t bad_indent_t \ diff --git a/t/bad_data_size_t.c b/t/bad_data_size_t.c index c5eac564..fa981c54 100644 --- a/t/bad_data_size_t.c +++ b/t/bad_data_size_t.c @@ -1,56 +1,7 @@ #include "maxminddb_test_helper.h" +#include "mmdb_test_writer.h" #include -/* MMDB binary format constants */ -#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" -#define METADATA_MARKER_LEN 14 -#define DATA_SEPARATOR_SIZE 16 - -static int write_map(uint8_t *buf, uint32_t size) { - buf[0] = (7 << 5) | (size & 0x1f); - return 1; -} - -static int write_string(uint8_t *buf, const char *str, uint32_t len) { - buf[0] = (2 << 5) | (len & 0x1f); - memcpy(buf + 1, str, len); - return 1 + len; -} - -static int write_uint16(uint8_t *buf, uint16_t value) { - buf[0] = (5 << 5) | 2; - buf[1] = (value >> 8) & 0xff; - buf[2] = value & 0xff; - return 3; -} - -static int write_uint32(uint8_t *buf, uint32_t value) { - buf[0] = (6 << 5) | 4; - buf[1] = (value >> 24) & 0xff; - buf[2] = (value >> 16) & 0xff; - buf[3] = (value >> 8) & 0xff; - buf[4] = value & 0xff; - return 5; -} - -static int write_uint64(uint8_t *buf, uint64_t value) { - buf[0] = (0 << 5) | 8; - buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ - buf[2] = (value >> 56) & 0xff; - buf[3] = (value >> 48) & 0xff; - buf[4] = (value >> 40) & 0xff; - buf[5] = (value >> 32) & 0xff; - buf[6] = (value >> 24) & 0xff; - buf[7] = (value >> 16) & 0xff; - buf[8] = (value >> 8) & 0xff; - buf[9] = value & 0xff; - return 10; -} - -static int write_meta_key(uint8_t *buf, const char *key) { - return write_string(buf, key, strlen(key)); -} - /* * Write a map control byte with a large size using case-31 encoding. * Type 7 (map) fits in the base types: control byte = (7 << 5) | size_marker. diff --git a/t/bad_epoch_t.c b/t/bad_epoch_t.c index e92f4324..a8354117 100644 --- a/t/bad_epoch_t.c +++ b/t/bad_epoch_t.c @@ -1,56 +1,7 @@ #include "maxminddb_test_helper.h" +#include "mmdb_test_writer.h" #include -/* MMDB binary format constants */ -#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" -#define METADATA_MARKER_LEN 14 -#define DATA_SEPARATOR_SIZE 16 - -static int write_map(uint8_t *buf, uint32_t size) { - buf[0] = (7 << 5) | (size & 0x1f); - return 1; -} - -static int write_string(uint8_t *buf, const char *str, uint32_t len) { - buf[0] = (2 << 5) | (len & 0x1f); - memcpy(buf + 1, str, len); - return 1 + len; -} - -static int write_uint16(uint8_t *buf, uint16_t value) { - buf[0] = (5 << 5) | 2; - buf[1] = (value >> 8) & 0xff; - buf[2] = value & 0xff; - return 3; -} - -static int write_uint32(uint8_t *buf, uint32_t value) { - buf[0] = (6 << 5) | 4; - buf[1] = (value >> 24) & 0xff; - buf[2] = (value >> 16) & 0xff; - buf[3] = (value >> 8) & 0xff; - buf[4] = value & 0xff; - return 5; -} - -static int write_uint64(uint8_t *buf, uint64_t value) { - buf[0] = (0 << 5) | 8; - buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ - buf[2] = (value >> 56) & 0xff; - buf[3] = (value >> 48) & 0xff; - buf[4] = (value >> 40) & 0xff; - buf[5] = (value >> 32) & 0xff; - buf[6] = (value >> 24) & 0xff; - buf[7] = (value >> 16) & 0xff; - buf[8] = (value >> 8) & 0xff; - buf[9] = value & 0xff; - return 10; -} - -static int write_meta_key(uint8_t *buf, const char *key) { - return write_string(buf, key, strlen(key)); -} - static void create_bad_epoch_db(const char *path) { uint32_t node_count = 1; uint32_t record_size = 24; diff --git a/t/max_depth_t.c b/t/max_depth_t.c index 128be6f1..530fc10d 100644 --- a/t/max_depth_t.c +++ b/t/max_depth_t.c @@ -1,58 +1,7 @@ #include "maxminddb_test_helper.h" +#include "mmdb_test_writer.h" #include -/* MMDB binary format constants */ -#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" -#define METADATA_MARKER_LEN 14 -#define DATA_SEPARATOR_SIZE 16 - -/* Write a map with size entries (size must be < 29) */ -static int write_map(uint8_t *buf, uint32_t size) { - buf[0] = (7 << 5) | (size & 0x1f); - return 1; -} - -/* Write a utf8_string (type 2). For short strings (len < 29). */ -static int write_string(uint8_t *buf, const char *str, uint32_t len) { - buf[0] = (2 << 5) | (len & 0x1f); - memcpy(buf + 1, str, len); - return 1 + len; -} - -static int write_uint16(uint8_t *buf, uint16_t value) { - buf[0] = (5 << 5) | 2; - buf[1] = (value >> 8) & 0xff; - buf[2] = value & 0xff; - return 3; -} - -static int write_uint32(uint8_t *buf, uint32_t value) { - buf[0] = (6 << 5) | 4; - buf[1] = (value >> 24) & 0xff; - buf[2] = (value >> 16) & 0xff; - buf[3] = (value >> 8) & 0xff; - buf[4] = value & 0xff; - return 5; -} - -static int write_uint64(uint8_t *buf, uint64_t value) { - buf[0] = (0 << 5) | 8; - buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ - buf[2] = (value >> 56) & 0xff; - buf[3] = (value >> 48) & 0xff; - buf[4] = (value >> 40) & 0xff; - buf[5] = (value >> 32) & 0xff; - buf[6] = (value >> 24) & 0xff; - buf[7] = (value >> 16) & 0xff; - buf[8] = (value >> 8) & 0xff; - buf[9] = value & 0xff; - return 10; -} - -static int write_meta_key(uint8_t *buf, const char *key) { - return write_string(buf, key, strlen(key)); -} - /* * Create a crafted MMDB file with deeply nested maps and write it to path. * The data section contains: {"a": {"a": {"a": ... {"a": "x"} ...}}} diff --git a/t/mmdb_test_writer.h b/t/mmdb_test_writer.h new file mode 100644 index 00000000..14f71d59 --- /dev/null +++ b/t/mmdb_test_writer.h @@ -0,0 +1,63 @@ +#ifndef MMDB_TEST_WRITER_H +#define MMDB_TEST_WRITER_H + +/* + * Shared helpers for tests that construct MMDB files in memory. + * Each function writes one MMDB data-format element into buf and returns + * the number of bytes written. + */ + +#include +#include + +/* MMDB binary format constants */ +#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" +#define METADATA_MARKER_LEN 14 +#define DATA_SEPARATOR_SIZE 16 + +static inline int write_map(uint8_t *buf, uint32_t size) { + buf[0] = (7 << 5) | (size & 0x1f); + return 1; +} + +static inline int write_string(uint8_t *buf, const char *str, uint32_t len) { + buf[0] = (2 << 5) | (len & 0x1f); + memcpy(buf + 1, str, len); + return 1 + len; +} + +static inline int write_uint16(uint8_t *buf, uint16_t value) { + buf[0] = (5 << 5) | 2; + buf[1] = (value >> 8) & 0xff; + buf[2] = value & 0xff; + return 3; +} + +static inline int write_uint32(uint8_t *buf, uint32_t value) { + buf[0] = (6 << 5) | 4; + buf[1] = (value >> 24) & 0xff; + buf[2] = (value >> 16) & 0xff; + buf[3] = (value >> 8) & 0xff; + buf[4] = value & 0xff; + return 5; +} + +static inline int write_uint64(uint8_t *buf, uint64_t value) { + buf[0] = (0 << 5) | 8; + buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ + buf[2] = (value >> 56) & 0xff; + buf[3] = (value >> 48) & 0xff; + buf[4] = (value >> 40) & 0xff; + buf[5] = (value >> 32) & 0xff; + buf[6] = (value >> 24) & 0xff; + buf[7] = (value >> 16) & 0xff; + buf[8] = (value >> 8) & 0xff; + buf[9] = value & 0xff; + return 10; +} + +static inline int write_meta_key(uint8_t *buf, const char *key) { + return write_string(buf, key, strlen(key)); +} + +#endif /* MMDB_TEST_WRITER_H */ diff --git a/t/overflow_bounds_t.c b/t/overflow_bounds_t.c index e12b6eb9..8e229edf 100644 --- a/t/overflow_bounds_t.c +++ b/t/overflow_bounds_t.c @@ -1,56 +1,7 @@ #include "maxminddb_test_helper.h" +#include "mmdb_test_writer.h" #include -/* MMDB binary format constants */ -#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" -#define METADATA_MARKER_LEN 14 -#define DATA_SEPARATOR_SIZE 16 - -static int write_map(uint8_t *buf, uint32_t size) { - buf[0] = (7 << 5) | (size & 0x1f); - return 1; -} - -static int write_string(uint8_t *buf, const char *str, uint32_t len) { - buf[0] = (2 << 5) | (len & 0x1f); - memcpy(buf + 1, str, len); - return 1 + len; -} - -static int write_uint16(uint8_t *buf, uint16_t value) { - buf[0] = (5 << 5) | 2; - buf[1] = (value >> 8) & 0xff; - buf[2] = value & 0xff; - return 3; -} - -static int write_uint32(uint8_t *buf, uint32_t value) { - buf[0] = (6 << 5) | 4; - buf[1] = (value >> 24) & 0xff; - buf[2] = (value >> 16) & 0xff; - buf[3] = (value >> 8) & 0xff; - buf[4] = value & 0xff; - return 5; -} - -static int write_uint64(uint8_t *buf, uint64_t value) { - buf[0] = (0 << 5) | 8; - buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ - buf[2] = (value >> 56) & 0xff; - buf[3] = (value >> 48) & 0xff; - buf[4] = (value >> 40) & 0xff; - buf[5] = (value >> 32) & 0xff; - buf[6] = (value >> 24) & 0xff; - buf[7] = (value >> 16) & 0xff; - buf[8] = (value >> 8) & 0xff; - buf[9] = value & 0xff; - return 10; -} - -static int write_meta_key(uint8_t *buf, const char *key) { - return write_string(buf, key, strlen(key)); -} - /* * Test that the bounds check in find_address_in_search_tree uses 64-bit * arithmetic. We can't realistically create a database large enough to From 04bf5ef994a420fee85826d7aa4e2aa14b747368 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 08:30:01 -0800 Subject: [PATCH 14/27] Add new test programs to t/CMakeLists.txt All 7 new tests were in Makefile.am but missing from CMakeLists.txt. Cross-platform tests: double_close_t, gai_error_t, overflow_bounds_t Unix-only tests: bad_data_size_t, bad_epoch_t, bad_indent_t, max_depth_t Co-Authored-By: Claude Opus 4.6 --- t/CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/CMakeLists.txt b/t/CMakeLists.txt index f0e91f76..433c6e62 100644 --- a/t/CMakeLists.txt +++ b/t/CMakeLists.txt @@ -9,7 +9,9 @@ set(TEST_TARGET_NAMES data_entry_list_t data-pool-t data_types_t + double_close_t dump_t + gai_error_t get_value_pointer_bug_t get_value_t ipv4_start_cache_t @@ -17,6 +19,7 @@ set(TEST_TARGET_NAMES metadata_pointers_t metadata_t no_map_get_value_t + overflow_bounds_t read_node_t version_t ) @@ -24,6 +27,10 @@ set(TEST_TARGET_NAMES if(UNIX) # or if (NOT WIN32) list(APPEND TEST_TARGET_NAMES bad_databases_t + bad_data_size_t + bad_epoch_t + bad_indent_t + max_depth_t threads_t ) find_package(Threads) From ebdb1eccbd35c9d2330778d73ad28717f75cde0d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 08:30:31 -0800 Subject: [PATCH 15/27] NULL data_section and metadata_section after munmap Both are const uint8_t * pointers into the mmap'd region. After munmap they become dangling. NULL them alongside the existing file_content and file_size cleanup in free_mmdb_struct(). Add assertions in double_close_t to verify the new NULL assignments. Co-Authored-By: Claude Opus 4.6 --- src/maxminddb.c | 2 ++ t/double_close_t.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/maxminddb.c b/src/maxminddb.c index 11daa27d..3d4eca68 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1925,6 +1925,8 @@ static void free_mmdb_struct(MMDB_s *const mmdb) { #endif mmdb->file_content = NULL; mmdb->file_size = 0; + mmdb->data_section = NULL; + mmdb->metadata_section = NULL; } if (NULL != mmdb->metadata.database_type) { diff --git a/t/double_close_t.c b/t/double_close_t.c index cf5c9d3c..52052f2c 100644 --- a/t/double_close_t.c +++ b/t/double_close_t.c @@ -16,6 +16,8 @@ void test_double_close(void) { MMDB_close(&mmdb); ok(mmdb.file_content == NULL, "file_content is NULL after first close"); + ok(mmdb.data_section == NULL, "data_section is NULL after close"); + ok(mmdb.metadata_section == NULL, "metadata_section is NULL after close"); cmp_ok(mmdb.metadata.languages.count, "==", 0, From cc91dc6ca6ef64afa4e544af0afcb5334be4b0ba Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 08:30:53 -0800 Subject: [PATCH 16/27] Add record_pointer bounds check to MMDB_read_node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MMDB_read_node is a public API. The node_number >= node_count check validates the logical number, but a truncated database file can still produce an out-of-bounds pointer. Both internal functions (find_address_in_search_tree, find_ipv4_start_node) already have this check — this brings the public API to parity. Returns MMDB_CORRUPT_SEARCH_TREE_ERROR matching the internal functions. Co-Authored-By: Claude Opus 4.6 --- src/maxminddb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/maxminddb.c b/src/maxminddb.c index 3d4eca68..e0617174 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1116,6 +1116,9 @@ int MMDB_read_node(const MMDB_s *const mmdb, const uint8_t *search_tree = mmdb->file_content; const uint8_t *record_pointer = &search_tree[(uint64_t)node_number * record_info.record_length]; + if (record_pointer + record_info.record_length > mmdb->data_section) { + return MMDB_CORRUPT_SEARCH_TREE_ERROR; + } node->left_record = record_info.left_record_getter(record_pointer); record_pointer += record_info.right_record_offset; node->right_record = record_info.right_record_getter(record_pointer); From db24787afd5feb8fc0090001e8b9a4ae5c7caf72 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 08:31:29 -0800 Subject: [PATCH 17/27] Add fwrite error checking in test files Check fwrite return values in test DB creation functions. On failure, clean up the file handle and allocated memory before bailing out. Affected files: bad_epoch_t.c, bad_data_size_t.c (2 calls), max_depth_t.c Co-Authored-By: Claude Opus 4.6 --- t/bad_data_size_t.c | 12 ++++++++++-- t/bad_epoch_t.c | 6 +++++- t/max_depth_t.c | 6 +++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/t/bad_data_size_t.c b/t/bad_data_size_t.c index fa981c54..c06d5356 100644 --- a/t/bad_data_size_t.c +++ b/t/bad_data_size_t.c @@ -123,7 +123,11 @@ static void create_bad_data_size_db(const char *path) { free(file); BAIL_OUT("fopen failed"); } - fwrite(file, 1, pos, f); + if (fwrite(file, 1, pos, f) != pos) { + fclose(f); + free(file); + BAIL_OUT("fwrite failed"); + } fclose(f); free(file); } @@ -251,7 +255,11 @@ static void create_bad_map_size_db(const char *path) { free(file); BAIL_OUT("fopen failed"); } - fwrite(file, 1, pos, f); + if (fwrite(file, 1, pos, f) != pos) { + fclose(f); + free(file); + BAIL_OUT("fwrite failed"); + } fclose(f); free(file); } diff --git a/t/bad_epoch_t.c b/t/bad_epoch_t.c index a8354117..fde51860 100644 --- a/t/bad_epoch_t.c +++ b/t/bad_epoch_t.c @@ -77,7 +77,11 @@ static void create_bad_epoch_db(const char *path) { free(file); BAIL_OUT("fopen failed"); } - fwrite(file, 1, pos, f); + if (fwrite(file, 1, pos, f) != pos) { + fclose(f); + free(file); + BAIL_OUT("fwrite failed"); + } fclose(f); free(file); } diff --git a/t/max_depth_t.c b/t/max_depth_t.c index 530fc10d..c67e55c3 100644 --- a/t/max_depth_t.c +++ b/t/max_depth_t.c @@ -84,7 +84,11 @@ static void create_deep_nesting_db(const char *path, int depth) { free(file); BAIL_OUT("fopen failed"); } - fwrite(file, 1, pos, f); + if (fwrite(file, 1, pos, f) != pos) { + fclose(f); + free(file); + BAIL_OUT("fwrite failed"); + } fclose(f); free(file); } From 1e3a55253e8513f31d6ccadeacf078d845c67375 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 08:31:55 -0800 Subject: [PATCH 18/27] Fix mixed tab/space indentation in Makefile.am The data-pool-t continuation line used spaces where all other continuation lines use tabs. Co-Authored-By: Claude Opus 4.6 --- t/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/Makefile.am b/t/Makefile.am index f38a2cc2..8f98ac49 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -18,7 +18,7 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ check_PROGRAMS = \ bad_pointers_t bad_databases_t bad_data_size_t bad_epoch_t bad_indent_t \ basic_lookup_t data_entry_list_t \ - data-pool-t data_types_t double_close_t dump_t gai_error_t get_value_t \ + data-pool-t data_types_t double_close_t dump_t gai_error_t get_value_t \ get_value_pointer_bug_t \ ipv4_start_cache_t ipv6_lookup_in_ipv4_t max_depth_t metadata_t \ metadata_pointers_t no_map_get_value_t overflow_bounds_t read_node_t \ From 91e17e26cf3615bb9757257b3462346e13879dc4 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 08:32:13 -0800 Subject: [PATCH 19/27] Clarify mmdb_error behavior on GAI failure in docs Document that *mmdb_error is set to MMDB_SUCCESS when *gai_error is non-zero, since no database error occurred in that case. Recommend checking *gai_error first. Co-Authored-By: Claude Opus 4.6 --- doc/libmaxminddb.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index 4f64ffcd..5684f8e8 100644 --- a/doc/libmaxminddb.md +++ b/doc/libmaxminddb.md @@ -489,6 +489,10 @@ This function always returns an `MMDB_lookup_result_s` structure, but you should also check the `gai_error` and `mmdb_error` parameters. If either of these indicates an error then the returned structure is meaningless. +When `*gai_error` is non-zero (i.e., `getaddrinfo()` failed), `*mmdb_error` +is set to `MMDB_SUCCESS` because no database error occurred. You should always +check `*gai_error` first. + If no error occurred you still need to make sure that the `found_entry` member in the returned result is true. If it's not, this means that the IP address does not have an entry in the database. From e1635ba7e55e29648010fac55dbd76529825934a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:28:47 -0800 Subject: [PATCH 20/27] Extend bad_databases_t to test data extraction after successful lookup Previously, test_read() only tested MMDB_open and MMDB_lookup_string, asserting that all bad-data files must fail at one of these stages. New bad-data files (oversized array/map, deep nesting) pass both open and lookup but fail during MMDB_get_entry_data_list. The uint64-max-epoch database is fully valid but exercises extreme metadata values. Extend test_read() to try progressively deeper operations: 1. MMDB_open - if error, pass 2. MMDB_lookup_string - if error, pass 3. MMDB_get_entry_data_list - if error, pass 4. If no error at any level, still pass (extreme but valid data) This is backward-compatible: existing bad-data files that fail at open or lookup still behave identically. Co-Authored-By: Claude Opus 4.6 --- t/bad_databases_t.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/t/bad_databases_t.c b/t/bad_databases_t.c index 54003d72..2324f1ec 100644 --- a/t/bad_databases_t.c +++ b/t/bad_databases_t.c @@ -12,7 +12,7 @@ int test_read(const char *path, const struct stat *UNUSED(sbuf), int flags, struct FTW *UNUSED(ftw)) { - // Check if path is a regular file) + // Check if path is a regular file if (flags != FTW_F) { return 0; } @@ -32,13 +32,38 @@ int test_read(const char *path, } int gai_error, mmdb_error; - MMDB_lookup_string(mmdb, "1.1.1.1", &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) { BAIL_OUT("could not parse IP address"); } - cmp_ok( - mmdb_error, "!=", MMDB_SUCCESS, "opening %s returned an error", path); + if (mmdb_error != MMDB_SUCCESS) { + ok(1, "received error on lookup for %s", path); + MMDB_close(mmdb); + free(mmdb); + return 0; + } + + if (result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); + MMDB_free_entry_data_list(entry_data_list); + + if (status != MMDB_SUCCESS) { + ok(1, + "received error from MMDB_get_entry_data_list for %s", + path); + MMDB_close(mmdb); + free(mmdb); + return 0; + } + } + + // Some bad-data files (e.g. uint64-max-epoch) are valid databases with + // extreme metadata values. They don't produce errors in libmaxminddb + // but are useful for testing other reader implementations. + ok(1, "no error reading %s (database may have extreme but valid data)", path); MMDB_close(mmdb); free(mmdb); From a99acaf98ed98f07c49cc0e202f863903dfeac2d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:29:01 -0800 Subject: [PATCH 21/27] Update MaxMind-DB submodule with new bad-data files Point submodule to branch with 4 new test databases in bad-data/libmaxminddb/: - libmaxminddb-oversized-array.mmdb - libmaxminddb-oversized-map.mmdb - libmaxminddb-deep-nesting.mmdb - libmaxminddb-uint64-max-epoch.mmdb This commit should be updated to point to main after the MaxMind-DB PR merges. Co-Authored-By: Claude Opus 4.6 --- t/maxmind-db | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/maxmind-db b/t/maxmind-db index 880f6b4b..6a1d9342 160000 --- a/t/maxmind-db +++ b/t/maxmind-db @@ -1 +1 @@ -Subproject commit 880f6b4b5eb6c12ea9d5c70dd201dec2cb5639a2 +Subproject commit 6a1d9342149edb5a255556820ccfdfc190e3caeb From d4841c5d3d2dabf16862093c672041d0cdf38876 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:47:29 -0800 Subject: [PATCH 22/27] Remove mmdb_test_writer.h; rewrite tests to use submodule bad-data files The C-side MMDB builder functions are now superseded by the Go generators in MaxMind-DB. Remove the header and rewrite bad_data_size_t, bad_epoch_t, and max_depth_t to load from bad-data/libmaxminddb/ via a new bad_database_path() helper. Also remove the unused include from overflow_bounds_t. Co-Authored-By: Claude Opus 4.6 --- t/Makefile.am | 2 +- t/bad_data_size_t.c | 246 ++------------------------------------ t/bad_epoch_t.c | 97 +-------------- t/max_depth_t.c | 128 +++----------------- t/maxminddb_test_helper.c | 23 ++++ t/maxminddb_test_helper.h | 1 + t/mmdb_test_writer.h | 63 ---------- t/overflow_bounds_t.c | 2 - 8 files changed, 52 insertions(+), 510 deletions(-) delete mode 100644 t/mmdb_test_writer.h diff --git a/t/Makefile.am b/t/Makefile.am index 8f98ac49..71ea0aec 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -13,7 +13,7 @@ libmmdbtest_la_SOURCES = maxminddb_test_helper.c maxminddb_test_helper.h EXTRA_DIST = compile_c++_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 mmdb_test_writer.h + libtap/tap.c libtap/tap.h maxmind-db check_PROGRAMS = \ bad_pointers_t bad_databases_t bad_data_size_t bad_epoch_t bad_indent_t \ diff --git a/t/bad_data_size_t.c b/t/bad_data_size_t.c index c06d5356..b5ac9d26 100644 --- a/t/bad_data_size_t.c +++ b/t/bad_data_size_t.c @@ -1,148 +1,15 @@ #include "maxminddb_test_helper.h" -#include "mmdb_test_writer.h" -#include - -/* - * Write a map control byte with a large size using case-31 encoding. - * Type 7 (map) fits in the base types: control byte = (7 << 5) | size_marker. - * For case-31 size encoding: control byte size bits = 31, - * then 3 bytes for (size - 65821). - */ -static int write_large_map(uint8_t *buf, uint32_t size) { - uint32_t adjusted = size - 65821; - buf[0] = (7 << 5) | 31; /* type 7 (map), size = case 31 */ - buf[1] = (adjusted >> 16) & 0xff; - buf[2] = (adjusted >> 8) & 0xff; - buf[3] = adjusted & 0xff; - return 4; -} - -/* - * Write an array control byte with a large size using case-31 encoding. - * Type 11 (array) is extended: control byte = (0 << 5) | size_marker, - * followed by extended type byte 4. - * For case-31 size encoding: control byte size bits = 31, - * then 3 bytes for (size - 65821). - */ -static int write_large_array(uint8_t *buf, uint32_t size) { - uint32_t adjusted = size - 65821; - buf[0] = (0 << 5) | 31; /* extended type, size = case 31 */ - buf[1] = 4; /* extended type: 7 + 4 = 11 (array) */ - buf[2] = (adjusted >> 16) & 0xff; - buf[3] = (adjusted >> 8) & 0xff; - buf[4] = adjusted & 0xff; - return 5; -} - -/* - * Create a crafted MMDB with an array claiming 1,000,000 elements but - * only a few bytes of actual data. - */ -static void create_bad_data_size_db(const char *path) { - uint32_t node_count = 1; - uint32_t record_size = 24; - uint32_t record_value = node_count + 16; - - /* The data section needs enough bytes for the array header + a few - * elements but NOT enough for 1M elements. */ - size_t data_buf_size = 64; - size_t metadata_buf_size = 512; - size_t search_tree_size = 6; - size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size + - METADATA_MARKER_LEN + metadata_buf_size; - - uint8_t *file = calloc(1, total_size); - if (!file) { - BAIL_OUT("calloc failed"); - } - - size_t pos = 0; - - /* Search tree: 1 node, 24-bit records, both pointing to data */ - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - - /* 16-byte null separator */ - memset(file + pos, 0, DATA_SEPARATOR_SIZE); - pos += DATA_SEPARATOR_SIZE; - - /* Data section: array claiming 1,000,000 elements */ - pos += write_large_array(file + pos, 1000000); - - /* Only write a few bytes of actual data (way less than 1M entries) */ - pos += write_string(file + pos, "x", 1); - pos += write_string(file + pos, "y", 1); - - /* Pad to data_buf_size */ - size_t data_end = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size; - if (pos < data_end) { - pos = data_end; - } - - /* Metadata marker */ - memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); - pos += METADATA_MARKER_LEN; - - /* Metadata map */ - pos += write_map(file + pos, 9); - - pos += write_meta_key(file + pos, "binary_format_major_version"); - pos += write_uint16(file + pos, 2); - - pos += write_meta_key(file + pos, "binary_format_minor_version"); - pos += write_uint16(file + pos, 0); - - pos += write_meta_key(file + pos, "build_epoch"); - pos += write_uint64(file + pos, 1000000000ULL); - - pos += write_meta_key(file + pos, "database_type"); - pos += write_string(file + pos, "Test", 4); - - pos += write_meta_key(file + pos, "description"); - pos += write_map(file + pos, 0); - - pos += write_meta_key(file + pos, "ip_version"); - pos += write_uint16(file + pos, 4); - - pos += write_meta_key(file + pos, "languages"); - file[pos++] = (0 << 5) | 0; - file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ - - pos += write_meta_key(file + pos, "node_count"); - pos += write_uint32(file + pos, node_count); - - pos += write_meta_key(file + pos, "record_size"); - pos += write_uint16(file + pos, record_size); - - FILE *f = fopen(path, "wb"); - if (!f) { - free(file); - BAIL_OUT("fopen failed"); - } - if (fwrite(file, 1, pos, f) != pos) { - fclose(f); - free(file); - BAIL_OUT("fwrite failed"); - } - fclose(f); - free(file); -} void test_bad_data_size_rejected(void) { - const char *path = "/tmp/test_bad_data_size.mmdb"; - create_bad_data_size_db(path); + char *db_file = bad_database_path("libmaxminddb-oversized-array.mmdb"); MMDB_s mmdb; - int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-data-size MMDB"); if (status != MMDB_SUCCESS) { diag("MMDB_open failed: %s", MMDB_strerror(status)); - remove(path); + free(db_file); return; } @@ -165,116 +32,19 @@ void test_bad_data_size_rejected(void) { } MMDB_close(&mmdb); - remove(path); -} - -/* - * Create a crafted MMDB with a map claiming 1,000,000 entries but - * only a few bytes of actual data. - */ -static void create_bad_map_size_db(const char *path) { - uint32_t node_count = 1; - uint32_t record_size = 24; - uint32_t record_value = node_count + 16; - - size_t data_buf_size = 64; - size_t metadata_buf_size = 512; - size_t search_tree_size = 6; - size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size + - METADATA_MARKER_LEN + metadata_buf_size; - - uint8_t *file = calloc(1, total_size); - if (!file) { - BAIL_OUT("calloc failed"); - } - - size_t pos = 0; - - /* Search tree: 1 node, 24-bit records, both pointing to data */ - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - - /* 16-byte null separator */ - memset(file + pos, 0, DATA_SEPARATOR_SIZE); - pos += DATA_SEPARATOR_SIZE; - - /* Data section: map claiming 1,000,000 entries */ - pos += write_large_map(file + pos, 1000000); - - /* Only write a couple of key-value pairs (way less than 1M entries) */ - pos += write_string(file + pos, "k", 1); - pos += write_string(file + pos, "v", 1); - - /* Pad to data_buf_size */ - size_t data_end = search_tree_size + DATA_SEPARATOR_SIZE + data_buf_size; - if (pos < data_end) { - pos = data_end; - } - - /* Metadata marker */ - memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); - pos += METADATA_MARKER_LEN; - - /* Metadata map */ - pos += write_map(file + pos, 9); - - pos += write_meta_key(file + pos, "binary_format_major_version"); - pos += write_uint16(file + pos, 2); - - pos += write_meta_key(file + pos, "binary_format_minor_version"); - pos += write_uint16(file + pos, 0); - - pos += write_meta_key(file + pos, "build_epoch"); - pos += write_uint64(file + pos, 1000000000ULL); - - pos += write_meta_key(file + pos, "database_type"); - pos += write_string(file + pos, "Test", 4); - - pos += write_meta_key(file + pos, "description"); - pos += write_map(file + pos, 0); - - pos += write_meta_key(file + pos, "ip_version"); - pos += write_uint16(file + pos, 4); - - pos += write_meta_key(file + pos, "languages"); - file[pos++] = (0 << 5) | 0; - file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ - - pos += write_meta_key(file + pos, "node_count"); - pos += write_uint32(file + pos, node_count); - - pos += write_meta_key(file + pos, "record_size"); - pos += write_uint16(file + pos, record_size); - - FILE *f = fopen(path, "wb"); - if (!f) { - free(file); - BAIL_OUT("fopen failed"); - } - if (fwrite(file, 1, pos, f) != pos) { - fclose(f); - free(file); - BAIL_OUT("fwrite failed"); - } - fclose(f); - free(file); + free(db_file); } void test_bad_map_size_rejected(void) { - const char *path = "/tmp/test_bad_map_size.mmdb"; - create_bad_map_size_db(path); + char *db_file = bad_database_path("libmaxminddb-oversized-map.mmdb"); MMDB_s mmdb; - int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-map-size MMDB"); if (status != MMDB_SUCCESS) { diag("MMDB_open failed: %s", MMDB_strerror(status)); - remove(path); + free(db_file); return; } @@ -297,7 +67,7 @@ void test_bad_map_size_rejected(void) { } MMDB_close(&mmdb); - remove(path); + free(db_file); } int main(void) { diff --git a/t/bad_epoch_t.c b/t/bad_epoch_t.c index fde51860..918adca0 100644 --- a/t/bad_epoch_t.c +++ b/t/bad_epoch_t.c @@ -1,103 +1,16 @@ #include "maxminddb_test_helper.h" -#include "mmdb_test_writer.h" -#include - -static void create_bad_epoch_db(const char *path) { - uint32_t node_count = 1; - uint32_t record_size = 24; - uint32_t record_value = node_count + 16; - - size_t metadata_buf_size = 512; - size_t data_size = 10; - size_t search_tree_size = 6; - size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_size + - METADATA_MARKER_LEN + metadata_buf_size; - - uint8_t *file = calloc(1, total_size); - if (!file) { - BAIL_OUT("calloc failed"); - } - - size_t pos = 0; - - /* Search tree: 1 node, 24-bit records */ - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - - /* 16-byte null separator */ - memset(file + pos, 0, DATA_SEPARATOR_SIZE); - pos += DATA_SEPARATOR_SIZE; - - /* Data section: a simple map with one string entry */ - pos += write_map(file + pos, 1); - pos += write_string(file + pos, "ip", 2); - pos += write_string(file + pos, "test", 4); - - /* Metadata marker */ - memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); - pos += METADATA_MARKER_LEN; - - /* Metadata map */ - pos += write_map(file + pos, 9); - - pos += write_meta_key(file + pos, "binary_format_major_version"); - pos += write_uint16(file + pos, 2); - - pos += write_meta_key(file + pos, "binary_format_minor_version"); - pos += write_uint16(file + pos, 0); - - pos += write_meta_key(file + pos, "build_epoch"); - pos += write_uint64(file + pos, UINT64_MAX); - - pos += write_meta_key(file + pos, "database_type"); - pos += write_string(file + pos, "Test", 4); - - pos += write_meta_key(file + pos, "description"); - pos += write_map(file + pos, 0); - - pos += write_meta_key(file + pos, "ip_version"); - pos += write_uint16(file + pos, 4); - - pos += write_meta_key(file + pos, "languages"); - file[pos++] = (0 << 5) | 0; - file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ - - pos += write_meta_key(file + pos, "node_count"); - pos += write_uint32(file + pos, node_count); - - pos += write_meta_key(file + pos, "record_size"); - pos += write_uint16(file + pos, record_size); - - FILE *f = fopen(path, "wb"); - if (!f) { - free(file); - BAIL_OUT("fopen failed"); - } - if (fwrite(file, 1, pos, f) != pos) { - fclose(f); - free(file); - BAIL_OUT("fwrite failed"); - } - fclose(f); - free(file); -} void test_bad_epoch(void) { - const char *path = "/tmp/test_bad_epoch.mmdb"; - create_bad_epoch_db(path); + char *db_file = bad_database_path("libmaxminddb-uint64-max-epoch.mmdb"); /* Verify we can at least open the DB without crashing */ MMDB_s mmdb; - int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-epoch MMDB"); if (status != MMDB_SUCCESS) { diag("MMDB_open failed: %s", MMDB_strerror(status)); - remove(path); + free(db_file); return; } @@ -121,7 +34,7 @@ void test_bad_epoch(void) { sizeof(cmd), "%s -f %s -i 1.2.3.4 -v > /dev/null 2>&1", binary, - path); + db_file); int ret = system(cmd); /* system() returns the exit status; a signal-killed process gives * a non-zero status. WIFEXITED checks for normal exit. */ @@ -130,7 +43,7 @@ void test_bad_epoch(void) { end_skip; MMDB_close(&mmdb); - remove(path); + free(db_file); } int main(void) { diff --git a/t/max_depth_t.c b/t/max_depth_t.c index c67e55c3..15492bf3 100644 --- a/t/max_depth_t.c +++ b/t/max_depth_t.c @@ -1,110 +1,15 @@ #include "maxminddb_test_helper.h" -#include "mmdb_test_writer.h" -#include - -/* - * Create a crafted MMDB file with deeply nested maps and write it to path. - * The data section contains: {"a": {"a": {"a": ... {"a": "x"} ...}}} - * nested `depth` levels deep. All IP lookups resolve to this data. - */ -static void create_deep_nesting_db(const char *path, int depth) { - uint32_t node_count = 1; - uint32_t record_size = 24; - uint32_t record_value = node_count + 16; - - size_t data_size = (size_t)depth * 3 + 2; - size_t metadata_buf_size = 512; - size_t search_tree_size = 6; - size_t total_size = search_tree_size + DATA_SEPARATOR_SIZE + data_size + - METADATA_MARKER_LEN + metadata_buf_size; - - uint8_t *file = calloc(1, total_size); - if (!file) { - BAIL_OUT("calloc failed"); - } - - size_t pos = 0; - - /* Search tree: 1 node, 24-bit records */ - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - file[pos++] = (record_value >> 16) & 0xff; - file[pos++] = (record_value >> 8) & 0xff; - file[pos++] = record_value & 0xff; - - /* 16-byte null separator */ - memset(file + pos, 0, DATA_SEPARATOR_SIZE); - pos += DATA_SEPARATOR_SIZE; - - /* Data section: deeply nested maps */ - for (int i = 0; i < depth; i++) { - pos += write_map(file + pos, 1); - pos += write_string(file + pos, "a", 1); - } - pos += write_string(file + pos, "x", 1); - - /* Metadata marker */ - memcpy(file + pos, METADATA_MARKER, METADATA_MARKER_LEN); - pos += METADATA_MARKER_LEN; - - /* Metadata map */ - pos += write_map(file + pos, 9); - - pos += write_meta_key(file + pos, "binary_format_major_version"); - pos += write_uint16(file + pos, 2); - - pos += write_meta_key(file + pos, "binary_format_minor_version"); - pos += write_uint16(file + pos, 0); - - pos += write_meta_key(file + pos, "build_epoch"); - pos += write_uint64(file + pos, 1000000000ULL); - - pos += write_meta_key(file + pos, "database_type"); - pos += write_string(file + pos, "Test", 4); - - pos += write_meta_key(file + pos, "description"); - pos += write_map(file + pos, 0); - - pos += write_meta_key(file + pos, "ip_version"); - pos += write_uint16(file + pos, 4); - - pos += write_meta_key(file + pos, "languages"); - file[pos++] = (0 << 5) | 0; /* type 0 (extended), size 0 */ - file[pos++] = 4; /* extended type: 7 + 4 = 11 (array) */ - - pos += write_meta_key(file + pos, "node_count"); - pos += write_uint32(file + pos, node_count); - - pos += write_meta_key(file + pos, "record_size"); - pos += write_uint16(file + pos, record_size); - - FILE *f = fopen(path, "wb"); - if (!f) { - free(file); - BAIL_OUT("fopen failed"); - } - if (fwrite(file, 1, pos, f) != pos) { - fclose(f); - free(file); - BAIL_OUT("fwrite failed"); - } - fclose(f); - free(file); -} void test_deep_nesting_rejected(void) { - const char *path = "/tmp/test_max_depth.mmdb"; - /* 600 levels is above MAXIMUM_DATA_STRUCTURE_DEPTH (512) */ - create_deep_nesting_db(path, 600); + char *db_file = bad_database_path("libmaxminddb-deep-nesting.mmdb"); MMDB_s mmdb; - int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); cmp_ok(status, "==", MMDB_SUCCESS, "opened deeply nested MMDB"); if (status != MMDB_SUCCESS) { diag("MMDB_open failed: %s", MMDB_strerror(status)); - remove(path); + free(db_file); return; } @@ -130,47 +35,42 @@ void test_deep_nesting_rejected(void) { } MMDB_close(&mmdb); - remove(path); + free(db_file); } void test_valid_nesting_allowed(void) { - const char *path = "/tmp/test_valid_depth.mmdb"; - /* 10 levels is well within MAXIMUM_DATA_STRUCTURE_DEPTH (512) */ - create_deep_nesting_db(path, 10); + char *db_file = test_database_path("MaxMind-DB-test-nested.mmdb"); MMDB_s mmdb; - int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); cmp_ok(status, "==", MMDB_SUCCESS, "opened moderately nested MMDB"); if (status != MMDB_SUCCESS) { diag("MMDB_open failed: %s", MMDB_strerror(status)); - remove(path); + free(db_file); return; } int gai_error, mmdb_error; MMDB_lookup_result_s result = - MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + MMDB_lookup_string(&mmdb, "1.1.1.1", &gai_error, &mmdb_error); cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); ok(result.found_entry, "entry found"); if (result.found_entry) { - /* Looking up non-existent key "z" at depth 10 should NOT - * trigger the depth limit — it should return the normal - * "path does not match" error. */ - MMDB_entry_data_s entry_data; - const char *lookup_path[] = {"z", NULL}; - status = MMDB_aget_value(&result.entry, &entry_data, lookup_path); + MMDB_entry_data_list_s *entry_data_list = NULL; + status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); cmp_ok(status, "==", - MMDB_LOOKUP_PATH_DOES_NOT_MATCH_DATA_ERROR, - "MMDB_aget_value returns PATH_DOES_NOT_MATCH for " + MMDB_SUCCESS, + "MMDB_get_entry_data_list succeeds for " "valid nesting depth"); + MMDB_free_entry_data_list(entry_data_list); } MMDB_close(&mmdb); - remove(path); + free(db_file); } int main(void) { diff --git a/t/maxminddb_test_helper.c b/t/maxminddb_test_helper.c index 612c7a65..a049800f 100644 --- a/t/maxminddb_test_helper.c +++ b/t/maxminddb_test_helper.c @@ -70,6 +70,29 @@ char *test_database_path(const char *filename) { return path; } +char *bad_database_path(const char *filename) { + char *bad_db_dir; +#ifdef _WIN32 + bad_db_dir = "../t/maxmind-db/bad-data/libmaxminddb"; +#else + char cwd[500]; + char *UNUSED(tmp) = getcwd(cwd, 500); + + if (strcmp(basename(cwd), "t") == 0) { + bad_db_dir = "./maxmind-db/bad-data/libmaxminddb"; + } else { + bad_db_dir = "./t/maxmind-db/bad-data/libmaxminddb"; + } +#endif + + char *path = malloc(500); + assert(NULL != path); + + snprintf(path, 500, "%s/%s", bad_db_dir, filename); + + return path; +} + char *dup_entry_string_or_bail(MMDB_entry_data_s entry_data) { char *string = mmdb_strndup(entry_data.utf8_string, entry_data.data_size); if (NULL == string) { diff --git a/t/maxminddb_test_helper.h b/t/maxminddb_test_helper.h index 53e14dce..0b66d0b9 100644 --- a/t/maxminddb_test_helper.h +++ b/t/maxminddb_test_helper.h @@ -48,6 +48,7 @@ extern void for_all_record_sizes(const char *filename_fmt, const char *description)); extern void for_all_modes(void (*tests)(int mode, const char *description)); extern char *test_database_path(const char *filename); +extern char *bad_database_path(const char *filename); extern char *dup_entry_string_or_bail(MMDB_entry_data_s entry_data); extern MMDB_s *open_ok(const char *db_file, int mode, const char *mode_desc); extern MMDB_lookup_result_s lookup_string_ok(MMDB_s *mmdb, diff --git a/t/mmdb_test_writer.h b/t/mmdb_test_writer.h deleted file mode 100644 index 14f71d59..00000000 --- a/t/mmdb_test_writer.h +++ /dev/null @@ -1,63 +0,0 @@ -#ifndef MMDB_TEST_WRITER_H -#define MMDB_TEST_WRITER_H - -/* - * Shared helpers for tests that construct MMDB files in memory. - * Each function writes one MMDB data-format element into buf and returns - * the number of bytes written. - */ - -#include -#include - -/* MMDB binary format constants */ -#define METADATA_MARKER "\xab\xcd\xefMaxMind.com" -#define METADATA_MARKER_LEN 14 -#define DATA_SEPARATOR_SIZE 16 - -static inline int write_map(uint8_t *buf, uint32_t size) { - buf[0] = (7 << 5) | (size & 0x1f); - return 1; -} - -static inline int write_string(uint8_t *buf, const char *str, uint32_t len) { - buf[0] = (2 << 5) | (len & 0x1f); - memcpy(buf + 1, str, len); - return 1 + len; -} - -static inline int write_uint16(uint8_t *buf, uint16_t value) { - buf[0] = (5 << 5) | 2; - buf[1] = (value >> 8) & 0xff; - buf[2] = value & 0xff; - return 3; -} - -static inline int write_uint32(uint8_t *buf, uint32_t value) { - buf[0] = (6 << 5) | 4; - buf[1] = (value >> 24) & 0xff; - buf[2] = (value >> 16) & 0xff; - buf[3] = (value >> 8) & 0xff; - buf[4] = value & 0xff; - return 5; -} - -static inline int write_uint64(uint8_t *buf, uint64_t value) { - buf[0] = (0 << 5) | 8; - buf[1] = 2; /* extended type: 7 + 2 = 9 (uint64) */ - buf[2] = (value >> 56) & 0xff; - buf[3] = (value >> 48) & 0xff; - buf[4] = (value >> 40) & 0xff; - buf[5] = (value >> 32) & 0xff; - buf[6] = (value >> 24) & 0xff; - buf[7] = (value >> 16) & 0xff; - buf[8] = (value >> 8) & 0xff; - buf[9] = value & 0xff; - return 10; -} - -static inline int write_meta_key(uint8_t *buf, const char *key) { - return write_string(buf, key, strlen(key)); -} - -#endif /* MMDB_TEST_WRITER_H */ diff --git a/t/overflow_bounds_t.c b/t/overflow_bounds_t.c index 8e229edf..136cd370 100644 --- a/t/overflow_bounds_t.c +++ b/t/overflow_bounds_t.c @@ -1,6 +1,4 @@ #include "maxminddb_test_helper.h" -#include "mmdb_test_writer.h" -#include /* * Test that the bounds check in find_address_in_search_tree uses 64-bit From 51764c89505fff3ddea82e7e295f8d1c4202aaf4 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:48:08 -0800 Subject: [PATCH 23/27] Zero data/metadata section sizes in MMDB_close After unmapping, the data_section and metadata_section pointers were NULLed but their _size fields remained stale. Zero them to make the struct fully inert after close. Co-Authored-By: Claude Opus 4.6 --- Changes.md | 10 ++++++---- src/maxminddb.c | 2 ++ t/double_close_t.c | 4 ++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Changes.md b/Changes.md index 21484fbd..8f55ec5e 100644 --- a/Changes.md +++ b/Changes.md @@ -26,10 +26,12 @@ * Fixed a NULL pointer dereference in `mmdblookup` when displaying metadata for a database with an out-of-range `build_epoch`. The `gmtime()` return value is now checked before passing to `strftime()`. -* `MMDB_close()` now NULLs the `file_content` pointer after unmapping. - Previously, calling `MMDB_close()` twice on the same struct (or calling - it after a failed `MMDB_open()` that succeeded at mapping) would - double-munmap the file content, which is undefined behavior. +* `MMDB_close()` now NULLs the `file_content`, `data_section`, and + `metadata_section` pointers and zeroes `file_size`, `data_section_size`, + and `metadata_section_size` after unmapping. Previously, calling + `MMDB_close()` twice on the same struct (or calling it after a failed + `MMDB_open()` that succeeded at mapping) would double-munmap the file + content, which is undefined behavior. * Fixed a stack buffer overflow in `print_indentation()` when `MMDB_dump_entry_data_list()` was called with a negative `indent` value. The negative integer was cast to `size_t`, producing a massive diff --git a/src/maxminddb.c b/src/maxminddb.c index e0617174..6962bdaf 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1929,7 +1929,9 @@ static void free_mmdb_struct(MMDB_s *const mmdb) { mmdb->file_content = NULL; mmdb->file_size = 0; mmdb->data_section = NULL; + mmdb->data_section_size = 0; mmdb->metadata_section = NULL; + mmdb->metadata_section_size = 0; } if (NULL != mmdb->metadata.database_type) { diff --git a/t/double_close_t.c b/t/double_close_t.c index 52052f2c..942b4abf 100644 --- a/t/double_close_t.c +++ b/t/double_close_t.c @@ -27,6 +27,10 @@ void test_double_close(void) { 0, "description.count is 0 after close"); cmp_ok(mmdb.file_size, "==", 0, "file_size is 0 after close"); + cmp_ok(mmdb.data_section_size, "==", 0, + "data_section_size is 0 after close"); + cmp_ok(mmdb.metadata_section_size, "==", 0, + "metadata_section_size is 0 after close"); /* Second close should be a safe no-op (file_content was NULLed) */ MMDB_close(&mmdb); From 0e470e130436161210a72f01af2b9c3f31098985 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:48:27 -0800 Subject: [PATCH 24/27] Improve gmtime fallback text to "out of range" When gmtime() returns NULL for an extreme build_epoch value, display "out of range" instead of "unknown" since the failure is specifically due to the epoch being outside the representable range. Co-Authored-By: Claude Opus 4.6 --- bin/mmdblookup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/mmdblookup.c b/bin/mmdblookup.c index fbae4206..e14518b4 100644 --- a/bin/mmdblookup.c +++ b/bin/mmdblookup.c @@ -361,7 +361,7 @@ static void dump_meta(MMDB_s *mmdb) { if (tm != NULL) { strftime(date, sizeof(date), "%F %T UTC", tm); } else { - snprintf(date, sizeof(date), "unknown"); + snprintf(date, sizeof(date), "out of range"); } fprintf(stdout, From d3fff392e510b749fc6228cb1a663e3d88737c8d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:52:41 -0800 Subject: [PATCH 25/27] Add test for MMDB_read_node off-by-one bounds check Tests that MMDB_read_node rejects node_number == node_count (the off-by-one fix from >= instead of >) and accepts node_count - 1. Co-Authored-By: Claude Opus 4.6 --- t/CMakeLists.txt | 1 + t/Makefile.am | 1 + t/bad_search_tree_t.c | 49 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 t/bad_search_tree_t.c diff --git a/t/CMakeLists.txt b/t/CMakeLists.txt index 433c6e62..8ea56c44 100644 --- a/t/CMakeLists.txt +++ b/t/CMakeLists.txt @@ -5,6 +5,7 @@ add_library(tap # test programs set(TEST_TARGET_NAMES bad_pointers_t + bad_search_tree_t basic_lookup_t data_entry_list_t data-pool-t diff --git a/t/Makefile.am b/t/Makefile.am index 71ea0aec..8468f868 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -17,6 +17,7 @@ EXTRA_DIST = compile_c++_t.pl external_symbols_t.pl mmdblookup_t.pl \ check_PROGRAMS = \ bad_pointers_t bad_databases_t bad_data_size_t bad_epoch_t bad_indent_t \ + bad_search_tree_t \ basic_lookup_t data_entry_list_t \ data-pool-t data_types_t double_close_t dump_t gai_error_t get_value_t \ get_value_pointer_bug_t \ diff --git a/t/bad_search_tree_t.c b/t/bad_search_tree_t.c new file mode 100644 index 00000000..06e8cb85 --- /dev/null +++ b/t/bad_search_tree_t.c @@ -0,0 +1,49 @@ +#include "maxminddb_test_helper.h" + +/* + * Test the off-by-one fix in MMDB_read_node: node_number >= node_count + * must return MMDB_INVALID_NODE_NUMBER_ERROR. Previously the check used + * >, allowing node_number == node_count to read past the tree. + */ +void test_read_node_bounds(void) { + char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); + MMDB_s *mmdb = open_ok(db_file, MMDB_MODE_MMAP, "mmap mode"); + free(db_file); + + if (!mmdb) { + return; + } + + MMDB_search_node_s node; + + /* node_count - 1 is the last valid node */ + int status = + MMDB_read_node(mmdb, mmdb->metadata.node_count - 1, &node); + cmp_ok(status, + "==", + MMDB_SUCCESS, + "MMDB_read_node succeeds for last valid node"); + + /* node_count itself is out of bounds (the off-by-one fix) */ + status = MMDB_read_node(mmdb, mmdb->metadata.node_count, &node); + cmp_ok(status, + "==", + MMDB_INVALID_NODE_NUMBER_ERROR, + "MMDB_read_node rejects node_number == node_count"); + + /* node_count + 1 is also out of bounds */ + status = MMDB_read_node(mmdb, mmdb->metadata.node_count + 1, &node); + cmp_ok(status, + "==", + MMDB_INVALID_NODE_NUMBER_ERROR, + "MMDB_read_node rejects node_number > node_count"); + + MMDB_close(mmdb); + free(mmdb); +} + +int main(void) { + plan(NO_PLAN); + test_read_node_bounds(); + done_testing(); +} From 88af1d694739e69baaaab2faa1544bc768ea7663 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:53:08 -0800 Subject: [PATCH 26/27] Add comment for array size validation Add a clarifying comment before the array bounds check, matching the existing comment on the map bounds check. Co-Authored-By: Claude Opus 4.6 --- src/maxminddb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/maxminddb.c b/src/maxminddb.c index 6962bdaf..925acde9 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1728,6 +1728,7 @@ static int get_entry_data_list(const MMDB_s *const mmdb, case MMDB_DATA_TYPE_ARRAY: { uint32_t array_size = entry_data_list->entry_data.data_size; uint32_t array_offset = entry_data_list->entry_data.offset_to_next; + /* Each array element needs at least 1 byte. */ if (array_offset >= mmdb->data_section_size || array_size > mmdb->data_section_size - array_offset) { DEBUG_MSG("array size exceeds remaining data section"); From 5b71ac48ce190c20c2506c67a25316b7260885ae Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 23 Feb 2026 11:54:05 -0800 Subject: [PATCH 27/27] Add deep nested array test to max_depth_t Adds test_deep_array_nesting_rejected() which loads a 600-level nested array MMDB and asserts MMDB_INVALID_DATA_ERROR from MMDB_get_entry_data_list, complementing the existing map nesting test. Co-Authored-By: Claude Opus 4.6 --- t/max_depth_t.c | 37 +++++++++++++++++++++++++++++++++++++ t/maxmind-db | 2 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/t/max_depth_t.c b/t/max_depth_t.c index 15492bf3..f703b8d0 100644 --- a/t/max_depth_t.c +++ b/t/max_depth_t.c @@ -73,9 +73,46 @@ void test_valid_nesting_allowed(void) { free(db_file); } +void test_deep_array_nesting_rejected(void) { + char *db_file = + bad_database_path("libmaxminddb-deep-array-nesting.mmdb"); + + MMDB_s mmdb; + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "opened deeply nested array MMDB"); + + if (status != MMDB_SUCCESS) { + diag("MMDB_open failed: %s", MMDB_strerror(status)); + free(db_file); + return; + } + + int gai_error, mmdb_error; + MMDB_lookup_result_s result = + MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); + ok(result.found_entry, "entry found"); + + if (result.found_entry) { + MMDB_entry_data_list_s *entry_data_list = NULL; + 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 " + "for deeply nested arrays exceeding max depth"); + MMDB_free_entry_data_list(entry_data_list); + } + + MMDB_close(&mmdb); + free(db_file); +} + int main(void) { plan(NO_PLAN); test_deep_nesting_rejected(); + test_deep_array_nesting_rejected(); test_valid_nesting_allowed(); done_testing(); } diff --git a/t/maxmind-db b/t/maxmind-db index 6a1d9342..29cab1fe 160000 --- a/t/maxmind-db +++ b/t/maxmind-db @@ -1 +1 @@ -Subproject commit 6a1d9342149edb5a255556820ccfdfc190e3caeb +Subproject commit 29cab1feb43abacb7f33c14db1052f99882fb7c6