CVE-2022-28068: Out-of-bounds Write (CWE-787)
value: A heap buffer overflow in r_sleb128 function in radare2 5.4.2 and 5.4.0.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.5
Published: 2023-08-22T19:16Z
Repository: https://github.com/radareorg/radare2
Language: C
Files changed: libr/anal/dwarf_process.c, libr/bin/dwarf.c
Commit date: 2021-11-01 00:49:50 +0100
Commit message: Fix oobread crash in DWARF parser (tests_64924) ##crash

Reported by giantbranch of NSFOCUS TIANJI Lab

Diff: diff --git a/libr/anal/dwarf_process.c b/libr/anal/dwarf_process.c
index 2e04560b8483..9062ac6bfb1d 100644
--- a/libr/anal/dwarf_process.c
+++ b/libr/anal/dwarf_process.c
@@ -1002,31 +1002,34 @@ static VariableLocation *parse_dwarf_location (Context *ctx, const RBinDwarfAttr
 	const char *reg_name = NULL; /* literal */
 	size_t i;
 	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? */
 				VariableLocation *location = parse_dwarf_location (ctx, frame_base, NULL);
 				if (location) {
 					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:
 		case DW_OP_reg2:
 		case DW_OP_reg3:
diff --git a/libr/bin/dwarf.c b/libr/bin/dwarf.c
index 67e5b9c2ab41..6a9c5cadf6db 100644
--- a/libr/bin/dwarf.c
+++ b/libr/bin/dwarf.c
@@ -381,25 +381,22 @@ static inline bool is_printable_unit_type(ut64 unit_type) {
 static inline ut64 dwarf_read_offset(bool is_64bit, const ut8 **buf, const ut8 *buf_end) {
 	ut64 result;
 	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);
 	}
 	return result;
@@ -1855,34 +1852,31 @@ static const ut8 *parse_attr_value(const ut8 *obuf, int obuf_len,
  * @param debug_str Ptr to string section start
  * @param debug_str_len Length of the string section
  * @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]));
 
 		buf = parse_attr_value (buf, buf_end - buf, &abbrev->defs[i],
 			&die->attr_values[i], hdr, debug_str, debug_str_len);
 
 		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
 		if (attribute->attr_name == DW_AT_comp_dir && is_valid_string_form) {
 			const char *name = attribute->string.content;
 			sdb_set (sdb, "DW_AT_comp_dir", name, 0);
 		}
 		die->count++;
 	}
-
 	return buf;
 }
 
 /**
  * @brief Reads throught comp_unit buffer and parses all its DIEntries

Vulnerable code: 		/* TODO sometimes CFA is referenced, but we don't parse that yet
		   just an offset involving framebase of a function*/
			const ut8 *dump = &block.data[++i];
			offset = r_sleb128 (&dump, &block.data[loc->block.length]);
				return NULL;
				return NULL;
			break;
		result = READ32 (*buf);
		case 2:
		result = READ16 (*buf); break;
		case 4:
		result = READ32 (*buf); break;
		case 8:
		result = READ64 (*buf); break;
		default:
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) {
		bool is_valid_string_form = (attribute->attr_form == DW_FORM_strp ||
			attribute->attr_form == DW_FORM_string) &&
			attribute->string.content;

Fixed code: 			/* TODO sometimes CFA is referenced, but we don't parse that yet
			   just an offset involving framebase of a function*/
			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);
			return NULL;
		result = (ut64)READ32 (*buf);
	case 2: result = READ16 (*buf); break;
	case 4: result = READ32 (*buf); break;
	case 8: result = READ64 (*buf); break;
	default:
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) {
		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;