Fix bin_elf import & symbol leak

rizinorg · May 13, 2024 · 3d0e7df · 3d0e7df
1 parent e922185
commit 3d0e7df
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 17 deletions.
diff --git a/librz/bin/bin.c b/librz/bin/bin.c
@@ -203,16 +203,8 @@ RZ_API void rz_bin_reloc_free(RZ_NULLABLE RzBinReloc *reloc) {
 	if (!reloc) {
 		return;
 	}
-	/**
-	 * TODO: leak in bin_elf, but it will cause double free in bin_pe if free here,
-	 * Because in the bin_elf implementation RzBinObject->imports and RzBinObject->relocs->imports
-	 * are two pieces of data, but they are linked to each other in bin_pe
-	 * TODO: also, double free in bin_coff if free here,
-	 * Because in the bin_coff implementation imports and symbols are shared
-	 * between relocs
-	 */
-	//	rz_bin_import_free(reloc->import);
-	//	rz_bin_symbol_free(reloc->symbol);
+	rz_bin_import_free(reloc->import);
+	rz_bin_symbol_free(reloc->symbol);
 	free(reloc);
 }
 

diff --git a/librz/bin/bobj.c b/librz/bin/bobj.c
@@ -95,7 +95,7 @@ static int reloc_target_cmp(const void *a, const void *b, void *user) {
 
 #undef CMP_CHECK
 
-RZ_API RzBinRelocStorage *rz_bin_reloc_storage_new(RZ_OWN RzPVector /*<RzBinReloc *>*/ *relocs) {
+RZ_API RzBinRelocStorage *rz_bin_reloc_storage_new(RZ_OWN RzPVector /*<RzBinReloc *>*/ *relocs, RzBinPlugin *plugin) {
 	RzBinRelocStorage *ret = RZ_NEW0(RzBinRelocStorage);
 	if (!ret) {
 		return NULL;
@@ -125,6 +125,9 @@ RZ_API RzBinRelocStorage *rz_bin_reloc_storage_new(RZ_OWN RzPVector /*<RzBinRelo
 	ret->target_relocs_count = rz_pvector_len(&target_sorter);
 	ret->target_relocs = (RzBinReloc **)rz_pvector_flush(&target_sorter);
 	rz_pvector_fini(&target_sorter);
+	if (plugin && !strcmp(plugin->name, "coff")) {
+		ret->sym_imp_shared = true;
+	}
 	return ret;
 }
 
@@ -133,7 +136,11 @@ RZ_API void rz_bin_reloc_storage_free(RzBinRelocStorage *storage) {
 		return;
 	}
 	for (size_t i = 0; i < storage->relocs_count; i++) {
-		rz_bin_reloc_free(storage->relocs[i]);
+		if (storage->sym_imp_shared) {
+			free(storage->relocs[i]); // Not freeing symbol and import
+		} else {
+			rz_bin_reloc_free(storage->relocs[i]);
+		}
 	}
 	free(storage->relocs);
 	free(storage->target_relocs);
@@ -554,7 +561,7 @@ RZ_API RzBinRelocStorage *rz_bin_object_patch_relocs(RzBinFile *bf, RzBinObject
 		}
 		rz_bin_reloc_storage_free(o->relocs);
 		REBASE_PADDR(o, tmp, RzBinReloc);
-		o->relocs = rz_bin_reloc_storage_new(tmp);
+		o->relocs = rz_bin_reloc_storage_new(tmp, o->plugin);
 		bf->rbin->is_reloc_patched = true;
 	}
 	return o->relocs;

diff --git a/librz/bin/bobj_process_reloc.c b/librz/bin/bobj_process_reloc.c
@@ -47,7 +47,7 @@ RZ_IPI void rz_bin_set_and_process_relocs(RzBinFile *bf, RzBinObject *o, const R
 		process_handle_reloc(element, o, demangler, flags, imp_cb, sym_cb);
 	}
 
-	o->relocs = rz_bin_reloc_storage_new(relocs);
+	o->relocs = rz_bin_reloc_storage_new(relocs, plugin);
 }
 
 RZ_IPI void rz_bin_demangle_relocs_with_flags(RzBinObject *o, const RzDemanglerPlugin *demangler, RzDemanglerFlag flags) {

diff --git a/librz/bin/p/bin_pe.inc b/librz/bin/p/bin_pe.inc
@@ -431,7 +431,8 @@ static RzPVector /*<RzBinImport *>*/ *imports(RzBinFile *bf) {
 	if (!bf || !bf->o || !bf->o->bin_obj) {
 		return NULL;
 	}
-	RzPVector *ret = rz_pvector_new((RzListFree)rz_bin_import_free);
+	// rz_bin core will free imports
+	RzPVector *ret = rz_pvector_new(NULL);
 	if (!ret) {
 		return NULL;
 	}

diff --git a/librz/include/rz_bin.h b/librz/include/rz_bin.h
@@ -682,9 +682,10 @@ struct rz_bin_reloc_storage_t {
 	size_t relocs_count;
 	RzBinReloc **target_relocs; ///< all relocs that have a valid target_vaddr, ordered by their target_vaddr. size is target_relocs_count!
 	size_t target_relocs_count;
+	bool sym_imp_shared; // plugin frees reloc symbols and imports
 }; // RzBinRelocStorage
 
-RZ_API RzBinRelocStorage *rz_bin_reloc_storage_new(RZ_OWN RzPVector /*<RzBinReloc *>*/ *relocs);
+RZ_API RzBinRelocStorage *rz_bin_reloc_storage_new(RZ_OWN RzPVector /*<RzBinReloc *>*/ *relocs, RzBinPlugin *plugin);
 RZ_API void rz_bin_reloc_storage_free(RzBinRelocStorage *storage);
 RZ_API RzBinReloc *rz_bin_reloc_storage_get_reloc_in(RzBinRelocStorage *storage, ut64 vaddr, ut64 size);
 

diff --git a/test/integration/test_bin.c b/test/integration/test_bin.c
@@ -117,7 +117,7 @@ bool test_rz_bin_reloc_storage(void) {
 	mu_assert_notnull(rz, "reloc");
 	RzBinReloc *r3 = add_reloc(l, 0x1003, 0x1006, 0x200c);
 	mu_assert_notnull(r3, "reloc");
-	RzBinRelocStorage *relocs = rz_bin_reloc_storage_new(l);
+	RzBinRelocStorage *relocs = rz_bin_reloc_storage_new(l, NULL);
 
 	RzBinReloc *r = rz_bin_reloc_storage_get_reloc_in(relocs, 0xfff, 1);
 	mu_assert_null(r, "reloc in");