From b5cb90b28ec71fda3504da04e3cc94a362807f5e Mon Sep 17 00:00:00 2001 From: pancake Date: Sun, 13 Feb 2022 21:27:58 +0100 Subject: [PATCH] Prefer memleak over usaf in io.bank's rbtree bug ##crash * That's a workaround, proper fix will come later * Reproducer: bins/fuzzed/iobank-crash * Reported by Akyne Choi via huntr.dev --- libr/io/io_bank.c | 8 +++++++- libr/util/new_rbtree.c | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/libr/io/io_bank.c b/libr/io/io_bank.c index 882dfc48d18f0..91f3bd1764eea 100644 --- a/libr/io/io_bank.c +++ b/libr/io/io_bank.c @@ -226,12 +226,18 @@ R_API bool r_io_bank_map_add_top(RIO *io, const ut32 bankid, const ut32 mapid) { r_io_submap_set_to (bd, r_io_submap_from (sm) - 1); entry = r_rbnode_next (entry); } - while (entry && r_io_submap_to (((RIOSubMap *)entry->data)) <= r_io_submap_to (sm)) { + ut64 smto = r_io_submap_to (sm); + while (entry && r_io_submap_to (((RIOSubMap *)entry->data)) <= smto) { //delete all submaps that are completly included in sm RRBNode *next = r_rbnode_next (entry); // this can be optimized, there is no need to do search here + // XXX this is a workaround to avoid an UAF in Reproducer: iobank-crash + void *smfree = bank->submaps->free; + bank->submaps->free = NULL; bool a = r_crbtree_delete (bank->submaps, entry->data, _find_sm_by_from_vaddr_cb, NULL); + bank->submaps->free = smfree; if (!a) { + entry = NULL; break; } entry = next; diff --git a/libr/util/new_rbtree.c b/libr/util/new_rbtree.c index a40c44b00d745..2b720a15db08a 100644 --- a/libr/util/new_rbtree.c +++ b/libr/util/new_rbtree.c @@ -138,9 +138,9 @@ R_API bool r_crbtree_insert(RRBTree *tree, void *data, RRBComparator cmp, void * r_return_val_if_fail (tree && data && cmp, false); bool inserted = false; - if (tree->root == NULL) { + if (!tree->root) { tree->root = _node_new (data, NULL); - if (tree->root == NULL) { + if (!tree->root) { return false; } inserted = true;