Merge pull request #85 from angr/fix/pivot

filter out 'repz ret' because angr does not handle it properly atm

angrop/arch.py

@@ -42,7 +42,8 @@ def __init__(self, project, kernel_mode=False):
 
     def block_make_sense(self, block):
         capstr = str(block.capstone).lower()
-        if 'cli' in capstr or 'rex' in capstr:
+        # currently, angrop does not handle "repz ret" correctly, we filter it
+        if any(x in capstr for x in ('cli', 'rex', 'repz ret')):
             return False
         if not self.kernel_mode:
             if "fs:" in capstr or "gs:" in capstr or "iret" in capstr:

angrop/chain_builder/pivot.py

@@ -115,18 +115,20 @@ def pivot_reg(self, reg_val):
     def same_effect(g1, g2):
         if g1.sp_controllers != g2.sp_controllers:
             return False
-        if g1.changed_regs != g2.changed_regs:
+        if g1.stack_change != g2.stack_change:
+            return False
+        if g1.stack_change_after_pivot != g2.stack_change_after_pivot:
             return False
         return True
 
     def better_than(self, g1, g2):
         if not self.same_effect(g1, g2):
             return False
-        if g1.stack_change > g2.stack_change:
+        if g1.num_mem_access > g2.num_mem_access:
             return False
-        if g1.stack_change_after_pivot > g2.stack_change_after_pivot:
+        if not g1.changed_regs.issubset(g2.changed_regs):
             return False
-        if g1.num_mem_access > g2.num_mem_access:
+        if g1.block_length > g2.block_length:
             return False
         return True
 

angrop/gadget_finder/__init__.py

@@ -124,7 +124,7 @@ def get_duplicates(self):
 
     def find_gadgets(self, processes=4, show_progress=True):
         gadgets = []
-        self._cache = defaultdict(set)
+        self._cache = {}
 
         initargs = (self.gadget_analyzer,)
         with Pool(processes=processes, initializer=_set_global_gadget_analyzer, initargs=initargs) as pool:
@@ -137,7 +137,7 @@ def find_gadgets(self, processes=4, show_progress=True):
 
     def find_gadgets_single_threaded(self, show_progress=True):
         gadgets = []
-        self._cache = defaultdict(set)
+        self._cache = {}
 
         assert self.gadget_analyzer is not None
 
@@ -186,8 +186,13 @@ def _addresses_to_check_with_caching(self, show_progress=True):
                 continue
             if self._is_simple_gadget(a, bl):
                 h = self.block_hash(bl)
-                self._cache[h].add(a)
-
+                if h not in self._cache:
+                    self._cache[h] = {a}
+                else:
+                    # we only return the first unique gadget
+                    # so skip duplicates
+                    self._cache[h].add(a)
+                    continue
             yield a
 
     def block_hash(self, block):# pylint:disable=no-self-use

angrop/gadget_finder/gadget_analyzer.py

@@ -701,22 +701,30 @@ def _does_syscall(self, symbolic_p):
 
         return False
 
+    def _is_pivot_action(self, act):
+        """
+        check whether an sim_action is a stack pivoting action
+        """
+        if act.type != 'reg' or act.action != 'write':
+            return False
+        try:
+            storage = act.storage
+        except KeyError:
+            return False
+        if storage != self.arch.stack_pointer:
+            return False
+        # this gadget has done symbolic pivoting if there is a symbolic write to the stack pointer
+        if act.data.symbolic:
+            return True
+        return False
+
     def _does_pivot(self, final_state):
         """
         checks if the path does a stack pivoting at some point
         :param final_state: the state that finishes the gadget execution
         """
         for act in final_state.history.actions:
-            if act.type != 'reg' or act.action != 'write':
-                continue
-            try:
-                storage = act.storage
-            except KeyError:
-                continue
-            if storage != self.arch.stack_pointer:
-                continue
-            # this gadget has done symbolic pivoting if there is a symbolic write to the stack pointer
-            if act.data.symbolic:
+            if self._is_pivot_action(act):
                 return True
         return False
 
@@ -729,9 +737,14 @@ def _analyze_mem_access(self, final_state, init_state, gadget):
         """
         all_mem_actions = []
         sp_vars = final_state.regs.sp.variables
+        pivot_done = False
 
         # step 1: filter out irrelevant actions and irrelevant memory accesses
         for a in final_state.history.actions.hardcopy:
+            if self._is_pivot_action(a):
+                pivot_done = True
+                continue
+
             if a.type != 'mem':
                 continue
 
@@ -740,7 +753,7 @@ def _analyze_mem_access(self, final_state, init_state, gadget):
                 continue
 
             # ignore read/write on stack after pivot
-            if a.addr.ast.symbolic and not a.addr.ast.variables - sp_vars:
+            if pivot_done and a.addr.ast.symbolic and not a.addr.ast.variables - sp_vars:
                 continue
 
             # ignore read/write on stack

tests/test_find_gadgets.py

@@ -91,6 +91,27 @@ def test_pivot_gadget():
     gadget = rop.analyze_gadget(0x439ad3)
     assert gadget is None
 
+    proj = angr.Project(os.path.join(tests_dir, "x86_64", "libc.so.6"), auto_load_libs=False)
+    rop = proj.analyses.ROP()
+
+    """
+    402bc8  leave
+    402bc9  clc
+    402bca  repz ret
+    """
+    gadget = rop.analyze_gadget(0x402bc8)
+    assert gadget is None
+
+    # this is not a valid gadget because sal shifts the memory
+    # and we don't fully control the shifted memory
+    """
+    50843e  sal     byte ptr [rbp-0x11], cl
+    508441  leave
+    508442  ret
+    """
+    gadget = rop.analyze_gadget(0x50843e)
+    assert gadget is None
+
 def test_syscall_gadget():
     proj = angr.Project(os.path.join(tests_dir, "i386", "bronze_ropchain"), auto_load_libs=False)
     rop = proj.analyses.ROP()