From 6f55d2e48f8095116391433f1179b9bf56dd6ad1 Mon Sep 17 00:00:00 2001 From: hakril Date: Sat, 19 Oct 2019 00:39:03 +0200 Subject: [PATCH] Fix a bug in debugger when detaching after a Ctrl+c during a BP --- tests/test_debugger.py | 49 ++++++++++++++++++++++++++- windows/debug/debugger.py | 69 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 112 insertions(+), 6 deletions(-) diff --git a/tests/test_debugger.py b/tests/test_debugger.py index 5574dbb..09bd3fc 100644 --- a/tests/test_debugger.py +++ b/tests/test_debugger.py @@ -396,6 +396,7 @@ def test_standard_breakpoint_remove(proc32_64_debug, bptype): class TSTBP(bptype): TARGET = windows.winproxy.CreateFileW def trigger(self, dbg, exc): + # import pdb;pdb.set_trace() addr = exc.ExceptionRecord.ExceptionAddress ctx = dbg.current_thread.context filename = self.extract_arguments(dbg.current_process, dbg.current_thread)["lpFileName"] @@ -403,6 +404,7 @@ def test_standard_breakpoint_remove(proc32_64_debug, bptype): d = windows.debug.Debugger(proc32_64_debug) the_bp = TSTBP("kernel32!CreateFileW") + # import pdb;pdb.set_trace() d.add_bp(the_bp) threading.Thread(target=do_check).start() d.loop() @@ -564,4 +566,49 @@ def test_bp_exe_by_name(proc32_64_debug): # The goal is to test bp of format 'exename!offset' so we craft a string based on the entrypoint d.add_bp(TSTBP("{name}!{offset}".format(name=exename, offset=entrypoint))) d.loop() - assert TSTBP.COUNTER == 1 \ No newline at end of file + assert TSTBP.COUNTER == 1 + + +def test_keyboardinterrupt_when_bp_event(proc32_64_debug, monkeypatch): + class ShouldNotTrigger(windows.debug.Breakpoint): + COUNTER = 0 + def trigger(self, dbg, exc): + raise ValueError("This BP should not trigger in this test !") + + real_WaitForDebugEvent = windows.winproxy.WaitForDebugEvent + + def WaitForDebugEvent_KeyboardInterrupt(debug_event): + real_WaitForDebugEvent(debug_event) + if not debug_event.code == gdef.EXCEPTION_DEBUG_EVENT: + return + if not debug_event.u.Exception.ExceptionRecord.ExceptionCode in [gdef.EXCEPTION_BREAKPOINT, gdef.STATUS_WX86_BREAKPOINT]: + return # Not a BP + if debug_event.u.Exception.ExceptionRecord.ExceptionAddress == addr: + # Our own breakpoint + # Trigger the fake Ctrl+c + raise KeyboardInterrupt("TEST BP") + + xx = monkeypatch.setattr(windows.winproxy, "WaitForDebugEvent", WaitForDebugEvent_KeyboardInterrupt) + + # This should emultate a ctrl+c on when waiting for the event + # Our goal is to set the target back to a good state :) + TEST_CODE = "\xeb\xfe\xff\xff\xff\xff\xff" # Loop + invalid instr + addr = proc32_64_debug.virtual_alloc(0x1000) + proc32_64_debug.write_memory(addr, TEST_CODE) + d = windows.debug.Debugger(proc32_64_debug) + bad_thread = proc32_64_debug.create_thread(addr, 0) + d.add_bp(ShouldNotTrigger(addr)) + d.kill_on_exit(False) + try: + d.loop() + except KeyboardInterrupt as e: + for t in proc32_64_debug.threads: + t.suspend() + d.detach() + # So we have detached when a BP was triggered + # We should have the original memory under the BP + # We should have EIP/RIP decremented by one (should be at not + assert proc32_64_debug.read_memory(addr, len(TEST_CODE)) == TEST_CODE + assert bad_thread.context.pc == addr + else: + raise ValueError("Should have raised") \ No newline at end of file diff --git a/windows/debug/debugger.py b/windows/debug/debugger.py index 010578f..c90ebfe 100644 --- a/windows/debug/debugger.py +++ b/windows/debug/debugger.py @@ -3,7 +3,9 @@ from collections import defaultdict, namedtuple from contextlib import contextmanager import windows +import windows.generated_def as gdef import windows.winobject.exception as winexception + import windows.native_exec.simple_x86 as x86 import windows.native_exec.simple_x64 as x64 @@ -72,6 +74,7 @@ class Debugger(object): # [start] -> (size, current_proctection, original_prot) self._virtual_protected_memory = [] # List of memory-range modified by a MemBP + self._current_debug_event = None @classmethod @@ -132,8 +135,8 @@ class Debugger(object): del self._watched_pages[target.pid] del self._module_by_process[target.pid] - if target is self.current_process: - self._finish_debug_event(self.REMOVE_ME_debug_event, DBG_CONTINUE) + if target is self.current_process: # Bug if CTRL+C and current_process changed ? + self._finish_debug_event(self._current_debug_event, DBG_CONTINUE) self.current_process = None self.current_thread = None @@ -171,13 +174,35 @@ class Debugger(object): def _debug_event_generator(self): while True: debug_event = DEBUG_EVENT() - winproxy.WaitForDebugEvent(debug_event) + try: + winproxy.WaitForDebugEvent(debug_event) + except KeyboardInterrupt as e: + # So we will go out of the loop because of a Ctrl+c + # BP trigger will not be called. + # So AT LEAST quit loop with a coherent context + # Fix thread PC if we just triggered a BP + if (debug_event.code == gdef.EXCEPTION_DEBUG_EVENT and + debug_event.u.Exception.ExceptionRecord.ExceptionCode in [gdef.EXCEPTION_BREAKPOINT, gdef.STATUS_WX86_BREAKPOINT]): + # This is a breakpoint: One of ours ? + bp_addr = debug_event.u.Exception.ExceptionRecord.ExceptionAddress + if bp_addr in self.breakpoints[debug_event.dwProcessId]: + # Leave the thread in a coherent state for detach + # Should it be done in detach ? + # And just stock the "Interrupted debug_event" here ? + thread = self.threads[debug_event.dwThreadId] + ctx = thread.context + ctx.pc -= 1 + thread.set_context(ctx) + raise + finally: # If user Ctrl+c -> we could raise just at the return of WaitForDebugEvent + self._current_debug_event = debug_event yield debug_event def _finish_debug_event(self, event, action): if action not in [windef.DBG_CONTINUE, windef.DBG_EXCEPTION_NOT_HANDLED]: raise ValueError('Unknow action : <0>'.format(action)) winproxy.ContinueDebugEvent(event.dwProcessId, event.dwThreadId, action) + self._current_debug_event = None def _add_exe_to_module_list(self, create_process_event): """Add the intial exe file described by create_process_event to the list of module in the process""" @@ -262,6 +287,7 @@ class Debugger(object): addr = self._resolve(bp.addr, target) if addr is None: return False + dbgprint("Setting soft-BP at <{0:#x}> in <{1}>".format(addr, target), "DBG") bp._addr = addr self._memory_save[target.pid][addr] = target.read_memory(addr, 1) self.breakpoints[target.pid][addr] = bp @@ -303,6 +329,7 @@ class Debugger(object): x[int(empty_drx)] = bp target.set_context(ctx) self.breakpoints[target.owner.pid][addr] = bp + dbgprint("Setting HXBP at <{0:#x}> in <{1}> (Dr{2})".format(addr, target, empty_drx), "DBG") return True def _remove_breakpoint_HXBP(self, bp, target): @@ -496,6 +523,12 @@ class Debugger(object): def _handle_exception_breakpoint(self, exception, excp_addr): excp_bitness = self.get_exception_bitness(exception) + YOLO = False + if self.current_thread.context.EEFlags.TF: + dbgprint("Single step as begin of _handle_exception_breakpoint", "DBG") + # import pdb;pdb.set_trace() + YOLO = True + # Sub-method _do_setup() ? dbg_has_setup = None if not self.first_bp_encoutered: @@ -521,7 +554,9 @@ class Debugger(object): continue_flag = self._dispatch_breakpoint(exception, excp_addr) if self._killed_in_action(): return continue_flag - self._explicit_single_step[self.current_thread.tid] = self.current_thread.context.EEFlags.TF + self._explicit_single_step[self.current_thread.tid] = self.current_thread.context.EEFlags.TF and not YOLO + if self._explicit_single_step[self.current_thread.tid]: + dbgprint("Someone ask for an explicit Single step", "DBG") if excp_addr in self.breakpoints[self.current_process.pid]: # Setup BP if not suppressed self._pass_breakpoint(excp_addr) @@ -539,6 +574,8 @@ class Debugger(object): self.on_single_step(exception) if not self._killed_in_action(): self._explicit_single_step[self.current_thread.tid] = self.current_thread.context.EEFlags.TF + if self._explicit_single_step[self.current_thread.tid]: + dbgprint("Someone ask for an explicit Single step - 7", "DBG") return DBG_CONTINUE elif excp_addr in self.breakpoints[self.current_process.pid]: # Verif that's not a standard BP ? @@ -549,6 +586,8 @@ class Debugger(object): return DBG_CONTINUE ctx = self.current_thread.context self._explicit_single_step[self.current_thread.tid] = ctx.EEFlags.TF + if self._explicit_single_step[self.current_thread.tid]: + dbgprint("Someone ask for an explicit Single step - 2", "DBG") if excp_addr in self.breakpoints[self.current_process.pid]: ctx.EEFlags.RF = 1 self.current_thread.set_context(ctx) @@ -561,6 +600,8 @@ class Debugger(object): # Does not handle case where EEFlags.TF was by the debugge before trigering the exception # Should set the flag explicitly in single_step ? and not just use EEFlags.TF ? self._explicit_single_step[self.current_thread.tid] = self.current_thread.context.EEFlags.TF + if self._explicit_single_step[self.current_thread.tid]: + dbgprint("Someone ask for an explicit Single step - 3", "DBG") return continue_flag else: with self.DisabledMemoryBreakpoint(): @@ -570,6 +611,8 @@ class Debugger(object): # Does not handle case where EEFlags.TF was by the debugge before trigering the exception # Should set the flag explicitly in single_step ? and not just use EEFlags.TF ? self._explicit_single_step[self.current_thread.tid] = self.current_thread.context.EEFlags.TF + if self._explicit_single_step[self.current_thread.tid]: + dbgprint("Someone ask for an explicit Single step - 4", "DBG") return continue_flag # === Testing PAGE_NOACCESS(0x1L) === @@ -616,6 +659,8 @@ class Debugger(object): if self._killed_in_action(): return continue_flag self._explicit_single_step[self.current_thread.tid] = self.current_thread.context.EEFlags.TF + if self._explicit_single_step[self.current_thread.tid]: + dbgprint("Someone ask for an explicit Single step - 5", "DBG") # If BP has not been removed in trigger, pas it if fault_page in cp_watch_page and mem_bp in cp_watch_page[fault_page].bps: self._pass_memory_breakpoint(mem_bp, original_prot, fault_page) @@ -636,17 +681,23 @@ class Debugger(object): excp_code = exception.ExceptionRecord.ExceptionCode excp_addr = exception.ExceptionRecord.ExceptionAddress if excp_code in [EXCEPTION_BREAKPOINT, STATUS_WX86_BREAKPOINT]: + dbgprint("Handle exception as breakpoint", "DBG") return self._handle_exception_breakpoint(exception, excp_addr) elif excp_code in [EXCEPTION_SINGLE_STEP, STATUS_WX86_SINGLE_STEP]: + dbgprint("Handle exception as single step", "DBG") return self._handle_exception_singlestep(exception, excp_addr) elif excp_code == EXCEPTION_ACCESS_VIOLATION: + dbgprint("Handle exception as access_violation", "DBG") return self._handle_exception_access_violation(exception, excp_addr) else: with self.DisabledMemoryBreakpoint(): + dbgprint("Handle exception as on_exception", "DBG") continue_flag = self.on_exception(exception) if self._killed_in_action(): return continue_flag self._explicit_single_step[self.current_thread.tid] = self.current_thread.context.EEFlags.TF + if self._explicit_single_step[self.current_thread.tid]: + dbgprint("Someone ask for an explicit Single step - 6", "DBG") return continue_flag @@ -817,7 +868,6 @@ class Debugger(object): def loop(self): """Debugging loop: handle event / dispatch to breakpoint. Returns when all targets are dead/detached""" for debug_event in self._debug_event_generator(): - self.REMOVE_ME_debug_event = debug_event dbg_continue_flag = self._dispatch_debug_event(debug_event) if dbg_continue_flag is None: dbg_continue_flag = DBG_CONTINUE @@ -966,6 +1016,15 @@ class Debugger(object): return 32 return 64 + @staticmethod + def kill_on_exit(choice): + """If set to True(default in Windows) will kill all attached process on thread exit. + Otherwise, the thread detaches from all processes being debugged on exit. + + See: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-debugsetprocesskillonexit + """ + return windows.winproxy.DebugSetProcessKillOnExit(choice) + # Public callback def on_setup(self): """Called on the first breakpoint event occuring in the debugger.