From 712cd4243a7ea8c0b6a8ef13de254b20f75f7925 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sun, 3 May 2026 18:03:15 -0700 Subject: [PATCH 1/4] Reject unsupported sockaddr families --- Changes.md | 7 ++++++ doc/libmaxminddb.md | 5 ++++- include/maxminddb.h | 1 + src/maxminddb.c | 15 ++++++++++++- t/CMakeLists.txt | 1 + t/Makefile.am | 2 +- t/invalid_sockaddr_t.c | 51 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 t/invalid_sockaddr_t.c diff --git a/Changes.md b/Changes.md index df03c288..117e6ce4 100644 --- a/Changes.md +++ b/Changes.md @@ -1,3 +1,10 @@ +## next release + +- Fixed an out-of-bounds read in `MMDB_lookup_sockaddr()` when callers passed a + `sockaddr` with an unsupported address family. The function now rejects any + family other than `AF_INET` and `AF_INET6` with + `MMDB_INVALID_NETWORK_ADDRESS_ERROR`. + ## 1.13.3 - 2026-03-05 - Fixed validation of empty maps and arrays at the end of the metadata section. diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index ef416715..779f481c 100644 --- a/doc/libmaxminddb.md +++ b/doc/libmaxminddb.md @@ -393,6 +393,8 @@ status codes are: array index larger than an array or smaller than the minimum offset from the end of an array. It can also happen when the path expects to find a map or array where none exist. +- `MMDB_INVALID_NETWORK_ADDRESS_ERROR` - `MMDB_lookup_sockaddr()` was given a + `sockaddr` whose family is neither `AF_INET` nor `AF_INET6`. All status codes should be treated as `int` values. @@ -518,7 +520,8 @@ MMDB_lookup_result_s MMDB_lookup_sockaddr( ``` This function looks up an IP address that has already been resolved by -`getaddrinfo()`. +`getaddrinfo()`. The `sockaddr` passed to this function must be an `AF_INET` or +`AF_INET6` address. Other than not calling `getaddrinfo()` itself, this function is identical to the `MMDB_lookup_string()` function. diff --git a/include/maxminddb.h b/include/maxminddb.h index 9c1a96b9..59f404db 100644 --- a/include/maxminddb.h +++ b/include/maxminddb.h @@ -86,6 +86,7 @@ extern "C" { #define MMDB_LOOKUP_PATH_DOES_NOT_MATCH_DATA_ERROR (9) #define MMDB_INVALID_NODE_NUMBER_ERROR (10) #define MMDB_IPV6_LOOKUP_IN_IPV4_DATABASE_ERROR (11) + #define MMDB_INVALID_NETWORK_ADDRESS_ERROR (12) #if !(MMDB_UINT128_IS_BYTE_ARRAY) #if MMDB_UINT128_USING_MODE diff --git a/src/maxminddb.c b/src/maxminddb.c index 5aa3b254..0293ef96 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -926,23 +926,33 @@ MMDB_lookup_result_s MMDB_lookup_sockaddr(const MMDB_s *const mmdb, uint8_t mapped_address[16]; uint8_t const *address; + // Reject families other than AF_INET/AF_INET6 before casting to + // sockaddr_in/sockaddr_in6, which would otherwise read past the + // truncated struct sockaddr the caller passed in. if (mmdb->metadata.ip_version == 4) { if (sockaddr->sa_family == AF_INET6) { *mmdb_error = MMDB_IPV6_LOOKUP_IN_IPV4_DATABASE_ERROR; return result; } + if (sockaddr->sa_family != AF_INET) { + *mmdb_error = MMDB_INVALID_NETWORK_ADDRESS_ERROR; + return result; + } address = (uint8_t const *)&((struct sockaddr_in const *)sockaddr) ->sin_addr.s_addr; } else { if (sockaddr->sa_family == AF_INET6) { address = (uint8_t const *)&((struct sockaddr_in6 const *)sockaddr) ->sin6_addr.s6_addr; - } else { + } else if (sockaddr->sa_family == AF_INET) { address = mapped_address; memset(mapped_address, 0, 12); memcpy(mapped_address + 12, &((struct sockaddr_in const *)sockaddr)->sin_addr.s_addr, 4); + } else { + *mmdb_error = MMDB_INVALID_NETWORK_ADDRESS_ERROR; + return result; } } @@ -2254,6 +2264,9 @@ const char *MMDB_strerror(int error_code) { case MMDB_IPV6_LOOKUP_IN_IPV4_DATABASE_ERROR: return "You attempted to look up an IPv6 address in an IPv4-only " "database"; + case MMDB_INVALID_NETWORK_ADDRESS_ERROR: + return "The sockaddr family is unsupported; only AF_INET and " + "AF_INET6 are accepted"; default: return "Unknown error code"; } diff --git a/t/CMakeLists.txt b/t/CMakeLists.txt index 5065cca8..56c9ee93 100644 --- a/t/CMakeLists.txt +++ b/t/CMakeLists.txt @@ -32,6 +32,7 @@ if(UNIX) # or if (NOT WIN32) bad_epoch_t bad_indent_t empty_container_metadata_t + invalid_sockaddr_t max_depth_t threads_t ) diff --git a/t/Makefile.am b/t/Makefile.am index a85c507d..0cb3718b 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -21,7 +21,7 @@ check_PROGRAMS = \ basic_lookup_t data_entry_list_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 \ + get_value_pointer_bug_t invalid_sockaddr_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 \ threads_t version_t diff --git a/t/invalid_sockaddr_t.c b/t/invalid_sockaddr_t.c new file mode 100644 index 00000000..32535437 --- /dev/null +++ b/t/invalid_sockaddr_t.c @@ -0,0 +1,51 @@ +#include "maxminddb_test_helper.h" + +static void test_invalid_sockaddr_family(const char *filename, + sa_family_t family, + const char *open_msg, + const char *family_msg) { + char *db_file = test_database_path(filename); + MMDB_s *mmdb = open_ok(db_file, MMDB_MODE_MMAP, open_msg); + free(db_file); + + if (!mmdb) { + return; + } + + struct sockaddr addr = {.sa_family = family}; + int mmdb_error = MMDB_SUCCESS; + MMDB_lookup_result_s result = + MMDB_lookup_sockaddr(mmdb, &addr, &mmdb_error); + + ok(!result.found_entry, "%s: no entry returned", family_msg); + cmp_ok(result.netmask, "==", 0, "%s: netmask left at zero", family_msg); + cmp_ok(mmdb_error, + "==", + MMDB_INVALID_NETWORK_ADDRESS_ERROR, + "%s: MMDB_lookup_sockaddr rejects unsupported family", + family_msg); + + MMDB_close(mmdb); + free(mmdb); +} + +int main(void) { + plan(NO_PLAN); + test_invalid_sockaddr_family("MaxMind-DB-test-ipv4-24.mmdb", + AF_UNIX, + "opened IPv4 test database (AF_UNIX)", + "AF_UNIX against IPv4 db"); + test_invalid_sockaddr_family("MaxMind-DB-test-ipv4-24.mmdb", + AF_UNSPEC, + "opened IPv4 test database (AF_UNSPEC)", + "AF_UNSPEC against IPv4 db"); + test_invalid_sockaddr_family("MaxMind-DB-test-ipv6-24.mmdb", + AF_UNIX, + "opened IPv6 test database (AF_UNIX)", + "AF_UNIX against IPv6 db"); + test_invalid_sockaddr_family("MaxMind-DB-test-ipv6-24.mmdb", + AF_UNSPEC, + "opened IPv6 test database (AF_UNSPEC)", + "AF_UNSPEC against IPv6 db"); + done_testing(); +} From 8d7eb66d46c273c00f3e9297db34f8a7b424737d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sun, 3 May 2026 18:03:50 -0700 Subject: [PATCH 2/4] Reject empty metadata sections --- Changes.md | 4 ++++ src/maxminddb.c | 9 ++++++++- t/CMakeLists.txt | 1 + t/Makefile.am | 3 ++- t/maxmind-db | 2 +- t/metadata_marker_t.c | 23 +++++++++++++++++++++++ 6 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 t/metadata_marker_t.c diff --git a/Changes.md b/Changes.md index 117e6ce4..46314ed1 100644 --- a/Changes.md +++ b/Changes.md @@ -4,6 +4,10 @@ `sockaddr` with an unsupported address family. The function now rejects any family other than `AF_INET` and `AF_INET6` with `MMDB_INVALID_NETWORK_ADDRESS_ERROR`. +- Fixed metadata parsing for files that end immediately after the + `\xAB\xCD\xEFMaxMind.com` marker. Such files are now rejected as invalid + metadata instead of allowing a zero-length metadata section to reach the + decoder. ## 1.13.3 - 2026-03-05 diff --git a/src/maxminddb.c b/src/maxminddb.c index 0293ef96..2cd19abe 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -518,7 +518,7 @@ static const uint8_t *find_metadata(const uint8_t *file_content, } } while (NULL != tmp); - if (search_area == start) { + if (search_area == start || max_size <= 0) { return NULL; } @@ -1431,6 +1431,13 @@ static int decode_one(const MMDB_s *const mmdb, MMDB_entry_data_s *entry_data) { const uint8_t *mem = mmdb->data_section; + if (mmdb->data_section_size == 0) { + // decode_one is also called with a fake mmdb whose data_section + // points at the metadata; either way an empty section is invalid. + DEBUG_MSG("decode_one called with an empty section"); + return MMDB_INVALID_DATA_ERROR; + } + // We subtract rather than add as it possible that offset + 1 // could overflow for a corrupt database while an underflow // from data_section_size - 1 should not be possible. diff --git a/t/CMakeLists.txt b/t/CMakeLists.txt index 56c9ee93..04627b60 100644 --- a/t/CMakeLists.txt +++ b/t/CMakeLists.txt @@ -17,6 +17,7 @@ set(TEST_TARGET_NAMES get_value_t ipv4_start_cache_t ipv6_lookup_in_ipv4_t + metadata_marker_t metadata_pointers_t metadata_t no_map_get_value_t diff --git a/t/Makefile.am b/t/Makefile.am index 0cb3718b..630c664c 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -23,7 +23,8 @@ check_PROGRAMS = \ gai_error_t get_value_t \ get_value_pointer_bug_t invalid_sockaddr_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 \ + metadata_marker_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 diff --git a/t/maxmind-db b/t/maxmind-db index 819f226f..1486d037 160000 --- a/t/maxmind-db +++ b/t/maxmind-db @@ -1 +1 @@ -Subproject commit 819f226fbf8290c2b171ac077e6e050618dd3574 +Subproject commit 1486d037d53617e3f111933753ac52f80e1b4b1c diff --git a/t/metadata_marker_t.c b/t/metadata_marker_t.c new file mode 100644 index 00000000..ee004bb3 --- /dev/null +++ b/t/metadata_marker_t.c @@ -0,0 +1,23 @@ +#include "maxminddb_test_helper.h" + +static void test_trailing_metadata_marker(void) { + char *db_file = bad_database_path("libmaxminddb-metadata-marker-only.mmdb"); + MMDB_s mmdb; + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, + "==", + MMDB_INVALID_METADATA_ERROR, + "MMDB_open rejects a file containing only the metadata marker"); + + if (status == MMDB_SUCCESS) { + MMDB_close(&mmdb); + } + + free(db_file); +} + +int main(void) { + plan(NO_PLAN); + test_trailing_metadata_marker(); + done_testing(); +} From 8e108fb7732b90299e284090b8c0ecbebab8cdfa Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sun, 3 May 2026 18:12:23 -0700 Subject: [PATCH 3/4] Reject search tree records in separator --- Changes.md | 3 ++ src/maxminddb.c | 15 ++++-- t/bad_search_tree_t.c | 123 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 4 deletions(-) diff --git a/Changes.md b/Changes.md index 46314ed1..2ff9e861 100644 --- a/Changes.md +++ b/Changes.md @@ -8,6 +8,9 @@ `\xAB\xCD\xEFMaxMind.com` marker. Such files are now rejected as invalid metadata instead of allowing a zero-length metadata section to reach the decoder. +- Fixed search-tree validation for records that point into the 16-byte separator + before the data section. These records are now rejected as corrupt instead of + being exposed as apparent data entries with underflowed offsets. ## 1.13.3 - 2026-03-05 diff --git a/src/maxminddb.c b/src/maxminddb.c index 2cd19abe..cf06924f 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1000,12 +1000,12 @@ static int find_address_in_search_tree(const MMDB_s *const mmdb, result->netmask = current_bit; - if (value >= (uint64_t)node_count + mmdb->data_section_size) { - // The pointer points off the end of the database. + uint8_t type = record_type(mmdb, value); + if (type == MMDB_RECORD_TYPE_INVALID) { return MMDB_CORRUPT_SEARCH_TREE_ERROR; } - if (value == node_count) { + if (type == MMDB_RECORD_TYPE_EMPTY) { // record is empty result->found_entry = false; return MMDB_SUCCESS; @@ -1093,7 +1093,14 @@ static uint8_t record_type(const MMDB_s *const mmdb, uint64_t record) { return MMDB_RECORD_TYPE_EMPTY; } - if (record - node_count < mmdb->data_section_size) { + uint64_t data_offset = record - node_count; + if (data_offset < MMDB_DATA_SECTION_SEPARATOR) { + DEBUG_MSG("record points into the data section separator"); + return MMDB_RECORD_TYPE_INVALID; + } + + data_offset -= MMDB_DATA_SECTION_SEPARATOR; + if (data_offset < mmdb->data_section_size) { return MMDB_RECORD_TYPE_DATA; } diff --git a/t/bad_search_tree_t.c b/t/bad_search_tree_t.c index 2ec8dd31..a7cb38de 100644 --- a/t/bad_search_tree_t.c +++ b/t/bad_search_tree_t.c @@ -1,5 +1,19 @@ +// Required for mkstemp visibility under -D_POSIX_C_SOURCE=200112L on glibc. +#define _XOPEN_SOURCE 500 + #include "maxminddb_test_helper.h" +#include +#include + +#ifdef _WIN32 + #include + #include + #define unlink _unlink +#else + #include +#endif + /* * 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 @@ -41,8 +55,117 @@ void test_read_node_bounds(void) { free(mmdb); } +static FILE *open_temp_file(char *path, size_t path_size) { +#ifdef _WIN32 + errno_t err = tmpnam_s(path, path_size); + if (err != 0) { + return NULL; + } + return fopen(path, "wb"); +#else + snprintf(path, path_size, "./bad-search-tree-XXXXXX"); + int fd = mkstemp(path); + if (fd < 0) { + return NULL; + } + return fdopen(fd, "wb"); +#endif +} + +static int copy_file(FILE *dest, const char *source_path) { + FILE *source = fopen(source_path, "rb"); + if (!source) { + return -1; + } + + unsigned char buffer[4096]; + size_t bytes_read; + while ((bytes_read = fread(buffer, 1, sizeof(buffer), source)) > 0) { + if (fwrite(buffer, 1, bytes_read, dest) != bytes_read) { + fclose(source); + return -1; + } + } + + int result = ferror(source) ? -1 : 0; + fclose(source); + return result; +} + +void test_separator_record_rejected(void) { + char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); + char temp_path[64]; + FILE *temp = open_temp_file(temp_path, sizeof(temp_path)); + if (!temp) { + free(db_file); + BAIL_OUT("could not create temp file: %s", strerror(errno)); + } + + if (copy_file(temp, db_file) != 0) { + fclose(temp); + unlink(temp_path); + free(db_file); + BAIL_OUT("could not copy test database: %s", strerror(errno)); + } + + // Overwrite node 0's left record with node_count + 1, which points into + // the 16-byte separator between the search tree and data section. + if (fseek(temp, 0, SEEK_SET) != 0) { + fclose(temp); + unlink(temp_path); + free(db_file); + BAIL_OUT("fseek failed: %s", strerror(errno)); + } + unsigned char bad_record[3] = {0x00, 0x00, 0xA4}; + if (fwrite(bad_record, 1, sizeof(bad_record), temp) != sizeof(bad_record)) { + fclose(temp); + unlink(temp_path); + free(db_file); + BAIL_OUT("fwrite failed: %s", strerror(errno)); + } + if (fclose(temp) != 0) { + unlink(temp_path); + free(db_file); + BAIL_OUT("fclose failed: %s", strerror(errno)); + } + + MMDB_s mmdb; + int status = MMDB_open(temp_path, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "opened crafted bad search tree MMDB"); + if (status != MMDB_SUCCESS) { + unlink(temp_path); + free(db_file); + return; + } + + MMDB_search_node_s node; + status = MMDB_read_node(&mmdb, 0, &node); + cmp_ok( + status, "==", MMDB_SUCCESS, "MMDB_read_node succeeds for crafted node"); + cmp_ok(node.left_record_type, + "==", + MMDB_RECORD_TYPE_INVALID, + "MMDB_read_node marks separator record as invalid"); + + int gai_error = 0; + int mmdb_error = 0; + MMDB_lookup_result_s result = + MMDB_lookup_string(&mmdb, "1.1.1.1", &gai_error, &mmdb_error); + cmp_ok(gai_error, "==", 0, "lookup string parse succeeds"); + cmp_ok(mmdb_error, + "==", + MMDB_CORRUPT_SEARCH_TREE_ERROR, + "MMDB_lookup_string rejects records pointing into the separator"); + ok(!result.found_entry, "lookup does not report an entry for corrupt tree"); + + MMDB_close(&mmdb); + unlink(temp_path); + free(db_file); +} + int main(void) { plan(NO_PLAN); test_read_node_bounds(); + test_separator_record_rejected(); done_testing(); } From 5f40c2776d35391f88f7fd8e345d8fedb28d6f77 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sun, 3 May 2026 18:18:35 -0700 Subject: [PATCH 4/4] Return an error for invalid search nodes --- Changes.md | 3 + doc/libmaxminddb.md | 6 +- src/maxminddb.c | 5 ++ t/bad_search_tree_t.c | 134 +++++++++++------------------------------- 4 files changed, 46 insertions(+), 102 deletions(-) diff --git a/Changes.md b/Changes.md index 2ff9e861..c4b2af66 100644 --- a/Changes.md +++ b/Changes.md @@ -11,6 +11,9 @@ - Fixed search-tree validation for records that point into the 16-byte separator before the data section. These records are now rejected as corrupt instead of being exposed as apparent data entries with underflowed offsets. +- `MMDB_read_node()` now returns `MMDB_CORRUPT_SEARCH_TREE_ERROR` instead of + `MMDB_SUCCESS` with `MMDB_RECORD_TYPE_INVALID` record types when a node's + child record is invalid. ## 1.13.3 - 2026-03-05 diff --git a/doc/libmaxminddb.md b/doc/libmaxminddb.md index 779f481c..10ecc743 100644 --- a/doc/libmaxminddb.md +++ b/doc/libmaxminddb.md @@ -773,7 +773,11 @@ to an `MMDB_search_node_s` structure that will be populated by this function. The return value is a status code. If you pass a `node_number` that is greater than or equal to the number of nodes in the database, this function will return -`MMDB_INVALID_NODE_NUMBER_ERROR`, otherwise it will return `MMDB_SUCCESS`. +`MMDB_INVALID_NODE_NUMBER_ERROR`. If the node's child records are invalid or +point outside the search tree or data section, it will return +`MMDB_CORRUPT_SEARCH_TREE_ERROR`. Otherwise it will return `MMDB_SUCCESS`. On +any non-`MMDB_SUCCESS` return the contents of `*node` are unspecified and must +not be read. The first node in the search tree is always node 0. If you wanted to iterate over the whole search tree, you would start by reading node 0 and then following diff --git a/src/maxminddb.c b/src/maxminddb.c index cf06924f..8d82a9e7 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1143,6 +1143,11 @@ int MMDB_read_node(const MMDB_s *const mmdb, node->left_record_type = record_type(mmdb, node->left_record); node->right_record_type = record_type(mmdb, node->right_record); + if (node->left_record_type == MMDB_RECORD_TYPE_INVALID || + node->right_record_type == MMDB_RECORD_TYPE_INVALID) { + return MMDB_CORRUPT_SEARCH_TREE_ERROR; + } + // Note that offset will be invalid if the record type is not // MMDB_RECORD_TYPE_DATA, but that's ok. Any use of the record entry // for other data types is a programming error. diff --git a/t/bad_search_tree_t.c b/t/bad_search_tree_t.c index a7cb38de..4eb0cf39 100644 --- a/t/bad_search_tree_t.c +++ b/t/bad_search_tree_t.c @@ -1,19 +1,5 @@ -// Required for mkstemp visibility under -D_POSIX_C_SOURCE=200112L on glibc. -#define _XOPEN_SOURCE 500 - #include "maxminddb_test_helper.h" -#include -#include - -#ifdef _WIN32 - #include - #include - #define unlink _unlink -#else - #include -#endif - /* * 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 @@ -55,114 +41,60 @@ void test_read_node_bounds(void) { free(mmdb); } -static FILE *open_temp_file(char *path, size_t path_size) { -#ifdef _WIN32 - errno_t err = tmpnam_s(path, path_size); - if (err != 0) { - return NULL; - } - return fopen(path, "wb"); -#else - snprintf(path, path_size, "./bad-search-tree-XXXXXX"); - int fd = mkstemp(path); - if (fd < 0) { - return NULL; - } - return fdopen(fd, "wb"); -#endif -} - -static int copy_file(FILE *dest, const char *source_path) { - FILE *source = fopen(source_path, "rb"); - if (!source) { - return -1; - } - - unsigned char buffer[4096]; - size_t bytes_read; - while ((bytes_read = fread(buffer, 1, sizeof(buffer), source)) > 0) { - if (fwrite(buffer, 1, bytes_read, dest) != bytes_read) { - fclose(source); - return -1; - } - } - - int result = ferror(source) ? -1 : 0; - fclose(source); - return result; -} - -void test_separator_record_rejected(void) { - char *db_file = test_database_path("MaxMind-DB-test-ipv4-24.mmdb"); - char temp_path[64]; - FILE *temp = open_temp_file(temp_path, sizeof(temp_path)); - if (!temp) { - free(db_file); - BAIL_OUT("could not create temp file: %s", strerror(errno)); - } - - if (copy_file(temp, db_file) != 0) { - fclose(temp); - unlink(temp_path); - free(db_file); - BAIL_OUT("could not copy test database: %s", strerror(errno)); - } - - // Overwrite node 0's left record with node_count + 1, which points into - // the 16-byte separator between the search tree and data section. - if (fseek(temp, 0, SEEK_SET) != 0) { - fclose(temp); - unlink(temp_path); - free(db_file); - BAIL_OUT("fseek failed: %s", strerror(errno)); - } - unsigned char bad_record[3] = {0x00, 0x00, 0xA4}; - if (fwrite(bad_record, 1, sizeof(bad_record), temp) != sizeof(bad_record)) { - fclose(temp); - unlink(temp_path); - free(db_file); - BAIL_OUT("fwrite failed: %s", strerror(errno)); - } - if (fclose(temp) != 0) { - unlink(temp_path); - free(db_file); - BAIL_OUT("fclose failed: %s", strerror(errno)); - } - +// Open a pre-built corruption fixture and assert that all rejection paths +// fire. `lookup_ip` must traverse the corrupted record from node 0 — high +// bit 0 for a corrupted left record, 1 for a corrupted right record. +static void check_corrupt_record(const char *label, + const char *fixture, + const char *lookup_ip) { + char *db_file = bad_database_path(fixture); MMDB_s mmdb; - int status = MMDB_open(temp_path, MMDB_MODE_MMAP, &mmdb); - cmp_ok(status, "==", MMDB_SUCCESS, "opened crafted bad search tree MMDB"); + int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb); + cmp_ok(status, "==", MMDB_SUCCESS, "%s: opened crafted bad MMDB", label); if (status != MMDB_SUCCESS) { - unlink(temp_path); free(db_file); return; } MMDB_search_node_s node; status = MMDB_read_node(&mmdb, 0, &node); - cmp_ok( - status, "==", MMDB_SUCCESS, "MMDB_read_node succeeds for crafted node"); - cmp_ok(node.left_record_type, + cmp_ok(status, "==", - MMDB_RECORD_TYPE_INVALID, - "MMDB_read_node marks separator record as invalid"); + MMDB_CORRUPT_SEARCH_TREE_ERROR, + "%s: MMDB_read_node rejects record as corrupt", + label); int gai_error = 0; int mmdb_error = 0; MMDB_lookup_result_s result = - MMDB_lookup_string(&mmdb, "1.1.1.1", &gai_error, &mmdb_error); - cmp_ok(gai_error, "==", 0, "lookup string parse succeeds"); + MMDB_lookup_string(&mmdb, lookup_ip, &gai_error, &mmdb_error); + cmp_ok(gai_error, "==", 0, "%s: lookup string parse succeeds", label); cmp_ok(mmdb_error, "==", MMDB_CORRUPT_SEARCH_TREE_ERROR, - "MMDB_lookup_string rejects records pointing into the separator"); - ok(!result.found_entry, "lookup does not report an entry for corrupt tree"); + "%s: MMDB_lookup_string rejects record", + label); + ok(!result.found_entry, "%s: lookup reports no entry", label); MMDB_close(&mmdb); - unlink(temp_path); free(db_file); } +void test_separator_record_rejected(void) { + // Records in the half-open range [node_count + 1, node_count + 16) point + // into the 16-byte separator between the search tree and data section + // and must be rejected as corrupt. + check_corrupt_record("left record, first separator byte", + "libmaxminddb-separator-record-min-left.mmdb", + "1.1.1.1"); + check_corrupt_record("right record, first separator byte", + "libmaxminddb-separator-record-min-right.mmdb", + "128.0.0.0"); + check_corrupt_record("left record, last separator byte", + "libmaxminddb-separator-record-max-left.mmdb", + "1.1.1.1"); +} + int main(void) { plan(NO_PLAN); test_read_node_bounds();