Fix stale pointer issue in RBinELF. Add r_mem_dup

libr/bin/format/elf/elf.c

@@ -1158,13 +1158,22 @@ struct r_bin_elf_field_t* Elf_(r_bin_elf_get_fields)(struct Elf_(r_bin_elf_obj_t
 }
 
 void* Elf_(r_bin_elf_free)(struct Elf_(r_bin_elf_obj_t)* bin) {
+	int i;
 	if (!bin) return NULL;
 	free (bin->phdr);
 	free (bin->shdr);
 	free (bin->strtab);
 	//free (bin->strtab_section);
-	free (bin->imports_by_ord);
-	free (bin->symbols_by_ord);
+	if (bin->imports_by_ord) {
+		for (i=0; i<bin->imports_by_ord_size; i++)
+			free (bin->imports_by_ord[i]);
+		free (bin->imports_by_ord);
+	}
+	if (bin->symbols_by_ord) {
+		for (i=0; i<bin->symbols_by_ord_size; i++)
+			free (bin->symbols_by_ord[i]);
+		free (bin->symbols_by_ord);
+	}
 	r_buf_free (bin->b);
 	free (bin);
 	return NULL;

libr/bin/format/elf/elf.h

@@ -1,5 +1,3 @@
-/* radare - LGPL - Copyright 2008 nibble<.ds@gmail.com> */
-
 #include <r_types.h>
 #include <r_util.h>
 #include <r_lib.h>
@@ -30,7 +28,7 @@ typedef struct r_bin_elf_section_t {
 typedef struct r_bin_elf_symbol_t {
 	ut64 offset;
 	ut64 size;
-	int ordinal;
+	ut32 ordinal;
 	char bind[ELF_STRING_LENGTH];
 	char type[ELF_STRING_LENGTH];
 	char name[ELF_STRING_LENGTH];

libr/bin/p/bin_elf.c

@@ -7,9 +7,25 @@
 #include <r_bin.h>
 #include "elf/elf.h"
 
+#define ELFOBJ struct Elf_(r_bin_elf_obj_t)
 static int check(RBinFile *arch);
 static int check_bytes(const ut8 *buf, ut64 length);
 
+//TODO: implement r_bin_symbol_dup() and r_bin_symbol_free ?
+static void setsymord (ELFOBJ* eobj, ut32 ord, RBinSymbol *ptr) {
+	if (! eobj->symbols_by_ord || ord >= eobj->symbols_by_ord_size)
+		return;
+	free (eobj->symbols_by_ord[ord]);
+	eobj->symbols_by_ord[ord] = r_mem_dup (ptr, sizeof (RBinSymbol));
+}
+
+static void setimpord (ELFOBJ* eobj, ut32 ord, RBinImport *ptr) {
+	if (!eobj->imports_by_ord && ord >= eobj->imports_by_ord_size)
+		return;
+	free (eobj->imports_by_ord[ord]);
+	eobj->imports_by_ord[ord] = r_mem_dup (ptr, sizeof (RBinImport));
+}
+
 static Sdb* get_sdb (RBinObject *o) {
 	if (!o) return NULL;
 	struct Elf_(r_bin_elf_obj_t) *bin = (struct Elf_(r_bin_elf_obj_t) *) o->bin_obj;
@@ -192,14 +208,15 @@ static RList* sections(RBinFile *arch) {
 }
 
 static RList* symbols(RBinFile *arch) {
-	RList *ret = NULL;
-	RBinSymbol *ptr = NULL;
-	ut64 base = 0;
+	int i, has_va = Elf_(r_bin_elf_has_va) (arch->o->bin_obj);
+	struct Elf_(r_bin_elf_obj_t) *bin;
 	struct r_bin_elf_symbol_t *symbol = NULL;
-	struct Elf_(r_bin_elf_obj_t) *bin = arch->o->bin_obj;
-	int i;
-
-	int has_va = Elf_(r_bin_elf_has_va) (arch->o->bin_obj);
+	RBinSymbol *ptr = NULL;
+	RList *ret = NULL;
+	ut64 base = 0;
+	if (!arch || !arch->o || !arch->o->bin_obj)
+		return NULL;
+	bin = arch->o->bin_obj;
 	if (!has_va) {
 		// find base address for non-linked object (.o) //
 		if (arch->o->sections) {
@@ -231,8 +248,7 @@ static RList* symbols(RBinFile *arch) {
 		ptr->paddr = symbol[i].offset + base;
 		ptr->size = symbol[i].size;
 		ptr->ordinal = symbol[i].ordinal;
-		if(bin->symbols_by_ord && ptr->ordinal < bin->symbols_by_ord_size)
-			bin->symbols_by_ord[ptr->ordinal] = ptr;
+		setsymord (bin, ptr->ordinal, ptr);
 		r_list_append (ret, ptr);
 	}
 	free (symbol);
@@ -253,8 +269,7 @@ static RList* symbols(RBinFile *arch) {
 		ptr->paddr = symbol[i].offset;
 		ptr->size = symbol[i].size;
 		ptr->ordinal = symbol[i].ordinal;
-		if(bin->symbols_by_ord && ptr->ordinal < bin->symbols_by_ord_size)
-			bin->symbols_by_ord[ptr->ordinal] = ptr;
+		setsymord (bin, ptr->ordinal, ptr);
 		r_list_append (ret, ptr);
 	}
 	free (symbol);
@@ -263,10 +278,10 @@ static RList* symbols(RBinFile *arch) {
 }
 
 static RList* imports(RBinFile *arch) {
-	RList *ret = NULL;
-	RBinImport *ptr = NULL;
-	struct r_bin_elf_symbol_t *import = NULL;
 	struct Elf_(r_bin_elf_obj_t) *bin = arch->o->bin_obj;
+	struct r_bin_elf_symbol_t *import = NULL;
+	RBinImport *ptr = NULL;
+	RList *ret = NULL;
 	int i;
 
 	if (!(ret = r_list_new ()))
@@ -277,12 +292,11 @@ static RList* imports(RBinFile *arch) {
 	for (i = 0; !import[i].last; i++) {
 		if (!(ptr = R_NEW0 (RBinImport)))
 			break;
-		strncpy (ptr->name, import[i].name, R_BIN_SIZEOF_STRINGS);
-		strncpy (ptr->bind, import[i].bind, R_BIN_SIZEOF_STRINGS);
-		strncpy (ptr->type, import[i].type, R_BIN_SIZEOF_STRINGS);
+		strncpy (ptr->name, import[i].name, sizeof(ptr->name)-1);
+		strncpy (ptr->bind, import[i].bind, sizeof(ptr->bind)-1);
+		strncpy (ptr->type, import[i].type, sizeof(ptr->type)-1);
 		ptr->ordinal = import[i].ordinal;
-		if(bin->imports_by_ord && ptr->ordinal < bin->imports_by_ord_size)
-			bin->imports_by_ord[ptr->ordinal] = ptr;
+		setimpord (bin, ptr->ordinal, ptr);
 		r_list_append (ret, ptr);
 	}
 	free (import);
@@ -290,9 +304,9 @@ static RList* imports(RBinFile *arch) {
 }
 
 static RList* libs(RBinFile *arch) {
+	struct r_bin_elf_lib_t *libs = NULL;
 	RList *ret = NULL;
 	char *ptr = NULL;
-	struct r_bin_elf_lib_t *libs = NULL;
 	int i;
 
 	if (!(ret = r_list_new ()))
@@ -310,9 +324,12 @@ static RList* libs(RBinFile *arch) {
 
 static RBinReloc *reloc_convert(struct Elf_(r_bin_elf_obj_t) *bin, RBinElfReloc *rel, ut64 GOT) {
 	RBinReloc *r = NULL;
-	ut64 B = bin->baddr, P = B + rel->rva;
+	ut64 B, P;
 	char *str;
 
+	if (!bin || !rel) return NULL;
+	B = bin->baddr;
+	P = B + rel->rva;
 	if (!(r = R_NEW0 (RBinReloc)))
 		return r;
 
@@ -401,6 +418,10 @@ static RList* relocs(RBinFile *arch) {
 	if (!(ret = r_list_new ()))
 		return NULL;
 	ret->free = free;
+	/* FIXME: This is a _temporary_ fix/workaround to prevent a use-after-
+	 * free detected by ASan that would corrupt the relocation names */
+	r_list_free (imports (arch));
+
 #if 1
 	if ((got_addr = Elf_ (r_bin_elf_get_section_addr) (arch->o->bin_obj, ".got")) == -1 &&
 		(got_addr = Elf_ (r_bin_elf_get_section_addr) (arch->o->bin_obj, ".got.plt")) == -1) 

libr/include/r_bin.h

@@ -277,8 +277,8 @@ typedef struct r_bin_symbol_t {
 	char descriptor[R_BIN_SIZEOF_STRINGS+1];
 	ut64 vaddr;
 	ut64 paddr;
-	ut64 size;
-	ut64 ordinal;
+	ut32 size;
+	ut32 ordinal;
 	ut32 visibility;
 } RBinSymbol;
 
@@ -288,7 +288,7 @@ typedef struct r_bin_import_t {
 	char type[R_BIN_SIZEOF_STRINGS+1];
 	char classname[R_BIN_SIZEOF_STRINGS+1];
 	char descriptor[R_BIN_SIZEOF_STRINGS+1];
-	ut64 ordinal;
+	ut32 ordinal;
 	ut32 visibility;
 } RBinImport;
 
@@ -307,9 +307,9 @@ typedef struct r_bin_string_t {
 	char string[R_BIN_SIZEOF_STRINGS+1];
 	ut64 vaddr;
 	ut64 paddr;
-	ut64 ordinal;
-	int size; // size of buffer containing the string in bytes
-	int length; // length of string in chars
+	ut32 ordinal;
+	ut32 size; // size of buffer containing the string in bytes
+	ut32 length; // length of string in chars
 	char type; // Ascii Wide cp850 utf8 ...
 } RBinString;
 

libr/include/r_util.h

@@ -313,6 +313,7 @@ R_API void r_cache_flush (RCache *c);
 R_API void r_prof_start(RProfile *p);
 R_API double r_prof_end(RProfile *p);
 
+R_API void *r_mem_dup (void *s, int l);
 R_API int r_mem_protect(void *ptr, int size, const char *prot);
 R_API int r_mem_set_num (ut8 *dest, int dest_size, ut64 num, int endian);
 R_API int r_mem_eq(ut8 *a, ut8 *b, int len);

libr/util/mem.c

@@ -237,3 +237,10 @@ R_API int r_mem_protect(void *ptr, int size, const char *prot) {
 #endif
 	return R_TRUE;
 }
+
+R_API void *r_mem_dup (void *s, int l) {
+	void *d = malloc (l);
+	if (!d) return NULL;
+	memcpy (d, s, l);
+	return d;
+}