Do not refresh clobber state for store address registers (#36)

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

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
 

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))