diff --git a/angrop/chain_builder/__init__.py b/angrop/chain_builder/__init__.py index 410897a..f87bc44 100644 --- a/angrop/chain_builder/__init__.py +++ b/angrop/chain_builder/__init__.py @@ -44,7 +44,8 @@ def __init__(self, project, rop_gadgets, pivot_gadgets, syscall_gadgets, arch, b self._pivot = Pivot(self) self._sys_caller = SysCaller(self) if not SysCaller.supported_os(self.project.loader.main_object.os): - l.warning("%s is not a fully supported OS, SysCaller may not work on this OS", self.project.loader.main_object.os) + l.warning("%s is not a fully supported OS, SysCaller may not work on this OS", + self.project.loader.main_object.os) def set_regs(self, *args, **kwargs): """ diff --git a/angrop/chain_builder/builder.py b/angrop/chain_builder/builder.py index a0d1205..894c03f 100644 --- a/angrop/chain_builder/builder.py +++ b/angrop/chain_builder/builder.py @@ -204,4 +204,4 @@ def _get_fill_val(self): @abstractmethod def update(self): - raise NotImplementedError("each Builder class should have an `update` method!") \ No newline at end of file + raise NotImplementedError("each Builder class should have an `update` method!") diff --git a/angrop/chain_builder/pivot.py b/angrop/chain_builder/pivot.py index ab7b626..91dcc0e 100644 --- a/angrop/chain_builder/pivot.py +++ b/angrop/chain_builder/pivot.py @@ -17,7 +17,7 @@ def cmp(g1, g2): return -1 if g1.stack_change + g1.stack_change_after_pivot > g2.stack_change + g2.stack_change_after_pivot: return 1 - + if g1.block_length < g2.block_length: return -1 if g1.block_length > g2.block_length: @@ -25,7 +25,9 @@ def cmp(g1, g2): return 0 class Pivot(Builder): - + """ + a chain_builder that builds stack pivoting rop chains + """ def __init__(self, chain_builder): super().__init__(chain_builder) self._pivot_gadgets = None @@ -101,9 +103,10 @@ def pivot_reg(self, reg_val): if len(variables) == 1 and variables.pop().startswith(f'reg_{reg}'): return chain else: - chain_str = '\n-----\n'.join([str(self.project.factory.block(g.addr).capstone)for g in chain._gadgets]) + insts = [str(self.project.factory.block(g.addr).capstone) for g in chain._gadgets] + chain_str = '\n-----\n'.join(insts) l.exception("Somehow angrop thinks\n%s\ncan be use for stack pivoting", chain_str) - except Exception as e: + except Exception: continue raise RopException(f"Fail to pivot the stack to {reg}!") @@ -115,7 +118,7 @@ def same_effect(g1, g2): if g1.changed_regs != g2.changed_regs: return False return True - + def better_than(self, g1, g2): if not self.same_effect(g1, g2): return False @@ -144,4 +147,4 @@ def _filter_gadgets(self, gadgets): break gadgets -= to_remove gadgets = sorted(gadgets, key=functools.cmp_to_key(cmp)) - return gadgets \ No newline at end of file + return gadgets diff --git a/angrop/chain_builder/sys_caller.py b/angrop/chain_builder/sys_caller.py index d8cb71a..622f5ae 100644 --- a/angrop/chain_builder/sys_caller.py +++ b/angrop/chain_builder/sys_caller.py @@ -4,9 +4,7 @@ import angr from .func_caller import FuncCaller -from .. import common from ..errors import RopException -from ..rop_gadget import RopGadget l = logging.getLogger(__name__) diff --git a/angrop/gadget_finder/__init__.py b/angrop/gadget_finder/__init__.py index 0259920..2ff6db4 100644 --- a/angrop/gadget_finder/__init__.py +++ b/angrop/gadget_finder/__init__.py @@ -39,8 +39,11 @@ def run_worker(addr): return _global_gadget_analyzer.analyze_gadget(addr) class GadgetFinder: - - def __init__(self, project, fast_mode=None, only_check_near_rets=True, max_block_size=None, max_sym_mem_access=None, is_thumb=False, kernel_mode=False): + """ + a class to find ROP gadgets + """ + def __init__(self, project, fast_mode=None, only_check_near_rets=True, max_block_size=None, + max_sym_mem_access=None, is_thumb=False, kernel_mode=False): # configurations self.project = project self.fast_mode = fast_mode @@ -187,9 +190,10 @@ def _addresses_to_check_with_caching(self, show_progress=True): yield a - def block_hash(self, block): + def block_hash(self, block):# pylint:disable=no-self-use """ a hash to uniquely identify a simple block + TODO: block.bytes is too primitive """ return block.bytes diff --git a/angrop/gadget_finder/gadget_analyzer.py b/angrop/gadget_finder/gadget_analyzer.py index fac5c9e..cb952c0 100644 --- a/angrop/gadget_finder/gadget_analyzer.py +++ b/angrop/gadget_finder/gadget_analyzer.py @@ -34,7 +34,8 @@ def __init__(self, project, fast_mode, kernel_mode=False, arch=None, stack_gsize # initial state that others are based off, all analysis should copy the state first and work on # the copied state self._stack_bsize = stack_gsize * self.project.arch.bytes # number of controllable bytes on stack - self._state = rop_utils.make_symbolic_state(self.project, self.arch.reg_set.union({self.arch.base_pointer}), stack_gsize=stack_gsize) + sym_reg_set = self.arch.reg_set.union({self.arch.base_pointer}) + self._state = rop_utils.make_symbolic_state(self.project, sym_reg_set, stack_gsize=stack_gsize) self._concrete_sp = self._state.solver.eval(self._state.regs.sp) @rop_utils.timeout(3) @@ -224,7 +225,8 @@ def _identify_transit_type(self, final_state, ctrl_type): continue if act.size != self.project.arch.bits: continue - if (act.data.ast == final_state.ip).symbolic or not final_state.solver.eval(act.data.ast == final_state.ip): + if (act.data.ast == final_state.ip).symbolic or \ + not final_state.solver.eval(act.data.ast == final_state.ip): continue sols = final_state.solver.eval_upto(final_state.regs.sp-act.addr.ast, 2) if len(sols) != 1: @@ -738,7 +740,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 a.addr.ast.symbolic and not a.addr.ast.variables - sp_vars: continue # ignore read/write on stack diff --git a/angrop/rop.py b/angrop/rop.py index bd5e8d5..d928232 100644 --- a/angrop/rop.py +++ b/angrop/rop.py @@ -50,7 +50,9 @@ def __init__(self, only_check_near_rets=True, max_block_size=None, max_sym_mem_a self.roparg_filler = None # gadget finder configurations - self.gadget_finder = GadgetFinder(self.project, fast_mode=fast_mode, only_check_near_rets=only_check_near_rets, max_block_size=max_block_size, max_sym_mem_access=max_sym_mem_access, is_thumb=is_thumb, kernel_mode=kernel_mode) + self.gadget_finder = GadgetFinder(self.project, fast_mode=fast_mode, only_check_near_rets=only_check_near_rets, + max_block_size=max_block_size, max_sym_mem_access=max_sym_mem_access, + is_thumb=is_thumb, kernel_mode=kernel_mode) self.arch = self.gadget_finder.arch # chain builder @@ -69,12 +71,13 @@ def _screen_gadgets(self): # the duplicates (other gadgets with the same instructions) block = self.project.factory.block(g.addr) h = self.gadget_finder.block_hash(block) + addr = None if h not in self._duplicates: continue for addr in self._duplicates[h]: if not self._contain_badbytes(addr): break - else: + if not addr: continue g = self.gadget_finder.analyze_gadget(addr) if type(g) is RopGadget: @@ -103,7 +106,8 @@ def find_gadgets(self, processes=4, show_progress=True): Saves stack pivots in self.stack_pivots :param processes: number of processes to use """ - self._all_gadgets, self._duplicates = self.gadget_finder.find_gadgets(processes=processes, show_progress=show_progress) + self._all_gadgets, self._duplicates = self.gadget_finder.find_gadgets(processes=processes, + show_progress=show_progress) self._screen_gadgets() return self.rop_gadgets @@ -113,7 +117,8 @@ def find_gadgets_single_threaded(self, show_progress=True): Saves gadgets in self.gadgets Saves stack pivots in self.stack_pivots """ - self._all_gadgets, self._duplicates = self.gadget_finder.find_gadgets_single_threaded(show_progress=show_progress) + self._all_gadgets, self._duplicates = self.gadget_finder.find_gadgets_single_threaded( + show_progress=show_progress) self._screen_gadgets() return self.rop_gadgets diff --git a/angrop/rop_gadget.py b/angrop/rop_gadget.py index 9d087af..44fbf22 100644 --- a/angrop/rop_gadget.py +++ b/angrop/rop_gadget.py @@ -176,9 +176,9 @@ def __str__(self): for move in self.reg_moves: s += "Register move: [%s to %s, %d bits]\n" % (move.from_reg, move.to_reg, move.bits) s += "Register dependencies:\n" - for reg in self.reg_dependencies: + for reg, deps in self.reg_dependencies.items(): controllers = self.reg_controllers.get(reg, []) - dependencies = [x for x in self.reg_dependencies[reg] if x not in controllers] + dependencies = [x for x in deps if x not in controllers] s += " " + reg + ": [" + " ".join(controllers) + " (" + " ".join(dependencies) + ")]" + "\n" for mem_access in self.mem_changes: if mem_access.op == "__add__": @@ -273,7 +273,7 @@ def sp_controllers(self): def __repr__(self): return f"" - + def copy(self): new = super().copy() new.stack_change_after_pivot = self.stack_change_after_pivot diff --git a/angrop/rop_utils.py b/angrop/rop_utils.py index df1f6fc..469722c 100644 --- a/angrop/rop_utils.py +++ b/angrop/rop_utils.py @@ -231,7 +231,7 @@ def step_one_block(project, state, stop_at_syscall=False): last_inst_addr = block.capstone.insns[-2].address else: last_inst_addr = block.capstone.insns[-1].address - for i in range(num_insts): # considering that it may get into kernel mode + for _ in range(num_insts): # considering that it may get into kernel mode if state.addr != last_inst_addr: state = step_one_inst(project, state, stop_at_syscall=stop_at_syscall) if stop_at_syscall and is_in_kernel(project, state): @@ -243,8 +243,7 @@ def step_one_block(project, state, stop_at_syscall=False): if stop_at_syscall and is_in_kernel(project, succ.flat_successors[0]): return None, succ.flat_successors[0] return succ, None - else: - raise RopException("Fail to reach the last instruction!") + raise RopException("Fail to reach the last instruction!") def step_one_inst(project, state, stop_at_syscall=False): if is_in_kernel(project, state): @@ -262,7 +261,8 @@ def step_one_inst(project, state, stop_at_syscall=False): raise RopException(f"fail to step state: {state}") return succ.flat_successors[0] -def step_to_unconstrained_successor(project, state, max_steps=2, allow_simprocedures=False, stop_at_syscall=False, precise_action=False): +def step_to_unconstrained_successor(project, state, max_steps=2, allow_simprocedures=False, + stop_at_syscall=False, precise_action=False): """ steps up to two times to try to find an unconstrained successor :param state: the input state diff --git a/tests/test_find_gadgets.py b/tests/test_find_gadgets.py index 40c0f25..1f59b7a 100644 --- a/tests/test_find_gadgets.py +++ b/tests/test_find_gadgets.py @@ -3,7 +3,6 @@ import angr import angrop # pylint: disable=unused-import -from angrop.rop_gadget import RopGadget, PivotGadget l = logging.getLogger(__name__) @@ -58,6 +57,7 @@ def test_arm_thumb_mode(): assert gadget.block_length == 6 def test_pivot_gadget(): + # pylint: disable=pointless-string-statement proj = angr.Project(os.path.join(tests_dir, "i386", "bronze_ropchain"), auto_load_libs=False) rop = proj.analyses.ROP()