Fix access to uninitialized memory. (#3011)

This zeros memory after reallocation.
It also makes the invalid values in alias_id consistent to be
always 0 if invalid.
Not -1 or 0.

Mapping.c

@@ -458,8 +458,10 @@ static inline bool char_ends_mnem(const char c, cs_arch arch)
 void map_set_alias_id(MCInst *MI, const SStream *O,
 		      const name_map *alias_mnem_id_map, int map_size)
 {
-	if (!MCInst_isAlias(MI))
+	if (!MCInst_isAlias(MI)) {
+		MI->flat_insn->alias_id = 0;
 		return;
+	}
 
 	char alias_mnem[16] = { 0 };
 	int i = 0, j = 0;
@@ -467,7 +469,7 @@ void map_set_alias_id(MCInst *MI, const SStream *O,
 	// Skip spaces and tabs
 	while (is_blank_char(asm_str_buf[i])) {
 		if (!asm_str_buf[i]) {
-			MI->flat_insn->alias_id = -1;
+			MI->flat_insn->alias_id = 0;
 			return;
 		}
 		++i;
@@ -478,8 +480,8 @@ void map_set_alias_id(MCInst *MI, const SStream *O,
 		alias_mnem[j] = asm_str_buf[i];
 	}
 
-	MI->flat_insn->alias_id =
-		name2id(alias_mnem_id_map, map_size, alias_mnem);
+	int alias_id = name2id(alias_mnem_id_map, map_size, alias_mnem);
+	MI->flat_insn->alias_id = alias_id < 0 ? 0 : alias_id;
 }
 
 /// Does a binary search over the given map and searches for @id.

cs.c

@@ -1415,6 +1415,7 @@ size_t CAPSTONE_API cs_disasm(csh ud, const uint8_t *buffer, size_t size,
 		if (f == cache_size) {
 			// full cache, so expand the cache to contain incoming insns
 			cache_size = cache_size * 8 / 5; // * 1.6 ~ golden ratio
+			size_t old_total_size = total_size;
 			total_size += (sizeof(cs_insn) * cache_size);
 			tmp = cs_mem_realloc(total, total_size);
 			if (tmp == NULL) { // insufficient memory
@@ -1429,6 +1430,10 @@ size_t CAPSTONE_API cs_disasm(csh ud, const uint8_t *buffer, size_t size,
 				handle->errnum = CS_ERR_MEM;
 				return 0;
 			}
+			// Zero reallocated memory to prevent
+			// access to uninitialized memory down the line.
+			memset(((uint8_t *)tmp) + old_total_size, 0,
+			       total_size - old_total_size);
 
 			total = tmp;
 			// continue to fill in the cache after the last instruction

include/capstone/capstone.h

@@ -598,7 +598,7 @@ typedef struct cs_insn {
 
 	/// If this instruction is an alias instruction, this member is set with
 	/// the alias ID.
-	/// Otherwise to <ARCH>_INS_INVALID.
+	/// Otherwise to <ARCH>_INS_INVALID (= 0).
 	/// -- Only supported by auto-sync archs --
 	uint64_t alias_id;
 

tests/integration/test_poc.c

@@ -270,6 +270,54 @@ static void test_sh_oob_read_ghsa_5q63_4654_94v6(void)
 	return;
 }
 
+static void test_arm_pop_ghsa_8qp8_2vg2_8mr4(void)
+{
+	uint8_t arm_bytes[132] = { 0 };
+	for (int i = 0; i < 33; i++) {
+		arm_bytes[i * 4 + 0] = 0x00;
+		arm_bytes[i * 4 + 1] = 0x00;
+		arm_bytes[i * 4 + 2] = 0xa0;
+		arm_bytes[i * 4 + 3] = 0xe1;
+	}
+	csh h = { 0 };
+	if (cs_open(CS_ARCH_ARM, CS_MODE_ARM, &h) != CS_ERR_OK) {
+		assert(0);
+		return;
+	}
+
+	if (cs_option(h, CS_OPT_DETAIL, CS_OPT_ON) != CS_ERR_OK) {
+		assert(0);
+		return;
+	}
+	cs_insn *insn;
+	size_t c = cs_disasm(h, arm_bytes, 132, 0x1000, 0, &insn);
+	if (c)
+		cs_free(insn, c);
+	cs_close(&h);
+}
+
+/// Shouldn't trigger MSAN. Just added here to check.
+static void test_tms320_ghsa_8qp8_2vg2_8mr4(void)
+{
+	uint8_t tms_bytes[] = { 0x00, 0x00, 0x18, 0x18 };
+	csh h;
+	if (cs_open(CS_ARCH_TMS320C64X, CS_MODE_BIG_ENDIAN, &h) != CS_ERR_OK) {
+		assert(0);
+		return;
+	}
+
+	if (cs_option(h, CS_OPT_DETAIL, CS_OPT_ON) != CS_ERR_OK) {
+		assert(0);
+		return;
+	}
+
+	cs_insn *insn;
+	size_t c = cs_disasm(h, tms_bytes, 4, 0x1000, 0, &insn);
+	if (c)
+		cs_free(insn, c);
+	cs_close(&h);
+}
+
 int main()
 {
 	test_overflow_cs_insn_bytes();
@@ -280,6 +328,8 @@ int main()
 	test_ub_isintn_xtensa_offset();
 	test_stack_overflow_issue_3010();
 	test_sh_oob_read_ghsa_5q63_4654_94v6();
+	test_arm_pop_ghsa_8qp8_2vg2_8mr4();
+	test_tms320_ghsa_8qp8_2vg2_8mr4();
 
 	return 0;
 }