Fix oobread crash in DWARF parser (tests_64924) ##crash

Reported by giantbranch of NSFOCUS TIANJI Lab

Author: trufae

libr/anal/dwarf_process.c

@@ -1004,13 +1004,18 @@ static VariableLocation *parse_dwarf_location (Context *ctx, const RBinDwarfAttr
 	for (i = 0; i < block.length; i++) {
 		switch (block.data[i]) {
 		case DW_OP_fbreg: {
-		/* TODO sometimes CFA is referenced, but we don't parse that yet
-		   just an offset involving framebase of a function*/
+			/* TODO sometimes CFA is referenced, but we don't parse that yet
+			   just an offset involving framebase of a function*/
 			if (i == block.length - 1) {
 				return NULL;
 			}
-			const ut8 *dump = &block.data[++i];
-			offset = r_sleb128 (&dump, &block.data[loc->block.length]);
+			i++;
+			const ut8 *dump = block.data + i;
+			if (loc->block.length > block.length) {
+				// eprintf ("skip = %d%c", loc->block.length, 10);
+				return NULL;
+			}
+			offset = r_sleb128 (&dump, block.data + loc->block.length);
 			if (frame_base) {
 				/* recursive parsing, but frame_base should be only one, but someone
 				   could make malicious resource exhaustion attack, so a depth counter might be cool? */
@@ -1019,12 +1024,10 @@ static VariableLocation *parse_dwarf_location (Context *ctx, const RBinDwarfAttr
 					location->offset += offset;
 					return location;
 				}
-				return NULL;
 			} else {
 				/* Might happen if frame_base has a frame_base reference? I don't think it can tho */
-				return NULL;
 			}
-			break;
+			return NULL;
 		}
 		case DW_OP_reg0:
 		case DW_OP_reg1:

libr/bin/dwarf.c

@@ -383,21 +383,18 @@ static inline ut64 dwarf_read_offset(bool is_64bit, const ut8 **buf, const ut8 *
 	if (is_64bit) {
 		result = READ64 (*buf);
 	} else {
-		result = READ32 (*buf);
+		result = (ut64)READ32 (*buf);
 	}
 	return result;
 }
 
 static inline ut64 dwarf_read_address(size_t size, const ut8 **buf, const ut8 *buf_end) {
 	ut64 result;
 	switch (size) {
-		case 2:
-		result = READ16 (*buf); break;
-		case 4:
-		result = READ32 (*buf); break;
-		case 8:
-		result = READ64 (*buf); break;
-		default:
+	case 2: result = READ16 (*buf); break;
+	case 4: result = READ32 (*buf); break;
+	case 8: result = READ64 (*buf); break;
+	default:
 		result = 0;
 		*buf += size;
 		eprintf ("Weird dwarf address size: %zu.", size);
@@ -1857,8 +1854,7 @@ static const ut8 *parse_attr_value(const ut8 *obuf, int obuf_len,
  * @param sdb
  * @return const ut8* Updated buffer
  */
-static const ut8 *parse_die(const ut8 *buf, const ut8 *buf_end, RBinDwarfAbbrevDecl *abbrev,
-		RBinDwarfCompUnitHdr *hdr, RBinDwarfDie *die, const ut8 *debug_str, size_t debug_str_len, Sdb *sdb) {
+static const ut8 *parse_die(const ut8 *buf, const ut8 *buf_end, RBinDwarfAbbrevDecl *abbrev, RBinDwarfCompUnitHdr *hdr, RBinDwarfDie *die, const ut8 *debug_str, size_t debug_str_len, Sdb *sdb) {
 	size_t i;
 	for (i = 0; i < abbrev->count - 1; i++) {
 		memset (&die->attr_values[i], 0, sizeof (die->attr_values[i]));
@@ -1868,9 +1864,8 @@ static const ut8 *parse_die(const ut8 *buf, const ut8 *buf_end, RBinDwarfAbbrevD
 
 		RBinDwarfAttrValue *attribute = &die->attr_values[i];
 
-		bool is_valid_string_form = (attribute->attr_form == DW_FORM_strp ||
-			attribute->attr_form == DW_FORM_string) &&
-			attribute->string.content;
+		bool is_string = (attribute->attr_form == DW_FORM_strp || attribute->attr_form == DW_FORM_string);
+		bool is_valid_string_form = is_string && attribute->string.content;
 		// TODO  does this have a purpose anymore?
 		// Or atleast it needs to rework becase there will be
 		// more comp units -> more comp dirs and only the last one will be kept
@@ -1880,7 +1875,6 @@ static const ut8 *parse_die(const ut8 *buf, const ut8 *buf_end, RBinDwarfAbbrevD
 		}
 		die->count++;
 	}
-
 	return buf;
 }