Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
## 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`.
- 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.
- 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

- Fixed validation of empty maps and arrays at the end of the metadata section.
Expand Down
11 changes: 9 additions & 2 deletions doc/libmaxminddb.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -770,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
Expand Down
1 change: 1 addition & 0 deletions include/maxminddb.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 38 additions & 6 deletions src/maxminddb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -990,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;
Expand Down Expand Up @@ -1083,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;
}

Expand Down Expand Up @@ -1126,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.
Expand Down Expand Up @@ -1421,6 +1443,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.
Expand Down Expand Up @@ -2254,6 +2283,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";
}
Expand Down
2 changes: 2 additions & 0 deletions t/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -32,6 +33,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
)
Expand Down
5 changes: 3 additions & 2 deletions t/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ 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 \
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
Expand Down
55 changes: 55 additions & 0 deletions t/bad_search_tree_t.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,63 @@ void test_read_node_bounds(void) {
free(mmdb);
}

// 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(db_file, MMDB_MODE_MMAP, &mmdb);
cmp_ok(status, "==", MMDB_SUCCESS, "%s: opened crafted bad MMDB", label);
if (status != MMDB_SUCCESS) {
free(db_file);
return;
}

MMDB_search_node_s node;
status = MMDB_read_node(&mmdb, 0, &node);
cmp_ok(status,
"==",
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, 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,
"%s: MMDB_lookup_string rejects record",
label);
ok(!result.found_entry, "%s: lookup reports no entry", label);

MMDB_close(&mmdb);
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();
test_separator_record_rejected();
done_testing();
}
51 changes: 51 additions & 0 deletions t/invalid_sockaddr_t.c
Original file line number Diff line number Diff line change
@@ -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();
}
Comment thread
oschwald marked this conversation as resolved.
23 changes: 23 additions & 0 deletions t/metadata_marker_t.c
Original file line number Diff line number Diff line change
@@ -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();
}
Comment thread
oschwald marked this conversation as resolved.
Loading