CVE-2018-14017: Out-of-bounds Read (CWE-125)
value: The r_bin_java_annotation_new function in shlr/java/class.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 .class file because of missing input validation in r_bin_java_line_number_table_attr_new.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2018-07-12T20:29Z
Repository: https://github.com/radareorg/radare2
Language: C
Files changed: shlr/java/class.c
Commit date: 2018-06-27 13:56:59 +0200
Commit message: Fix #10498 - Fix crash in fuzzed java files (#10511)


Diff: diff --git a/shlr/java/class.c b/shlr/java/class.c
index 4d6bf0666f..c28f951463 100644
--- a/shlr/java/class.c
+++ b/shlr/java/class.c
@@ -2068,30 +2068,29 @@ R_API RBinJavaAttrInfo *r_bin_java_read_next_attr(RBinJavaObj *bin, const ut64 o
 	return attr;
 }
 
 R_API RBinJavaAttrInfo *r_bin_java_read_next_attr_from_buffer(ut8 *buffer, st64 sz, st64 buf_offset) {
 	RBinJavaAttrInfo *attr = NULL;
-	char *name = NULL;
 	ut64 offset = 0;
 	ut16 name_idx;
 	st64 nsz;
-	RBinJavaAttrMetas *type_info = NULL;
 
 	if (!buffer || ((int) sz) < 4 || buf_offset < 0) {
 		eprintf ("r_bin_Java_read_next_attr_from_buffer: invalid buffer size %d\n", (int) sz);
 		return NULL;
 	}
 	name_idx = R_BIN_JAVA_USHORT (buffer, offset);
 	offset += 2;
 	nsz = R_BIN_JAVA_UINT (buffer, offset);
 	offset += 4;
-	name = r_bin_java_get_utf8_from_bin_cp_list (R_BIN_JAVA_GLOBAL_BIN, name_idx);
+
+	char *name = r_bin_java_get_utf8_from_bin_cp_list (R_BIN_JAVA_GLOBAL_BIN, name_idx);
 	if (!name) {
 		name = strdup ("unknown");
 	}
 	IFDBG eprintf("r_bin_java_read_next_attr: name_idx = %d is %s\n", name_idx, name);
-	type_info = r_bin_java_get_attr_type_by_name (name);
+	RBinJavaAttrMetas *type_info = r_bin_java_get_attr_type_by_name (name);
 	if (type_info) {
 		IFDBG eprintf("Typeinfo: %s, was %s\n", type_info->name, name);
 		// printf ("SZ %d %d %d\n", nsz, sz, buf_offset);
 		if (nsz > sz) {
 			free (name);
@@ -3430,11 +3429,11 @@ R_API RBinJavaAttrInfo *r_bin_java_code_attr_new(ut8 *buffer, ut64 sz, ut64 buf_
 	attr->info.code_attr.code_length = R_BIN_JAVA_UINT (buffer, offset);
 	offset += 4;
 	// BUG: possible unsigned integer overflow here
 	attr->info.code_attr.code_offset = buf_offset + offset;
 	attr->info.code_attr.code = (ut8 *) malloc (attr->info.code_attr.code_length);
-	if (attr->info.code_attr.code == NULL) {
+	if (!attr->info.code_attr.code) {
 		eprintf ("Handling Code Attributes: Unable to allocate memory "
 			"(%u bytes) for a code.\n", attr->info.code_attr.code_length);
 		return attr;
 	}
 	R_BIN_JAVA_GLOBAL_BIN->current_code_attr = attr;
@@ -3775,22 +3774,22 @@ R_API RBinJavaAttrInfo *r_bin_java_line_number_table_attr_new(ut8 *buffer, ut64
 	offset += 2;
 	attr->info.line_number_table_attr.line_number_table = r_list_newf (free);
 
 	ut32 linenum_len = attr->info.line_number_table_attr.line_number_table_length;
 	RList *linenum_list = attr->info.line_number_table_attr.line_number_table;
-	if (linenum_len > sz) {
-		free (attr);
-		return NULL;
-	}
 	for (i = 0; i < linenum_len; i++) {
 		curpos = buf_offset + offset;
 		// printf ("%llx %llx \n", curpos, sz);
 		// XXX if (curpos + 8 >= sz) break;
 		lnattr = R_NEW0 (RBinJavaLineNumberAttribute);
 		if (!lnattr) {
 			break;
 		}
+		// wtf it works
+		if (offset - 2 > sz) {
+			break;
+		}
 		lnattr->start_pc = R_BIN_JAVA_USHORT (buffer, offset);
 		offset += 2;
 		lnattr->line_number = R_BIN_JAVA_USHORT (buffer, offset);
 		offset += 2;
 		lnattr->file_offset = curpos;
@@ -3980,10 +3979,14 @@ R_API RBinJavaAttrInfo *r_bin_java_local_variable_type_table_attr_new(ut8 *buffe
 		lvattr = R_NEW0 (RBinJavaLocalVariableTypeAttribute);
 		if (!lvattr) {
 			perror ("calloc");
 			break;
 		}
+		if (offset + 10 > sz) {
+			eprintf ("oob");
+			break;
+		}
 		lvattr->start_pc = R_BIN_JAVA_USHORT (buffer, offset);
 		offset += 2;
 		lvattr->length = R_BIN_JAVA_USHORT (buffer, offset);
 		offset += 2;
 		lvattr->name_idx = R_BIN_JAVA_USHORT (buffer, offset);

Vulnerable code: 	char *name = NULL;
	RBinJavaAttrMetas *type_info = NULL;
	name = r_bin_java_get_utf8_from_bin_cp_list (R_BIN_JAVA_GLOBAL_BIN, name_idx);
	type_info = r_bin_java_get_attr_type_by_name (name);
	if (attr->info.code_attr.code == NULL) {
	if (linenum_len > sz) {
		free (attr);
		return NULL;
	}
Fixed code: 
	char *name = r_bin_java_get_utf8_from_bin_cp_list (R_BIN_JAVA_GLOBAL_BIN, name_idx);
	RBinJavaAttrMetas *type_info = r_bin_java_get_attr_type_by_name (name);
	if (!attr->info.code_attr.code) {
		// wtf it works
		if (offset - 2 > sz) {
			break;
		}
		if (offset + 10 > sz) {
			eprintf ("oob");
			break;
		}