From 93b5daf60316acf55e10d3a85f1cb9eebf7ec5c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20J=2E=20Rodr=C3=ADguez?= Date: Fri, 3 Jul 2026 10:15:07 +0300 Subject: [PATCH] Do not refresh clobber state for store address registers (#36) The explicit-dst clobber refresh added in the ROP chain search (PR #35) zeroed the clobber counter of a gadget's dst whenever a step wrote it. For store operations (mov [dst], src), dst is the address base register, which is read as an address rather than written, so refreshing it could mask an earlier clobber and yield an invalid ROP chain. Guard the refresh on whether the gadget actually writes dst (Gadget.writes_reg), so it applies to register writes (mov, pop, xchg) but not to store address registers. Add a discriminating regression test. Closes #36 Co-Authored-By: Claude Opus 4.8 --- rop3/gadget.py | 12 ++++++++++++ rop3/ropchain.py | 8 +++++++- tests/test_ropchain.py | 26 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) 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))