Initialize retctx,ctx before freeing the inner elements

In rz_core_analysis_type_match retctx structure was initialized on the stack only after a "goto out_function", where a field of that structure was freed. When the goto path is taken, the field is not properly initialized and it cause cause a crash of Rizin or have other effects. Fixes: CVE-2021-4022
rizinorg · Dec 24, 2021 · 6ce71d8 · 6ce71d8
1 parent 5c099a1
commit 6ce71d8
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 17 deletions.
diff --git a/librz/analysis/var.c b/librz/analysis/var.c
@@ -1103,13 +1103,15 @@ RZ_API void rz_analysis_extract_rarg(RzAnalysis *analysis, RzAnalysisOp *op, RzA
 RZ_API void rz_analysis_extract_vars(RzAnalysis *analysis, RzAnalysisFunction *fcn, RzAnalysisOp *op) {
 	rz_return_if_fail(analysis && fcn && op);
 
-	const char *BP = analysis->reg->name[RZ_REG_NAME_BP];
-	const char *SP = analysis->reg->name[RZ_REG_NAME_SP];
+	const char *BP = rz_reg_get_name(analysis->reg, RZ_REG_NAME_BP);
+	const char *SP = rz_reg_get_name(analysis->reg, RZ_REG_NAME_SP);
 	if (BP) {
 		extract_arg(analysis, fcn, op, BP, "+", RZ_ANALYSIS_VAR_KIND_BPV);
 		extract_arg(analysis, fcn, op, BP, "-", RZ_ANALYSIS_VAR_KIND_BPV);
 	}
-	extract_arg(analysis, fcn, op, SP, "+", RZ_ANALYSIS_VAR_KIND_SPV);
+	if (SP) {
+		extract_arg(analysis, fcn, op, SP, "+", RZ_ANALYSIS_VAR_KIND_SPV);
+	}
 }
 
 static RzList *var_generate_list(RzAnalysis *a, RzAnalysisFunction *fcn, int kind) {

diff --git a/librz/core/analysis_tp.c b/librz/core/analysis_tp.c
@@ -829,6 +829,19 @@ RZ_API void rz_core_analysis_type_match(RzCore *core, RzAnalysisFunction *fcn, H
 	dtrace->ht = ht_pp_new_size(fcn->ninstr, opt.dupvalue, opt.freefn, opt.calcsizeV);
 	dtrace->ht->opt = opt;
 
+	// Create a new context to store the return type propagation state
+	struct ReturnTypeAnalysisCtx retctx = {
+		.resolved = false,
+		.ret_type = NULL,
+		.ret_reg = NULL,
+	};
+	struct TypeAnalysisCtx ctx = {
+		.retctx = &retctx,
+		.cur_idx = 0,
+		.prev_dest = NULL,
+		.str_flag = false
+	};
+
 	HtUP *op_cache = NULL;
 	const char *pc = rz_reg_get_name(core->dbg->reg, RZ_REG_NAME_PC);
 	if (!pc) {
@@ -842,18 +855,6 @@ RZ_API void rz_core_analysis_type_match(RzCore *core, RzAnalysisFunction *fcn, H
 	rz_list_sort(fcn->bbs, bb_cmpaddr);
 	// TODO: The algorithm can be more accurate if blocks are followed by their jmp/fail, not just by address
 	RzAnalysisBlock *bb;
-	// Create a new context to store the return type propagation state
-	struct ReturnTypeAnalysisCtx retctx = {
-		.resolved = false,
-		.ret_type = NULL,
-		.ret_reg = NULL
-	};
-	struct TypeAnalysisCtx ctx = {
-		.retctx = &retctx,
-		.cur_idx = 0,
-		.prev_dest = NULL,
-		.str_flag = false
-	};
 	rz_list_foreach (fcn->bbs, it, bb) {
 		ut64 addr = bb->addr;
 		rz_reg_set_value(core->dbg->reg, r, addr);

diff --git a/librz/core/canalysis.c b/librz/core/canalysis.c
@@ -5358,11 +5358,14 @@ RZ_API void rz_core_analysis_esil(RzCore *core, const char *str, const char *tar
 		ESIL = core->analysis->esil;
 		if (!ESIL) {
 			eprintf("ESIL not initialized\n");
-			return;
+			goto out_pop_regs;
 		}
 		rz_core_analysis_esil_init_mem(core, NULL, UT64_MAX, UT32_MAX);
 	}
 	const char *spname = rz_reg_get_name(core->analysis->reg, RZ_REG_NAME_SP);
+	if (!spname) {
+		goto out_pop_regs;
+	}
 	EsilBreakCtx ctx = {
 		&op,
 		fcn,
@@ -5383,7 +5386,7 @@ RZ_API void rz_core_analysis_esil(RzCore *core, const char *str, const char *tar
 	pcname = rz_reg_get_name(core->analysis->reg, RZ_REG_NAME_PC);
 	if (!pcname || !*pcname) {
 		eprintf("Cannot find program counter register in the current profile.\n");
-		return;
+		goto out_pop_regs;
 	}
 	esil_analysis_stop = false;
 	rz_cons_break_push(cccb, core);
@@ -5691,6 +5694,7 @@ RZ_API void rz_core_analysis_esil(RzCore *core, const char *str, const char *tar
 	ESIL->user = NULL;
 	rz_analysis_op_fini(&op);
 	rz_cons_break_pop();
+out_pop_regs:
 	// restore register
 	rz_reg_arena_pop(core->analysis->reg);
 }