Various fixes

rop3/gadget.py

@@ -61,7 +61,7 @@ def has_src(self) -> bool:
 
     def calculate_side_effects(self) -> None:
         arch = arch_singleton.arch
-        excluded = {arch.normalize_reg(r) for r in (self.dst, self.src, arch.sp) if r is not None}
+        excluded = {arch.normalize_reg(r) for r in (self.dst, arch.sp) if r is not None}
 
         for decode in self.decodes:
             explicit = {decode.reg_name(r) for r in decode.regs_write}

rop3/operation.py

@@ -249,9 +249,16 @@ def is_dst(self):
 
     def set_dst(self, dst):
         if self.is_dst():
-            self.reg = dst
             self.generic = False
-            self.type = arch_singleton.arch.op_reg
+            if self.is_mem():
+                self.reg = dst
+            else:
+                try:
+                    self.imm = self._parse_imm(dst)
+                    self.type = arch_singleton.arch.op_imm
+                except (ValueError, TypeError):
+                    self.reg = dst
+                    self.type = arch_singleton.arch.op_reg
 
     def is_src(self):
         if self.reg is not None:
@@ -261,17 +268,21 @@ def is_src(self):
     def set_src(self, src):
         if self.is_src():
             self.generic = False
-            try:
-                self.imm = self._parse_imm(src)
-                self.type = arch_singleton.arch.op_imm
-            except (ValueError, TypeError):
+            if self.is_mem():
                 self.reg = src
-                self.type = arch_singleton.arch.op_reg
+            else:
+                try:
+                    self.imm = self._parse_imm(src)
+                    self.type = arch_singleton.arch.op_imm
+                except (ValueError, TypeError):
+                    self.reg = src
+                    self.type = arch_singleton.arch.op_reg
 
     def is_equal(self, decode, operand):
         operand_reg = None
 
-        if self.generic and self.is_src() and operand.type == arch_singleton.arch.op_imm:
+        # Allows generic reg -> imm substitution (not for mem)
+        if self.generic and self.is_src() and self.is_reg() and operand.type == arch_singleton.arch.op_imm:
             return (True, operand.value.imm)
 
         if self.type != operand.type:

rop3/ropchain.py

@@ -234,9 +234,19 @@ def backtrack(
                 for gad in comb_gadgets[index]:
                     for side_reg in gad.side_regs:
                         side_effected[side_reg] += 1
+
+                    norm_dst = arch.normalize_reg(gad.dst) if gad.dst else None
+                    saved_dst = side_effected[norm_dst] if norm_dst else 0
+                    if saved_dst:
+                        side_effected[norm_dst] = 0
+
                     ropchain.append(gad)
                     yield from backtrack(index + 1, ropchain, side_effected)
                     ropchain.pop()
+
+                    if saved_dst:
+                        side_effected[norm_dst] = saved_dst
+
                     for side_reg in gad.side_regs:
                         side_effected[side_reg] -= 1
 

rop3/roplang/mov.yaml

@@ -44,18 +44,18 @@ mov:
       op2: src
 
   # clc
-  # cmovc dst, src
-  - 
+  # cmovae dst, src
+  -
     - mnemonic: clc
-    - mnemonic: cmovc
+    - mnemonic: cmovae
       op1: dst
       op2: src
 
   # stc
-  # cmovc dst, src
+  # cmovb dst, src
   -
     - mnemonic: stc
-    - mnemonic: cmovc
+    - mnemonic: cmovb
       op1: dst
       op2: src
 

tests/test_operation.py

@@ -65,3 +65,81 @@ def test_operand_parse_imm_supports_hex_and_negative(x64):
     assert op._parse_imm('0xffffffff') == 0xffffffff
     assert op._parse_imm('-1') == -1
     assert op._parse_imm(42) == 42
+
+
+def test_ld_with_src_matches_memory_not_register(x64):
+    '''
+    Regression (#30, #33): `ld` (mov dst, [src]) with a concrete --src must
+    match a memory load `mov <reg>, [src]`, not a register move `mov <reg>, src`.
+    A previous bug overwrote the memory operand type with op_reg in set_src.
+    '''
+    gadgets = [
+        make_gadget(b'\x48\x8b\x03\xc3', 0x1000),   # mov rax, [rbx] ; ret
+        make_gadget(b'\x48\x89\xd8\xc3', 0x1010),   # mov rax, rbx ; ret (must NOT match)
+    ]
+    matched = operation.Operation('ld', src='rbx').filter_gadgets(gadgets)
+    assert [g.text_repr for g in matched] == ['mov rax, qword ptr [rbx] ; ret']
+
+
+def test_ld_does_not_match_immediate_load(x64):
+    '''
+    Regression (#33, error 3): a generic memory address must not be resolved
+    into an immediate, so `mov rax, 0xcafe` is not a valid `ld` (load).
+    '''
+    gadgets = [
+        make_gadget(b'\x48\xc7\xc0\xfe\xca\x00\x00\xc3', 0x1000),   # mov rax, 0xcafe ; ret
+    ]
+    assert operation.Operation('ld').filter_gadgets(gadgets) == []
+
+
+def test_set_dst_preserves_memory_type(x64):
+    '''
+    Regression (#33, error 1): binding a concrete register to a `[dst]`
+    placeholder must keep the operand a memory operand, not turn it into a reg.
+    '''
+    op = operation.Operand('[dst]')
+    assert op.is_mem()
+    op.set_dst('rax')
+    assert op.is_mem()
+    assert op.reg == 'rax'
+
+
+def test_set_src_preserves_memory_type(x64):
+    ''' Regression (#33, error 1): same as above for the `[src]` placeholder. '''
+    op = operation.Operand('[src]')
+    assert op.is_mem()
+    op.set_src('rbx')
+    assert op.is_mem()
+    assert op.reg == 'rbx'
+
+
+def test_set_dst_accepts_immediate(x64):
+    '''
+    Regression (#33, error 2): set_dst must accept immediates like set_src,
+    producing an op_imm operand rather than rejecting the value.
+    '''
+    op = operation.Operand('dst')
+    op.set_dst('0x10')
+    assert op.is_imm()
+    assert op.imm == 0x10
+
+
+def test_xchg_src_counted_as_side_effect(x64):
+    '''
+    Regression (#31): in `xchg dst, src` the `src` register is clobbered, so it
+    must be reported as a side effect (it was wrongly excluded before).
+    '''
+    gadget = make_gadget(b'\x48\x93\xc3', 0x1000)   # xchg rbx, rax ; ret
+    matched = operation.Operation('mov', dst='rbx', src='rax').filter_gadgets([gadget])
+    assert len(matched) == 1
+    assert 'rax' in matched[0].side_regs
+
+
+def test_mov_matches_clc_cmovae(x64):
+    '''
+    Regression (#32): the mov ROPLang uses the valid Capstone mnemonics
+    `cmovae`/`cmovb` (not `cmovc`), so `clc ; cmovae dst, src` is a valid mov.
+    '''
+    gadget = make_gadget(b'\xf8\x48\x0f\x43\xc3\xc3', 0x1000)   # clc ; cmovae rax, rbx ; ret
+    matched = operation.Operation('mov', dst='rax', src='rbx').filter_gadgets([gadget])
+    assert [g.text_repr for g in matched] == ['clc ; cmovae rax, rbx ; ret']

tests/test_ropchain.py

@@ -65,3 +65,32 @@ def test_search_generic_registers(x64):
     results = list(RopChain(None).search(gadgets, chain))
     assert results
     assert all(len(r) == 1 for r in results)
+
+
+def test_explicit_dst_clears_clobbered_register(x64):
+    '''
+    Regression (#34): a register written by a later step must no longer be
+    considered clobbered by an earlier step.
+
+    Step 1 `lc(rax)` uses `pop rax ; pop rbx ; ret`, which clobbers rbx.
+    Step 2 `lc(rbx)` rewrites rbx, so it must be clean again for step 3
+    `mov(rdx, rbx)`, which reads rbx. Before the fix the stale clobber on rbx
+    blocked step 3 and no chain was found.
+    '''
+    gadgets = [
+        make_gadget(b'\x58\x5b\xc3', 0x1000),       # pop rax ; pop rbx ; ret
+        make_gadget(b'\x5b\xc3', 0x1010),           # pop rbx ; ret
+        make_gadget(b'\x48\x89\xda\xc3', 0x1020),   # mov rdx, rbx ; ret
+    ]
+    chain = [
+        _op('lc', dst='rax'),
+        _op('lc', dst='rbx'),
+        _op('mov', dst='rdx', src='rbx'),
+    ]
+    results = list(RopChain(None).search(gadgets, chain))
+    assert results
+    assert [g.text_repr for g in results[0]] == [
+        'pop rax ; pop rbx ; ret',
+        'pop rbx ; ret',
+        'mov rdx, rbx ; ret',
+    ]