CVE-2022-0559: Use After Free (CWE-416)
value: Use After Free in GitHub repository radareorg/radare2 prior to 5.6.2.
CWE description: The product reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 9.8
Published: 2022-02-16T11:15Z
Repository: https://github.com/radareorg/radare2
Language: C
Files changed: libr/io/io_bank.c, libr/util/new_rbtree.c
Commit date: 2022-02-13 22:34:24 +0100
Commit message: 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

Diff: diff --git a/libr/io/io_bank.c b/libr/io/io_bank.c
index 882dfc48d18..91f3bd1764e 100644
--- a/libr/io/io_bank.c
+++ b/libr/io/io_bank.c
@@ -224,16 +224,22 @@ R_API bool r_io_bank_map_add_top(RIO *io, const ut32 bankid, const ut32 mapid) {
 	// guaranteed intersection
 	if (r_io_submap_from (bd) < r_io_submap_from (sm)) {
 		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;
 	}
 	if (entry && r_io_submap_from (((RIOSubMap *)entry->data)) <= r_io_submap_to (sm)) {
diff --git a/libr/util/new_rbtree.c b/libr/util/new_rbtree.c
index a40c44b00d7..2b720a15db0 100644
--- a/libr/util/new_rbtree.c
+++ b/libr/util/new_rbtree.c
@@ -136,13 +136,13 @@ static RRBNode *_rot_twice(RRBNode *root, int dir) {
 
 R_API bool r_crbtree_insert(RRBTree *tree, void *data, RRBComparator cmp, void *user) {
 	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;
 		goto out_exit;
 	}

Vulnerable code: 	while (entry && r_io_submap_to (((RIOSubMap *)entry->data)) <= r_io_submap_to (sm)) {
	if (tree->root == NULL) {
		if (tree->root == NULL) {
Fixed code: 	ut64 smto = r_io_submap_to (sm);
	while (entry && r_io_submap_to (((RIOSubMap *)entry->data)) <= smto) {
		// XXX this is a workaround to avoid an UAF in Reproducer: iobank-crash
		void *smfree = bank->submaps->free;
		bank->submaps->free = NULL;
		bank->submaps->free = smfree;
			entry = NULL;
	if (!tree->root) {
		if (!tree->root) {