CVE-2022-1237: Improper Validation of Array Index (CWE-129)
value: Improper Validation of Array Index in GitHub repository radareorg/radare2 prior to 5.6.8. This vulnerability is heap overflow and may be exploitable. For more general description of heap buffer overflow, see [CWE](https://cwe.mitre.org/data/definitions/122.html).
CWE description: The product uses untrusted input when calculating or using an array index, but the product does not validate or incorrectly validates the index to ensure the index references a valid position within the array.
CVSS v3 base score: 7.8
Published: 2022-04-06T10:15Z
Repository: https://github.com/radareorg/radare2
Language: C
Files changed: libr/bin/format/ne/ne.c
Commit date: 2022-04-05 19:10:49 +0200
Commit message: Fix oobread segfaults in the NE bin parser ##crash

* Reported by @han0nly via huntr.dev
* Reproducers: seed1 seed2
* BountyID: ad3c9c4c-76e7-40c8-bd4a-c095acd8bb40

Diff: diff --git a/libr/bin/format/ne/ne.c b/libr/bin/format/ne/ne.c
index d26860b26f6..210fe036925 100644
--- a/libr/bin/format/ne/ne.c
+++ b/libr/bin/format/ne/ne.c
@@ -1,6 +1,6 @@
-/* radare - LGPL - Copyright 2019 - GustavoLCR */
+/* radare - LGPL - Copyright 2019-2022 - GustavoLCR */
 
 #include "ne.h"
 
 static char *__get_target_os(r_bin_ne_obj_t *bin) {
 	switch (bin->ne_header->targOS) {
@@ -81,14 +81,14 @@ RList *r_bin_ne_get_segments(r_bin_ne_obj_t *bin) {
 		return NULL;
 	}
 	RList *segments = r_list_newf (free);
 	for (i = 0; i < bin->ne_header->SegCount; i++) {
 		RBinSection *bs = R_NEW0 (RBinSection);
-		NE_image_segment_entry *se = &bin->segment_entries[i];
 		if (!bs) {
 			return segments;
 		}
+		NE_image_segment_entry *se = &bin->segment_entries[i];
 		bs->size = se->length;
 		bs->vsize = se->minAllocSz ? se->minAllocSz : 64000;
 		bs->bits = R_SYS_BITS_16;
 		bs->is_data = se->flags & IS_DATA;
 		bs->perm = __translate_perms (se->flags);
@@ -423,11 +423,11 @@ RList *r_bin_ne_get_relocs(r_bin_ne_obj_t *bin) {
 	RList *symbols = bin->symbols;
 	if (!symbols) {
 		return NULL;
 	}
 
-	ut16 *modref = malloc (bin->ne_header->ModRefs * sizeof (ut16));
+	ut16 *modref = calloc (bin->ne_header->ModRefs, sizeof (ut16));
 	if (!modref) {
 		return NULL;
 	}
 	r_buf_read_at (bin->buf, (ut64)bin->ne_header->ModRefTable + bin->header_offset, (ut8 *)modref, bin->ne_header->ModRefs * sizeof (ut16));
 
@@ -449,11 +449,12 @@ RList *r_bin_ne_get_relocs(r_bin_ne_obj_t *bin) {
 		ut16 length = r_buf_read_le16_at (bin->buf, off);
 		if (!length) {
 			continue;
 		}
 		off += 2;
-		while (off < start + length * sizeof (NE_image_reloc_item)) {
+		size_t buf_size = r_buf_size (bin->buf);
+		while (off < start + length * sizeof (NE_image_reloc_item) && off < buf_size) {
 			RBinReloc *reloc = R_NEW0 (RBinReloc);
 			if (!reloc) {
 				return NULL;
 			}
 			NE_image_reloc_item rel;
@@ -482,14 +483,15 @@ RList *r_bin_ne_get_relocs(r_bin_ne_obj_t *bin) {
 				if (!imp) {
 					free (reloc);
 					break;
 				}
 				char *name;
-				if (rel.index > bin->ne_header->ModRefs) {
+				if (rel.index < 1 || rel.index > bin->ne_header->ModRefs) {
 					name = r_str_newf ("UnknownModule%d_%x", rel.index, off); // ????
 				} else {
-					offset = modref[rel.index - 1] + bin->header_offset + bin->ne_header->ImportNameTable;
+					int index = rel.index;
+					offset = modref[index - 1] + bin->header_offset + bin->ne_header->ImportNameTable;
 					name = __read_nonnull_str_at (bin->buf, offset);
 				}
 				if (rel.flags & IMPORTED_ORD) {
 					imp->ordinal = rel.func_ord;
 					imp->name = r_str_newf ("%s.%s", name, __func_name_from_ord(name, rel.func_ord));
@@ -561,20 +563,35 @@ void __init(RBuffer *buf, r_bin_ne_obj_t *bin) {
 	bin->ne_header = R_NEW0 (NE_image_header);
 	if (!bin->ne_header) {
 		return;
 	}
 	bin->buf = buf;
+	// XXX this is endian unsafe
 	r_buf_read_at (buf, bin->header_offset, (ut8 *)bin->ne_header, sizeof (NE_image_header));
+	if (bin->ne_header->FileAlnSzShftCnt > 8) {
+		bin->ne_header->FileAlnSzShftCnt = 8;
+	}
+	if (bin->ne_header->ModRefs * sizeof (ut16) >= r_buf_size (bin->buf)) {
+		bin->ne_header->ModRefs = r_buf_size (bin->buf) / sizeof (ut16);
+	}
 	bin->alignment = 1 << bin->ne_header->FileAlnSzShftCnt;
 	if (!bin->alignment) {
 		bin->alignment = 1 << 9;
 	}
 	bin->os = __get_target_os (bin);
 
 	ut16 offset = bin->ne_header->SegTableOffset + bin->header_offset;
-	ut16 size = bin->ne_header->SegCount * sizeof (NE_image_segment_entry);
+	size_t size = bin->ne_header->SegCount * sizeof (NE_image_segment_entry);
+	if (offset >= r_buf_size (bin->buf)) {
+		return;
+	}
+	size_t remaining = r_buf_size (bin->buf) - offset;
+	size = R_MIN (remaining, size);
 	bin->segment_entries = calloc (1, size);
+	if (size >= remaining) {
+		bin->ne_header->SegCount = size / sizeof (NE_image_segment_entry);
+	}
 	if (!bin->segment_entries) {
 		return;
 	}
 	r_buf_read_at (buf, offset, (ut8 *)bin->segment_entries, size);
 	bin->entry_table = calloc (1, bin->ne_header->EntryTableLength);

Vulnerable code: /* radare - LGPL - Copyright 2019 - GustavoLCR */
		NE_image_segment_entry *se = &bin->segment_entries[i];
	ut16 *modref = malloc (bin->ne_header->ModRefs * sizeof (ut16));
		while (off < start + length * sizeof (NE_image_reloc_item)) {
				if (rel.index > bin->ne_header->ModRefs) {
					offset = modref[rel.index - 1] + bin->header_offset + bin->ne_header->ImportNameTable;
	ut16 size = bin->ne_header->SegCount * sizeof (NE_image_segment_entry);
Fixed code: /* radare - LGPL - Copyright 2019-2022 - GustavoLCR */
		NE_image_segment_entry *se = &bin->segment_entries[i];
	ut16 *modref = calloc (bin->ne_header->ModRefs, sizeof (ut16));
		size_t buf_size = r_buf_size (bin->buf);
		while (off < start + length * sizeof (NE_image_reloc_item) && off < buf_size) {
				if (rel.index < 1 || rel.index > bin->ne_header->ModRefs) {
					int index = rel.index;
					offset = modref[index - 1] + bin->header_offset + bin->ne_header->ImportNameTable;
	// XXX this is endian unsafe
	if (bin->ne_header->FileAlnSzShftCnt > 8) {
		bin->ne_header->FileAlnSzShftCnt = 8;
	}
	if (bin->ne_header->ModRefs * sizeof (ut16) >= r_buf_size (bin->buf)) {
		bin->ne_header->ModRefs = r_buf_size (bin->buf) / sizeof (ut16);
	}
	size_t size = bin->ne_header->SegCount * sizeof (NE_image_segment_entry);
	if (offset >= r_buf_size (bin->buf)) {
		return;
	}
	size_t remaining = r_buf_size (bin->buf) - offset;
	size = R_MIN (remaining, size);
	if (size >= remaining) {
		bin->ne_header->SegCount = size / sizeof (NE_image_segment_entry);
	}