From 30853b1ab42e4dece7d450181ab54f1fcb65d0aa Mon Sep 17 00:00:00 2001 From: William Storey Date: Thu, 2 Nov 2023 13:46:27 -0700 Subject: [PATCH 1/3] Check return value of record_info_for_database() Currently we will have an assertion failure if this happens, but we're thinking of removing that. --- src/maxminddb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/maxminddb.c b/src/maxminddb.c index c965c2a3..27c0b6be 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1009,6 +1009,9 @@ static int find_ipv4_start_node(MMDB_s *const mmdb) { } record_info_s record_info = record_info_for_database(mmdb); + if (record_info.right_record_offset == 0) { + return MMDB_UNKNOWN_DATABASE_FORMAT_ERROR; + } const uint8_t *search_tree = mmdb->file_content; uint32_t node_value = 0; From e71488685bea517181249365980512615b2defe1 Mon Sep 17 00:00:00 2001 From: William Storey Date: Thu, 2 Nov 2023 13:48:09 -0700 Subject: [PATCH 2/3] Stop using assert() outside test code This is because in cases where the code is compiled with NDEBUG defined, we could have unsafe behavior. Instead of relying on assert, we check the return value of this function in the callers. --- Changes.md | 1 + src/maxminddb.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Changes.md b/Changes.md index d0cd76ca..ccff203e 100644 --- a/Changes.md +++ b/Changes.md @@ -5,6 +5,7 @@ * The feature test macro `_POSIX_C_SOURCE` is no longer set by `maxminddb.h`. As discussed in GitHub #318, this should be set by applications rather than by libraries. +* `assert()` is no longer used outside test code. ## 1.7.1 - 2022-09-30 diff --git a/src/maxminddb.c b/src/maxminddb.c index 27c0b6be..c4bf1916 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -8,7 +8,6 @@ #include "data-pool.h" #include "maxminddb-compat-util.h" #include "maxminddb.h" -#include #include #include #include @@ -993,10 +992,11 @@ static record_info_s record_info_for_database(const MMDB_s *const mmdb) { record_info.left_record_getter = &get_uint32; record_info.right_record_getter = &get_uint32; record_info.right_record_offset = 4; - } else { - assert(false); } + // Callers must check that right_record_offset is non-zero in case none of + // the above conditions matched. + return record_info; } From b859139628d56e31067406ae1f802c4d7cfd4fc4 Mon Sep 17 00:00:00 2001 From: William Storey Date: Thu, 2 Nov 2023 13:49:48 -0700 Subject: [PATCH 3/3] Rewrite a couple yoda style conditions --- src/maxminddb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/maxminddb.c b/src/maxminddb.c index c4bf1916..4868db40 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -928,7 +928,7 @@ static int find_address_in_search_tree(const MMDB_s *const mmdb, sa_family_t address_family, MMDB_lookup_result_s *result) { record_info_s record_info = record_info_for_database(mmdb); - if (0 == record_info.right_record_offset) { + if (record_info.right_record_offset == 0) { return MMDB_UNKNOWN_DATABASE_FORMAT_ERROR; } @@ -1074,7 +1074,7 @@ int MMDB_read_node(const MMDB_s *const mmdb, uint32_t node_number, MMDB_search_node_s *const node) { record_info_s record_info = record_info_for_database(mmdb); - if (0 == record_info.right_record_offset) { + if (record_info.right_record_offset == 0) { return MMDB_UNKNOWN_DATABASE_FORMAT_ERROR; }