Skip to content

Commit

Permalink
Fix UAF in aaaa on arm/thumb switching ##crash
Browse files Browse the repository at this point in the history
* Reported by @peacock-doris via huntr.dev
* Reproducer tests_65185
* This is a logic fix, but not the fully safe as changes in the code
  can result on UAF again, to properly protect r2 from crashing we
  need to break the ABI and add refcounting to RRegItem, which can't
  happen in 5.6.x because of abi-compat rules
  • Loading branch information
radare authored and trufae committed Mar 21, 2022
1 parent d22d160 commit a7ce296
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
13 changes: 11 additions & 2 deletions libr/anal/fcn.c
Expand Up @@ -815,7 +815,7 @@ static int fcn_recurse(RAnal *anal, RAnalFunction *fcn, ut64 addr, ut64 len, int
// note, we have still increased size of basic block
// (and function)
if (anal->verbose) {
eprintf("Enter branch delay at 0x%08"PFMT64x ". bb->sz=%"PFMT64u"\n", at - oplen, bb->size);
eprintf ("Enter branch delay at 0x%08"PFMT64x ". bb->sz=%"PFMT64u"\n", at - oplen, bb->size);
}
delay.idx = idx - oplen;
delay.cnt = op->delay;
Expand Down Expand Up @@ -882,6 +882,12 @@ static int fcn_recurse(RAnal *anal, RAnalFunction *fcn, ut64 addr, ut64 len, int
// swapped parameters wtf
r_anal_xrefs_set (anal, op->addr, op->ptr, R_ANAL_REF_TYPE_DATA);
}
if (anal->opt.vars && !varset) {
// XXX uses op.src/dst and fails because regprofile invalidates the regitems
// lets just call this BEFORE retpoline() to avoid such issue
r_anal_extract_vars (anal, fcn, op);
}
// this call may cause regprofile changes which cause ranalop.regitem references to be invalid
analyze_retpoline (anal, op);
switch (op->type & R_ANAL_OP_TYPE_MASK) {
case R_ANAL_OP_TYPE_CMOV:
Expand Down Expand Up @@ -973,7 +979,7 @@ static int fcn_recurse(RAnal *anal, RAnalFunction *fcn, ut64 addr, ut64 len, int
fcn->bp_off = fcn->stack - op->src[0]->delta;
}
if (op->dst && op->dst->reg && op->dst->reg->name && op->ptr > 0 && op->ptr != UT64_MAX) {
free(last_reg_mov_lea_name);
free (last_reg_mov_lea_name);
if ((last_reg_mov_lea_name = strdup(op->dst->reg->name))) {
last_reg_mov_lea_val = op->ptr;
last_is_reg_mov_lea = true;
Expand Down Expand Up @@ -1404,10 +1410,13 @@ static int fcn_recurse(RAnal *anal, RAnalFunction *fcn, ut64 addr, ut64 len, int
}
}
}
#if 0
if (anal->opt.vars && !varset) {
// XXX uses op.src/dst and fails because regprofile invalidates the regitems
// we must ranalop in here to avoid uaf
r_anal_extract_vars (anal, fcn, op);
}
#endif
if (op->type != R_ANAL_OP_TYPE_MOV && op->type != R_ANAL_OP_TYPE_CMOV && op->type != R_ANAL_OP_TYPE_LEA) {
last_is_reg_mov_lea = false;
}
Expand Down
2 changes: 1 addition & 1 deletion libr/anal/var.c
Expand Up @@ -1048,7 +1048,7 @@ static void extract_arg(RAnal *anal, RAnalFunction *fcn, RAnalOp *op, const char
free (vartype);
} else {
st64 frame_off = -(ptr + fcn->bp_off);
if (maxstackframe != 0 && (frame_off > maxstackframe || frame_off < -maxstackframe)) {
if (maxstackframe > 0 && (frame_off > maxstackframe || frame_off < -maxstackframe)) {
goto beach;
}
RAnalVar *var = get_stack_var (fcn, frame_off);
Expand Down

0 comments on commit a7ce296

Please sign in to comment.