From 9364349662c0a64f617700f58614327bd9565bc0 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 3 Mar 2026 10:16:48 -0800 Subject: [PATCH 1/3] Fix validation of empty maps/arrays at end of metadata The size validation added in c1d353a to prevent DoS via exaggerated map/array sizes used >= when checking offset against data_section_size. This rejected valid 0-length containers when they were the last field in metadata (offset_to_next == data_section_size). Change to > since the existing second condition (size > remaining) already correctly handles the boundary case for non-empty containers. Reported in vimt/MaxMind-DB-Writer-python#16. ENG-4322 Co-Authored-By: Claude Opus 4.6 --- Changes.md | 6 +++++ src/maxminddb.c | 4 ++-- t/CMakeLists.txt | 1 + t/Makefile.am | 3 ++- t/empty_container_metadata_t.c | 42 ++++++++++++++++++++++++++++++++++ t/maxmind-db | 2 +- 6 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 t/empty_container_metadata_t.c diff --git a/Changes.md b/Changes.md index 29591851..ac58c430 100644 --- a/Changes.md +++ b/Changes.md @@ -1,3 +1,9 @@ +## 1.13.3 - 2026-XX-XX + +- Fixed validation of empty maps and arrays at the end of the metadata section. + `MMDB_open` would incorrectly reject databases where a 0-element map or array + was the last field in metadata. Reported in vimt/MaxMind-DB-Writer-python#16. + ## 1.13.2 - 2026-02-25 - Fixed a compilation failure on macOS 26 (Tahoe) where `sys/endian.h` defines diff --git a/src/maxminddb.c b/src/maxminddb.c index 66b69be2..5aa3b254 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1729,7 +1729,7 @@ static int get_entry_data_list(const MMDB_s *const mmdb, 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 || + 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; @@ -1758,7 +1758,7 @@ static int get_entry_data_list(const MMDB_s *const mmdb, 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 || + 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; diff --git a/t/CMakeLists.txt b/t/CMakeLists.txt index 8ea56c44..5065cca8 100644 --- a/t/CMakeLists.txt +++ b/t/CMakeLists.txt @@ -31,6 +31,7 @@ if(UNIX) # or if (NOT WIN32) bad_data_size_t bad_epoch_t bad_indent_t + empty_container_metadata_t max_depth_t threads_t ) diff --git a/t/Makefile.am b/t/Makefile.am index 8468f868..a85c507d 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -19,7 +19,8 @@ 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 \ + data-pool-t data_types_t double_close_t dump_t empty_container_metadata_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 \ diff --git a/t/empty_container_metadata_t.c b/t/empty_container_metadata_t.c new file mode 100644 index 00000000..6ad3dc76 --- /dev/null +++ b/t/empty_container_metadata_t.c @@ -0,0 +1,42 @@ +#include "maxminddb_test_helper.h" + +static void test_db_opens_and_lookup_succeeds(const char *filename, + const char *open_msg) { + char *db_file = bad_database_path(filename); + + MMDB_s mmdb; + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, open_msg); + + if (status != MMDB_SUCCESS) { + diag("MMDB_open failed: %s", MMDB_strerror(status)); + free(db_file); + return; + } + + int gai_error, mmdb_error; + MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); + + MMDB_close(&mmdb); + free(db_file); +} + +void test_empty_map_last_in_metadata(void) { + test_db_opens_and_lookup_succeeds( + "libmaxminddb-empty-map-last-in-metadata.mmdb", + "opened MMDB with empty map at end of metadata"); +} + +void test_empty_array_last_in_metadata(void) { + test_db_opens_and_lookup_succeeds( + "libmaxminddb-empty-array-last-in-metadata.mmdb", + "opened MMDB with empty array at end of metadata"); +} + +int main(void) { + plan(NO_PLAN); + test_empty_map_last_in_metadata(); + test_empty_array_last_in_metadata(); + done_testing(); +} diff --git a/t/maxmind-db b/t/maxmind-db index 327e23aa..809dd1ca 160000 --- a/t/maxmind-db +++ b/t/maxmind-db @@ -1 +1 @@ -Subproject commit 327e23aaccd3873c7d92fd9a6076694c48d877cd +Subproject commit 809dd1ca37e5c9d3ebc63bf159f81ec332a4f08e From 8882bd45ceb1ea03cf0599a30ae9a48de272d0af Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 5 Mar 2026 07:51:12 -0800 Subject: [PATCH 2/3] Add gai_error assertion in empty container metadata test The test only checked mmdb_error after MMDB_lookup_string, but a GAI failure still sets mmdb_error to MMDB_SUCCESS. Assert gai_error == 0 to ensure the lookup actually reaches the database. Co-Authored-By: Claude Opus 4.6 --- t/empty_container_metadata_t.c | 1 + 1 file changed, 1 insertion(+) diff --git a/t/empty_container_metadata_t.c b/t/empty_container_metadata_t.c index 6ad3dc76..7799ae78 100644 --- a/t/empty_container_metadata_t.c +++ b/t/empty_container_metadata_t.c @@ -16,6 +16,7 @@ static void test_db_opens_and_lookup_succeeds(const char *filename, int gai_error, mmdb_error; MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error); + cmp_ok(gai_error, "==", 0, "getaddrinfo succeeded"); cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded"); MMDB_close(&mmdb); From b58722572d90297beb7c4e041637d05768f7e0ec Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 5 Mar 2026 07:51:17 -0800 Subject: [PATCH 3/3] Use ustar tar format in automake to support long paths The v7 tar format limits paths to 99 characters, which make dist exceeded with the new test database filenames. Switch to tar-ustar (POSIX.1-1988) which supports paths up to 256 characters. Co-Authored-By: Claude Opus 4.6 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index f35c3401..e5d57139 100644 --- a/configure.ac +++ b/configure.ac @@ -11,7 +11,7 @@ m4_ifdef([PKG_INSTALLDIR], [PKG_INSTALLDIR], [AC_SUBST([pkgconfigdir], [${libdir AC_CONFIG_FILES([src/libmaxminddb.pc]) LT_INIT -AM_INIT_AUTOMAKE(foreign m4_esyscmd([case `automake --version | head -n 1` in +AM_INIT_AUTOMAKE(foreign tar-ustar m4_esyscmd([case `automake --version | head -n 1` in *1.14*) echo subdir-objects;; *1.11*);; *) echo serial-tests;;