Vulnerability: The r_bin_mdmp_init_directory_entry function in mdmp.c in radare2 2.7.0 allows remote attackers to cause a denial of service (heap-based buffer over-read and application crash) via a crafted Mini Crash Dump file.
Vulnerability ID: CVE-2018-14016
CWE: CWE-125
Severity score: 2.9
Project: https://github.com/radareorg/radare2
Published: 2018-07-12
File: libr/bin/format/mdmp/mdmp.c
Commit message: Fix #10464 - oobread crash in mdmp (#10683)
Commit author: radare

Vulnerable version (before fix):
 static bool r_bin_mdmp_init_directory_entry(struct r_bin_mdmp_obj *obj, struct m 	/* We could confirm data sizes but a malcious MDMP will always get around 	** this! But we can ensure that the data is not outside of the file */ 	if (entry->location.rva + entry->location.data_size > obj->b->length) { 		eprintf("[ERROR] Size Mismatch - Stream data is larger than file size!\n"); 		return false; 	}  static bool r_bin_mdmp_init_directory_entry(struct r_bin_mdmp_obj *obj, struct m  static bool r_bin_mdmp_init_directory(struct r_bin_mdmp_obj *obj) { 	int i; 	ut8 *directory_base; 	struct minidump_directory *entry;  	directory_base = obj->b->buf + obj->hdr->stream_directory_rva;  	sdb_num_set (obj->kv, "mdmp_directory.offset", 			obj->hdr->stream_directory_rva, 0); static bool r_bin_mdmp_init_directory(struct r_bin_mdmp_obj *obj) { 			"(mdmp_location_descriptor)Location", 0);  	/* Parse each entry in the directory */ 	for (i = 0; i < (int)obj->hdr->number_of_streams; i++) { 		entry = (struct minidump_directory *)(directory_base + (i * sizeof (struct minidump_directory))); 		r_bin_mdmp_init_directory_entry (obj, entry); 	}  	return true;

Fixed version (after fix):
 static bool r_bin_mdmp_init_directory_entry(struct r_bin_mdmp_obj *obj, struct m 	/* We could confirm data sizes but a malcious MDMP will always get around 	** this! But we can ensure that the data is not outside of the file */ 	if (entry->location.rva + entry->location.data_size > obj->b->length) { 		eprintf ("[ERROR] Size Mismatch - Stream data is larger than file size!\n"); 		return false; 	}  static bool r_bin_mdmp_init_directory_entry(struct r_bin_mdmp_obj *obj, struct m  static bool r_bin_mdmp_init_directory(struct r_bin_mdmp_obj *obj) { 	int i; 	struct minidump_directory entry;  	sdb_num_set (obj->kv, "mdmp_directory.offset", 			obj->hdr->stream_directory_rva, 0); static bool r_bin_mdmp_init_directory(struct r_bin_mdmp_obj *obj) { 			"(mdmp_location_descriptor)Location", 0);  	/* Parse each entry in the directory */ 	ut64 rvadir = obj->hdr->stream_directory_rva; 	for (i = 0; i < (int)obj->hdr->number_of_streams; i++) { 		ut32 delta = i * sizeof (struct minidump_directory); 		int r = r_buf_read_at (obj->b, rvadir + delta, (ut8*) &entry, sizeof (struct minidump_directory)); 		if (r) { 			r_bin_mdmp_init_directory_entry (obj, &entry); 		} 	}  	return true;

Patch:
patch: @@ -362,7 +362,7 @@ static bool r_bin_mdmp_init_directory_entry(struct r_bin_mdmp_obj *obj, struct m
 	/* We could confirm data sizes but a malcious MDMP will always get around
 	** this! But we can ensure that the data is not outside of the file */
 	if (entry->location.rva + entry->location.data_size > obj->b->length) {
-		eprintf("[ERROR] Size Mismatch - Stream data is larger than file size!\n");
+		eprintf ("[ERROR] Size Mismatch - Stream data is larger than file size!\n");
 		return false;
 	}
 
@@ -646,10 +646,7 @@ static bool r_bin_mdmp_init_directory_entry(struct r_bin_mdmp_obj *obj, struct m
 
 static bool r_bin_mdmp_init_directory(struct r_bin_mdmp_obj *obj) {
 	int i;
-	ut8 *directory_base;
-	struct minidump_directory *entry;
-
-	directory_base = obj->b->buf + obj->hdr->stream_directory_rva;
+	struct minidump_directory entry;
 
 	sdb_num_set (obj->kv, "mdmp_directory.offset",
 			obj->hdr->stream_directory_rva, 0);
@@ -658,9 +655,13 @@ static bool r_bin_mdmp_init_directory(struct r_bin_mdmp_obj *obj) {
 			"(mdmp_location_descriptor)Location", 0);
 
 	/* Parse each entry in the directory */
+	ut64 rvadir = obj->hdr->stream_directory_rva;
 	for (i = 0; i < (int)obj->hdr->number_of_streams; i++) {
-		entry = (struct minidump_directory *)(directory_base + (i * sizeof (struct minidump_directory)));
-		r_bin_mdmp_init_directory_entry (obj, entry);
+		ut32 delta = i * sizeof (struct minidump_directory);
+		int r = r_buf_read_at (obj->b, rvadir + delta, (ut8*) &entry, sizeof (struct minidump_directory));
+		if (r) {
+			r_bin_mdmp_init_directory_entry (obj, &entry);
+		}
 	}
 
 	return true;
path: libr/bin/format/mdmp/mdmp.c