map: fix incorrect clear when reinserting an integer after removing bytes
What the developer wrote
Authored by Jon Griffiths
map: fix incorrect clear when reinserting an integer after removing bytes
Reported-by: Jordan Mecom (@jmecom) via https://github.com/jmecom/odo
What changed, and why it matters
This commit fixes a bug in a key-value map used by the libwally-core crypto library. When a caller removed a byte-keyed entry and then reinserted a new integer-keyed entry into the same slot, the code could mistakenly free or overwrite memory that was no longer valid. The fix ensures integer keys are explicitly set to 'no bytes' and null values are handled cleanly, preventing memory corruption.
Review callers of wally_map_add_integer and wally_map_add that remove and reinsert entries to confirm no other stale-field assumptions exist; run the new regression test under ASan/Valgrind; consider auditing map_remove and map_find for similar uninitialized/stale-field issues.
Security signals we found
memory corruption risk: stale pointer dereference/clear_and_free_bytes on reused map slot
potential double-free/use-after-free when reusing a removed item's key pointer
regression test added for remove-then-reinsert-integer scenario
Reported-by line credits external reporter Jordan Mecom
Evidence from the diff
In src/map.c’s map_add(), when appending a new item at map_in->items + map_in->num_items, the previous code checked if new_item->key was non-null before clearing it for an integer key. Because the slot may have been previously occupied by a removed byte-keyed entry, new_item->key could be a stale pointer. clear_and_free_bytes() on that stale pointer could lead to a double-free or use-after-free. The patch unconditionally sets new_item->key = NULL for integer keys and similarly sets new_item->value = NULL for null values, avoiding reliance on stale state. A Python regression test exercises the exact scenario: add two byte-keyed entries, remove the first, then add an integer-keyed entry.
Changed components
src/map.c: map_add()wally_map integer-keyed and byte-keyed entry insertionPython test suite: src/test/test_map.py MapTestsInspect captured patch +13 / −4
diff --git a/src/map.c b/src/map.c
index 332cf0a..084d845 100644
--- a/src/map.c
+++ b/src/map.c
@@ -252,14 +252,14 @@ int map_add(struct wally_map *map_in,
struct wally_map_item *new_item = map_in->items + map_in->num_items;
if (!key) {
- /* Integer key */
- if (new_item->key)
- clear_and_free_bytes(&new_item->key, &new_item->key_len);
+ new_item->key = NULL; /* Integer key */
} else if (!clone_bytes(&new_item->key, key, key_len))
return WALLY_ENOMEM; /* Failed to allocate byte key */
new_item->key_len = key_len;
- if (val) {
+ if (!val) {
+ new_item->value = NULL;
+ } else {
if (take_value)
new_item->value = (unsigned char *)val;
else if (!clone_bytes(&new_item->value, val, val_len)) {
diff --git a/src/test/test_map.py b/src/test/test_map.py
index 4fc45e0..b628991 100644
--- a/src/test/test_map.py
+++ b/src/test/test_map.py
@@ -194,6 +194,15 @@ class MapTests(unittest.TestCase):
self.assertEqual(wally_map_free(m), WALLY_OK)
+ # Test re-adding an integer after removing bytes
+ m = pointer(wally_map())
+ self.assertEqual(wally_map_init_alloc(0, None, m), WALLY_OK)
+ self.assertEqual(wally_map_add(*[m, *cases[0]]), WALLY_OK)
+ self.assertEqual(wally_map_add(*[m, *cases[1]]), WALLY_OK)
+ self.assertEqual(wally_map_remove(m, cases[0][0], cases[0][1]), WALLY_OK)
+ self.assertEqual(wally_map_add_integer(m, 1, cases[0][0], cases[0][1]), WALLY_OK)
+ self.assertEqual(wally_map_free(m), WALLY_OK)
+
def test_keypath_map(self):
"""Test keypath map functions"""
#
Why this scored 61/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.