From bc0cbd56258cf942d41b0ed011a216da6bd0b0ae Mon Sep 17 00:00:00 2001 From: William Storey Date: Fri, 10 Jan 2025 16:37:01 +0000 Subject: [PATCH] Set entry parameter to NULL or valid memory To avoid segfaults in callers that freed the parameter on error. --- Changes.md | 12 ++++++++++++ src/maxminddb.c | 2 ++ t/bad_pointers_t.c | 5 +++++ 3 files changed, 19 insertions(+) diff --git a/Changes.md b/Changes.md index ace517bd..0440f87c 100644 --- a/Changes.md +++ b/Changes.md @@ -1,3 +1,15 @@ +## 1.12.2 + +* `MMDB_get_entry_data_list()` now always sets the passed `entry_data_list` + parameter to either `NULL` or valid memory. This makes it safe for + callers to use `MMDB_free_entry_data_list()` on it even in case of error. + In 1.12.0 `MMDB_get_entry_data_list()` was changed to not set this + parameter to valid memory in additional error cases. That change caused + segfaults for certain libraries that assumed it was safe to free memory + on error. Doing so was never safe, but worked in some cases. This change + makes such calls safe. Reported by Petr Pisar. GitHub + maxmind/MaxMind-DB-Reader-XS#39. + ## 1.12.1 - 2025-01-08 * Added missing `cmake_uninstall.cmake.in` to the source distribution. This diff --git a/src/maxminddb.c b/src/maxminddb.c index d458f727..2ea2455e 100644 --- a/src/maxminddb.c +++ b/src/maxminddb.c @@ -1636,6 +1636,8 @@ int MMDB_get_metadata_as_entry_data_list( int MMDB_get_entry_data_list(MMDB_entry_s *start, MMDB_entry_data_list_s **const entry_data_list) { + *entry_data_list = NULL; + MMDB_data_pool_s *const pool = data_pool_new(MMDB_POOL_INIT_SIZE); if (!pool) { return MMDB_OUT_OF_MEMORY_ERROR; diff --git a/t/bad_pointers_t.c b/t/bad_pointers_t.c index 9bf31fb1..6572ae94 100644 --- a/t/bad_pointers_t.c +++ b/t/bad_pointers_t.c @@ -28,6 +28,11 @@ void run_tests(int mode, const char *mode_desc) { MMDB_INVALID_DATA_ERROR, "MMDB_get_entry_data_list returns MMDB_INVALID_DATA_ERROR for " "bad pointer in data section"); + + // This is not necessary as on error we should not need to free + // anything. However test that it is safe to do so. See change in + // 1.12.2. + MMDB_free_entry_data_list(entry_data_list); } {