diff --git a/rop3/gadget.py b/rop3/gadget.py index d2b6488..3a09a6b 100644 --- a/rop3/gadget.py +++ b/rop3/gadget.py @@ -72,6 +72,18 @@ def calculate_side_effects(self) -> None: if normalized not in excluded: self.side_regs.add(normalized) + def writes_reg(self, normalized_reg: str) -> bool: + ''' Whether the gadget explicitly or implicitly writes normalized_reg. ''' + arch = arch_singleton.arch + for decode in self.decodes: + explicit = {decode.reg_name(r) for r in decode.regs_write} + _, implicit_ids = decode.regs_access() + implicit = {decode.reg_name(r) for r in implicit_ids} + for reg in explicit | implicit: + if arch.normalize_reg(reg) == normalized_reg: + return True + return False + def subsumes(self, rhs) -> bool: if str(self.dst) != str(rhs.dst): return False diff --git a/rop3/ropchain.py b/rop3/ropchain.py index 59ff4ef..890e65d 100644 --- a/rop3/ropchain.py +++ b/rop3/ropchain.py @@ -235,8 +235,14 @@ def backtrack( for side_reg in gad.side_regs: side_effected[side_reg] += 1 + # A gadget that explicitly writes its dst produces a fresh + # value there, so clear any earlier clobber on it. Skip this + # for store operations (mov [dst], src): there dst is the + # address base register, which is read (not written), so its + # clobber state must be preserved (issue #36). norm_dst = arch.normalize_reg(gad.dst) if gad.dst else None - saved_dst = side_effected[norm_dst] if norm_dst else 0 + refresh_dst = bool(norm_dst) and gad.writes_reg(norm_dst) + saved_dst = side_effected[norm_dst] if refresh_dst else 0 if saved_dst: side_effected[norm_dst] = 0 diff --git a/tests/test_ropchain.py b/tests/test_ropchain.py index b1a06f0..9d532c8 100644 --- a/tests/test_ropchain.py +++ b/tests/test_ropchain.py @@ -94,3 +94,29 @@ def test_explicit_dst_clears_clobbered_register(x64): 'pop rbx ; ret', 'mov rdx, rbx ; ret', ] + + +def test_store_dst_does_not_clear_clobbered_address_register(x64): + ''' + Regression (#36): a store `st(rbx, rax)` is `mov [rbx], rax`, where rbx is + the address base register (read, not written). It must NOT refresh rbx's + clobber state. + + Step 1 `lc(rcx)` uses `pop rcx ; pop rbx ; ret`, which clobbers rbx. + Step 2 `st(rbx, rax)` reads rbx as an address; before the fix it wrongly + cleared rbx's clobber, so step 3 `mov(rdx, rbx)` (which reads rbx) was + allowed and an invalid chain was produced. After the fix rbx stays + clobbered and no chain is found. + ''' + gadgets = [ + make_gadget(b'\x59\x5b\xc3', 0x1000), # pop rcx ; pop rbx ; ret + make_gadget(b'\x48\x89\x03\xc3', 0x1010), # mov [rbx], rax ; ret + make_gadget(b'\x48\x89\xda\xc3', 0x1020), # mov rdx, rbx ; ret + ] + chain = [ + _op('lc', dst='rcx'), + _op('st', dst='rbx', src='rax'), + _op('mov', dst='rdx', src='rbx'), + ] + with pytest.raises(ropchain_mod.RopChainNotFound): + list(RopChain(None).search(gadgets, chain))