From 2e009f0f69d60000ef5e9ff05957791c6a6a5410 Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 15 Apr 2017 16:33:28 -0700 Subject: [PATCH 01/11] Modify exception and thread handling so Ctrl+C works Changed all `except:` to `except Exception:` and set `Thread.daemon` to `True` for all threads. --- CHANGELOG.rst | 1 + boofuzz/pedrpc.py | 35 ++++++++++++++++++++-------------- boofuzz/utils/crash_binning.py | 2 +- network_monitor.py | 6 +++--- process_monitor.py | 7 ++++--- process_monitor_unix.py | 4 +++- unit_tests/primitives.py | 4 ++-- utils/crashbin_explorer.py | 4 ++-- utils/pcap_cleaner.py | 2 +- vmcontrol.py | 10 +++++----- 10 files changed, 43 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8bbe4360..0e2a2cad 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -3,6 +3,7 @@ Next Features -------- - Console output - now with colors! +- SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py Fixes ----- diff --git a/boofuzz/pedrpc.py b/boofuzz/pedrpc.py index 1025b0ba..5393996b 100644 --- a/boofuzz/pedrpc.py +++ b/boofuzz/pedrpc.py @@ -4,6 +4,8 @@ import socket import cPickle +import select + class Client: def __init__(self, host, port): @@ -43,14 +45,14 @@ def __connect(self): self.__server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) self.__server_sock.settimeout(3.0) self.__server_sock.connect((self.__host, self.__port)) - except: + except Exception: if self.__retry != 5: self.__retry += 1 time.sleep(5) self.__connect() else: sys.stderr.write("PED-RPC> unable to connect to server %s:%d\n" % (self.__host, self.__port)) - raise Exception + raise # disable timeouts and lingering. self.__server_sock.settimeout(None) self.__server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, self.NOLINGER) @@ -104,7 +106,7 @@ def __method_missing(self, method_name, *args, **kwargs): try: self.__pickle_send((method_name, (args, kwargs))) break - except: + except Exception: # re-connect to the PED-RPC server if the sock died. self.__connect() @@ -131,7 +133,7 @@ def __pickle_recv(self): # it gets worse. you would think that simply returning here would break things, but it doesn't. # gotta track this down at some point. length = struct.unpack(" connection to server severed during recv()\n") - raise Exception + raise return cPickle.loads(received) @@ -165,9 +167,9 @@ def __pickle_send(self, data): try: self.__server_sock.send(struct.pack(" connection to server severed during send()\n") - raise Exception + raise class Server: @@ -184,7 +186,7 @@ def __init__(self, host, port): self.__server.settimeout(None) self.__server.bind((host, port)) self.__server.listen(1) - except: + except Exception: sys.stderr.write("unable to bind to %s:%d\n" % (host, port)) sys.exit(1) @@ -221,7 +223,7 @@ def __pickle_recv(self): chunk = self.__client_sock.recv(length) received += chunk length -= len(chunk) - except: + except Exception: sys.stderr.write("PED-RPC> connection client severed during recv()\n") raise Exception @@ -245,7 +247,7 @@ def __pickle_send(self, data): try: self.__client_sock.send(struct.pack(" connection to client severed during send()\n") raise Exception @@ -257,7 +259,12 @@ def serve_forever(self): self.__disconnect() # accept a client connection. - (self.__client_sock, self.__client_address) = self.__server.accept() + while True: + readable, writeable, errored = select.select([self.__server], [], [], .1) + if len(readable) > 0: + assert readable[0] == self.__server + (self.__client_sock, self.__client_address) = self.__server.accept() + break self.__debug("accepted connection from %s:%d" % (self.__client_address[0], self.__client_address[1])) @@ -265,7 +272,7 @@ def serve_forever(self): try: (method_name, (args, kwargs)) = self.__pickle_recv() self.__debug("%s(args=%s, kwargs=%s)" % (method_name, args, kwargs)) - except: + except Exception: continue try: @@ -282,5 +289,5 @@ def serve_forever(self): # transmit the return value to the client, continue on socket disconnect. try: self.__pickle_send(ret) - except: + except Exception: continue diff --git a/boofuzz/utils/crash_binning.py b/boofuzz/utils/crash_binning.py index 4af9bab3..4d1bcb5c 100644 --- a/boofuzz/utils/crash_binning.py +++ b/boofuzz/utils/crash_binning.py @@ -266,7 +266,7 @@ def last_crash_synopsis(self): for (addr, handler, handler_str) in self.last_crash.seh_unwind: try: disasm = self.pydbg.disasm(handler) - except: + except Exception: disasm = "[INVALID]" synopsis += "\t%08x -> %s %s\n" % (addr, handler_str, disasm) diff --git a/network_monitor.py b/network_monitor.py index 01d2dc55..c732c4d7 100644 --- a/network_monitor.py +++ b/network_monitor.py @@ -68,11 +68,11 @@ def create_usage(ifs): # if there is a DHCP address snag that, otherwise fall back to the IP address. try: ip = _winreg.QueryValueEx(key, "DhcpIPAddress")[0] - except: + except Exception: ip = _winreg.QueryValueEx(key, "IPAddress")[0][0] pcapy_device = pcapy_device + "\t" + ip - except: + except Exception: pass message += " [%d] %s\n" % (index, pcapy_device) @@ -282,7 +282,7 @@ def main(): # Now wait in a way that will not block signals like SIGINT helpers.pause_for_signal() - except: + except Exception: pass if __name__ == "__main__": diff --git a/process_monitor.py b/process_monitor.py index ad626f69..af3a35be 100644 --- a/process_monitor.py +++ b/process_monitor.py @@ -104,7 +104,7 @@ def run(self): self.dbg.attach(self.pid) self.dbg.run() self.process_monitor.log("debugger thread-%s exiting" % self.getName()) - except: + except Exception: pass # TODO: removing the following line appears to cause some concurrency issues. @@ -169,7 +169,7 @@ def __init__(self, host, port, crash_filename, proc=None, pid_to_ignore=None, le # restore any previously recorded crashes. try: self.crash_bin.import_file(self.crash_filename) - except: + except Exception: pass self.log("Process Monitor PED-RPC server initialized:") @@ -276,13 +276,14 @@ def pre_send(self, test_number): # un-serialize the crash bin from disk. this ensures we have the latest copy (ie: vmware image is cycling). try: self.crash_bin.import_file(self.crash_filename) - except: + except Exception: pass # if we don't already have a debugger thread, instantiate and start one now. if not self.debugger_thread or not self.debugger_thread.isAlive(): self.log("creating debugger thread", 5) self.debugger_thread = DebuggerThread(self, self.proc_name, self.ignore_pid) + self.debugger_thread.daemon = True self.debugger_thread.start() self.log("giving debugger thread 2 seconds to settle in", 5) time.sleep(2) diff --git a/process_monitor_unix.py b/process_monitor_unix.py index e111ea1b..4c3eb758 100644 --- a/process_monitor_unix.py +++ b/process_monitor_unix.py @@ -192,7 +192,9 @@ def start_target(self): self.dbg = DebuggerThread(self.start_commands[0]) self.dbg.spawn_target() # prevent blocking by spawning off another thread to waitpid - threading.Thread(target=self.dbg.start_monitoring).start() + t = threading.Thread(target=self.dbg.start_monitoring) + t.daemon = True + t.start() self.log("done. target up and running, giving it 5 seconds to settle in.") time.sleep(5) return True diff --git a/unit_tests/primitives.py b/unit_tests/primitives.py index b13bea73..57d9ffb9 100644 --- a/unit_tests/primitives.py +++ b/unit_tests/primitives.py @@ -74,7 +74,7 @@ def fuzz_extension_tests(): try: shutil.move(".fuzz_strings", ".fuzz_strings_backup") shutil.move(".fuzz_ints", ".fuzz_ints_backup") - except: + except Exception: pass # create extension libraries for unit test. @@ -112,5 +112,5 @@ def fuzz_extension_tests(): try: shutil.move(".fuzz_strings_backup", ".fuzz_strings") shutil.move(".fuzz_ints_backup", ".fuzz_ints") - except: + except Exception: pass \ No newline at end of file diff --git a/utils/crashbin_explorer.py b/utils/crashbin_explorer.py index cdf5ad7a..a79b08aa 100644 --- a/utils/crashbin_explorer.py +++ b/utils/crashbin_explorer.py @@ -20,7 +20,7 @@ raise Exception opts, args = getopt.getopt(sys.argv[2:], "t:g:", ["test=", "graph="]) -except: +except Exception: print USAGE sys.exit(1) @@ -35,7 +35,7 @@ try: crashbin = utils.crash_binning.CrashBinning() crashbin.import_file(sys.argv[1]) -except: +except Exception: print "unable to open crashbin: '%s'." % sys.argv[1] sys.exit(1) diff --git a/utils/pcap_cleaner.py b/utils/pcap_cleaner.py index b1fbcfb5..cfa3dd66 100644 --- a/utils/pcap_cleaner.py +++ b/utils/pcap_cleaner.py @@ -20,7 +20,7 @@ try: crashbin = utils.crash_binning.CrashBinning() crashbin.import_file(sys.argv[1]) -except: +except Exception: print "unable to open crashbin: '%s'." % sys.argv[1] sys.exit(1) diff --git a/vmcontrol.py b/vmcontrol.py index 4f6d6a3b..2686747f 100755 --- a/vmcontrol.py +++ b/vmcontrol.py @@ -15,7 +15,7 @@ from win32api import GetShortPathName # noinspection PyUnresolvedReferences from win32com.shell import shell -except: +except Exception: if os.name == "nt": print "[!] Failed to import win32api/win32com modules, please install these! Bailing..." sys.exit(1) @@ -82,7 +82,7 @@ def __init__(self, host, port, vmrun, vmx, snap_name=None, log_level=1, interact vmrun = fullpath + "\\vmrun.exe" print "[*] Using %s" % vmrun break - except: + except Exception: print "[!] Error while trying to find vmrun.exe. Try again without -i." sys.exit(1) @@ -110,7 +110,7 @@ def __init__(self, host, port, vmrun, vmx, snap_name=None, log_level=1, interact break else: print "[!] No .vmx file found in the selected folder, please try again" - except: + except Exception: print "[!] Error while trying to find the .vmx file. Try again without -i." sys.exit(1) @@ -286,7 +286,7 @@ def is_target_running(self): try: line = GetShortPathName(line) # skip invalid paths. - except: + except Exception: continue if self.vmx.lower() == line.lower(): @@ -350,7 +350,7 @@ def __init__(self, host, port, vmrun, vmx, snap_name=None, log_level=1, interact vmrun = fullpath + "\\VBoxManage.exe" print "[*] Using %s" % vmrun break - except: + except Exception: print "[!] Error while trying to find VBoxManage.exe. Try again without -I." sys.exit(1) From 1733c87b67bc4c84b32c68fc3f8fb54edac5c08a Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 15 Apr 2017 16:43:30 -0700 Subject: [PATCH 02/11] Make procmon --crash_bin optional (as documented) --- CHANGELOG.rst | 1 + process_monitor.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0e2a2cad..f0e838ad 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,6 +8,7 @@ Features Fixes ----- - Fixed: The pedrpc module was not being properly included in imports. +- Made process_monitor.py --crash_bin optional (as documented) 0.0.7 ===== diff --git a/process_monitor.py b/process_monitor.py index af3a35be..ec3dc05d 100644 --- a/process_monitor.py +++ b/process_monitor.py @@ -372,7 +372,7 @@ def set_stop_commands(self, new_stop_commands): PORT = int(arg) if not crash_bin: - ERR(USAGE) + crash_bin = 'crash-bin' # spawn the PED-RPC servlet. try: From 1bd176fbf02b66131a8eefc17409a34bc98337a3 Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 15 Apr 2017 18:04:00 -0700 Subject: [PATCH 03/11] Fixed procmon crashes when certain parameters... aren't given --- CHANGELOG.rst | 6 ++- boofuzz/pedrpc.py | 9 ++-- process_monitor.py | 108 ++++++++++++++++++++++++--------------------- 3 files changed, 65 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f0e838ad..9930f062 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -3,12 +3,14 @@ Next Features -------- - Console output - now with colors! -- SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py +- SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py. Fixes ----- - Fixed: The pedrpc module was not being properly included in imports. -- Made process_monitor.py --crash_bin optional (as documented) +- Made process_monitor.py --crash_bin optional (as documented). +- Improved procmon (process monitor) behavior when certain parameters aren't given. +- Improved procmon error handling. 0.0.7 ===== diff --git a/boofuzz/pedrpc.py b/boofuzz/pedrpc.py index 5393996b..7e4a2d96 100644 --- a/boofuzz/pedrpc.py +++ b/boofuzz/pedrpc.py @@ -276,15 +276,12 @@ def serve_forever(self): continue try: - # resolve a pointer to the requested method and call it. - # Wat. - exec("method_pointer = self.%s" % method_name) - # noinspection PyUnresolvedReferences - ret = method_pointer(*args, **kwargs) + method = getattr(self, method_name) + ret = method(*args, **kwargs) except AttributeError: # if the method can't be found notify the user and raise an error sys.stderr.write("PED-RPC> remote method %s cannot be found\n" % method_name) - continue + raise # transmit the return value to the client, continue on socket disconnect. try: diff --git a/process_monitor.py b/process_monitor.py index ec3dc05d..50e48db7 100644 --- a/process_monitor.py +++ b/process_monitor.py @@ -1,4 +1,5 @@ #!c:\\python\\python.exe +from __future__ import print_function import subprocess import threading import getopt @@ -96,19 +97,20 @@ def run(self): Main thread routine, called on thread.start(). Thread exits when this routine returns. """ - self.process_monitor.log("debugger thread-%s looking for process name: %s" % (self.getName(), self.proc_name)) + if self.proc_name is not None: + self.process_monitor.log("debugger thread-%s looking for process name: %s" % (self.getName(), self.proc_name)) - # watch for and try attaching to the process. - try: - self.watch() - self.dbg.attach(self.pid) - self.dbg.run() - self.process_monitor.log("debugger thread-%s exiting" % self.getName()) - except Exception: - pass + # watch for and try attaching to the process. + try: + self.watch() + self.dbg.attach(self.pid) + self.dbg.run() + self.process_monitor.log("debugger thread-%s exiting" % self.getName()) + except Exception: + pass - # TODO: removing the following line appears to cause some concurrency issues. - del self.dbg + # TODO: removing the following line appears to cause some concurrency issues. + del self.dbg def watch(self): """ @@ -232,7 +234,7 @@ def log(self, msg="", level=1): """ if self.log_level >= level: - print "[%s] %s" % (time.strftime("%I:%M.%S"), msg) + print("[%s] %s" % (time.strftime("%I:%M.%S"), msg)) def post_send(self): """ @@ -243,24 +245,27 @@ def post_send(self): """ crashes = 0 - av = self.debugger_thread.access_violation - - # if there was an access violation, wait for the debugger thread to finish then kill thread handle. - # it is important to wait for the debugger thread to finish because it could be taking its sweet ass time - # uncovering the details of the access violation. - if av: - while self.debugger_thread.isAlive(): - time.sleep(1) - - self.debugger_thread = None - - # serialize the crash bin to disk. - self.crash_bin.export_file(self.crash_filename) - # for binary in self.crash_bin.bins.keys(): - # crashes += len(self.crash_bin.bins[binary]) - for binary, crash_list in self.crash_bin.bins.iteritems(): - crashes += len(crash_list) - return not av + if self.debugger_thread is None: + return True + else: + av = self.debugger_thread.access_violation + + # if there was an access violation, wait for the debugger thread to finish then kill thread handle. + # it is important to wait for the debugger thread to finish because it could be taking its sweet ass time + # uncovering the details of the access violation. + if av: + while self.debugger_thread.isAlive(): + time.sleep(1) + + self.debugger_thread = None + + # serialize the crash bin to disk. + self.crash_bin.export_file(self.crash_filename) + # for binary in self.crash_bin.bins.keys(): + # crashes += len(self.crash_bin.bins[binary]) + for binary, crash_list in self.crash_bin.bins.iteritems(): + crashes += len(crash_list) + return not av def pre_send(self, test_number): """ @@ -279,14 +284,7 @@ def pre_send(self, test_number): except Exception: pass - # if we don't already have a debugger thread, instantiate and start one now. - if not self.debugger_thread or not self.debugger_thread.isAlive(): - self.log("creating debugger thread", 5) - self.debugger_thread = DebuggerThread(self, self.proc_name, self.ignore_pid) - self.debugger_thread.daemon = True - self.debugger_thread.start() - self.log("giving debugger thread 2 seconds to settle in", 5) - time.sleep(2) + self.start_target() def start_target(self): """ @@ -294,14 +292,28 @@ def start_target(self): @returns True if successful. No failure detection yet. """ + # if we don't already have a debugger thread, instantiate and start one now. + if not self.debugger_thread or not self.debugger_thread.isAlive(): + if len(self.start_commands) > 0: + self.log("starting target process") + + for command in self.start_commands: + try: + subprocess.Popen(command) + except WindowsError as e: + print('WindowsError "{0}" while starting "{1}"'.format(e.strerror, command), file=sys.stderr) + return False - self.log("starting target process") + self.log("done. target up and running, giving it 5 seconds to settle in.") + time.sleep(5) - for command in self.start_commands: - subprocess.Popen(command) + self.log("creating debugger thread", 5) + self.debugger_thread = DebuggerThread(self, self.proc_name, self.ignore_pid) + self.debugger_thread.daemon = True + self.debugger_thread.start() + self.log("giving debugger thread 2 seconds to settle in", 5) + time.sleep(2) - self.log("done. target up and running, giving it 5 seconds to settle in.") - time.sleep(5) return True def stop_target(self): @@ -375,10 +387,6 @@ def set_stop_commands(self, new_stop_commands): crash_bin = 'crash-bin' # spawn the PED-RPC servlet. - try: - servlet = ProcessMonitorPedrpcServer("0.0.0.0", PORT, crash_bin, proc_name, ignore_pid, log_level) - servlet.serve_forever() - except Exception as e: - # TODO: Add servlet.shutdown - # TODO: Add KeyboardInterrupt - ERR("Error starting RPC server!\n\t%s" % e) + servlet = ProcessMonitorPedrpcServer("0.0.0.0", PORT, crash_bin, proc_name, ignore_pid, log_level) + servlet.serve_forever() + # TODO: Handle errors From 2564eafeab7a2fe0e3c13ef14fd0afdd7e78ca55 Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 15 Apr 2017 20:32:08 -0700 Subject: [PATCH 04/11] Make procmon track processes by PID instead of... process name, which is somewhat kludgy. As a consequence, the stop_commands and proc_name arguments are no longer needed. --- CHANGELOG.rst | 6 ++-- boofuzz/sessions.py | 4 +-- process_monitor.py | 75 ++++++++++++++++++++++++++++----------------- 3 files changed, 53 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9930f062..f47e5d71 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -3,13 +3,15 @@ Next Features -------- - Console output - now with colors! -- SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py. +- The process monitor (procmon) now tracks processes by PID rather than searching by name. Therefore, stop_commands + and proc_name are no longer required. +- SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py (usually). Fixes ----- - Fixed: The pedrpc module was not being properly included in imports. - Made process_monitor.py --crash_bin optional (as documented). -- Improved procmon (process monitor) behavior when certain parameters aren't given. +- Improved procmon behavior when certain parameters aren't given. - Improved procmon error handling. 0.0.7 diff --git a/boofuzz/sessions.py b/boofuzz/sessions.py index 43b3b697..312f740f 100644 --- a/boofuzz/sessions.py +++ b/boofuzz/sessions.py @@ -82,8 +82,8 @@ def pedrpc_connect(self): time.sleep(1) # connection established. - for key in self.procmon_options.keys(): - eval('self.procmon.set_%s(self.procmon_options["%s"])' % (key, key)) + for key, value in self.procmon_options.items(): + getattr(self.procmon, 'set_{0}'.format(key))(value) # If the network monitor is alive, set it's options if self.netmon: diff --git a/process_monitor.py b/process_monitor.py index 50e48db7..ddc96ea9 100644 --- a/process_monitor.py +++ b/process_monitor.py @@ -25,7 +25,7 @@ class DebuggerThread(threading.Thread): - def __init__(self, process_monitor, process, pid_to_ignore=None): + def __init__(self, process_monitor, proc_name=None, ignore_pid=None, pid=None): """ Instantiate a new PyDbg instance and register user and access violation callbacks. """ @@ -33,13 +33,13 @@ def __init__(self, process_monitor, process, pid_to_ignore=None): threading.Thread.__init__(self) self.process_monitor = process_monitor - self.proc_name = process - self.ignore_pid = pid_to_ignore + self.proc_name = proc_name + self.ignore_pid = ignore_pid self.access_violation = False self.active = True self.dbg = pydbg.pydbg() - self.pid = None + self.pid = pid # give this thread a unique name. self.setName("%d" % time.time()) @@ -97,12 +97,15 @@ def run(self): Main thread routine, called on thread.start(). Thread exits when this routine returns. """ - if self.proc_name is not None: - self.process_monitor.log("debugger thread-%s looking for process name: %s" % (self.getName(), self.proc_name)) + if self.proc_name is not None or self.pid is not None: # watch for and try attaching to the process. try: - self.watch() + if self.pid is None and self.proc_name is not None: + self.process_monitor.log( + "debugger thread-%s looking for process name: %s" % (self.getName(), self.proc_name)) + self.watch() + self.process_monitor.log("debugger thread-%s attaching to pid: %s" % (self.getName(), self.pid)) self.dbg.attach(self.pid) self.dbg.run() self.process_monitor.log("debugger thread-%s exiting" % self.getName()) @@ -158,6 +161,7 @@ def __init__(self, host, port, crash_filename, proc=None, pid_to_ignore=None, le self.stop_commands = [] self.start_commands = [] + self._process = None self.test_number = None self.debugger_thread = None self.crash_bin = utils.crash_binning.CrashBinning() @@ -181,6 +185,13 @@ def __init__(self, host, port, crash_filename, proc=None, pid_to_ignore=None, le self.log("\t log level: %d" % self.log_level) self.log("awaiting requests...") + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + if self._process is not None: + self._process.kill() + # noinspection PyMethodMayBeStatic def alive(self): """ @@ -299,7 +310,7 @@ def start_target(self): for command in self.start_commands: try: - subprocess.Popen(command) + self._process = subprocess.Popen(command) except WindowsError as e: print('WindowsError "{0}" while starting "{1}"'.format(e.strerror, command), file=sys.stderr) return False @@ -307,12 +318,14 @@ def start_target(self): self.log("done. target up and running, giving it 5 seconds to settle in.") time.sleep(5) - self.log("creating debugger thread", 5) - self.debugger_thread = DebuggerThread(self, self.proc_name, self.ignore_pid) - self.debugger_thread.daemon = True - self.debugger_thread.start() - self.log("giving debugger thread 2 seconds to settle in", 5) - time.sleep(2) + if self._process is not None: + self.log("creating debugger thread", 5) + self.debugger_thread = DebuggerThread(self, proc_name=self.proc_name, ignore_pid=self.ignore_pid, + pid=self._process.pid) + self.debugger_thread.daemon = True + self.debugger_thread.start() + self.log("giving debugger thread 2 seconds to settle in", 5) + time.sleep(2) return True @@ -326,15 +339,18 @@ def stop_target(self): self.log("stopping target process") - for command in self.stop_commands: - if command == "TERMINATE_PID": - dbg = pydbg.pydbg() - for (pid, name) in dbg.enumerate_processes(): - if name.lower() == self.proc_name.lower(): - os.system("taskkill /pid %d" % pid) - break - else: - os.system(command) + if len(self.stop_commands) < 1: + self._process.kill() + else: + for command in self.stop_commands: + if command == "TERMINATE_PID": + dbg = pydbg.pydbg() + for (pid, name) in dbg.enumerate_processes(): + if name.lower() == self.proc_name.lower(): + os.system("taskkill /pid %d" % pid) + break + else: + os.system(command) def set_proc_name(self, new_proc_name): self.log("updating target process name to '%s'" % new_proc_name) @@ -349,7 +365,8 @@ def set_stop_commands(self, new_stop_commands): self.stop_commands = new_stop_commands -if __name__ == "__main__": +def main(): + global PORT opts = None # parse command line options. try: @@ -386,7 +403,9 @@ def set_stop_commands(self, new_stop_commands): if not crash_bin: crash_bin = 'crash-bin' - # spawn the PED-RPC servlet. - servlet = ProcessMonitorPedrpcServer("0.0.0.0", PORT, crash_bin, proc_name, ignore_pid, log_level) - servlet.serve_forever() - # TODO: Handle errors + with ProcessMonitorPedrpcServer("0.0.0.0", PORT, crash_bin, proc_name, ignore_pid, log_level) as servlet: + servlet.serve_forever() + + +if __name__ == "__main__": + main() From 18633041365d72d75778f585c286192ebce5e9ee Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 22 Apr 2017 17:59:42 -0700 Subject: [PATCH 05/11] Add --help parameter to process monitor Refactored script to use the cement framework. --- CHANGELOG.rst | 1 + process_monitor.py | 85 +++++++++++++++++++--------------------------- setup.py | 2 +- 3 files changed, 36 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f47e5d71..ae109dcb 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,7 @@ Features - Console output - now with colors! - The process monitor (procmon) now tracks processes by PID rather than searching by name. Therefore, stop_commands and proc_name are no longer required. +- Added --help parameter to process monitor. - SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py (usually). Fixes diff --git a/process_monitor.py b/process_monitor.py index ddc96ea9..ccd57547 100644 --- a/process_monitor.py +++ b/process_monitor.py @@ -2,26 +2,18 @@ from __future__ import print_function import subprocess import threading -import getopt import time import sys import os import pydbg import pydbg.defines -from boofuzz import utils +from cement.core.foundation import CementApp +from boofuzz import utils from boofuzz import pedrpc -PORT = 26002 -ERR = lambda msg: sys.stderr.write("ERR> " + msg + "\n") or sys.exit(1) -USAGE = """USAGE: process_monitor.py - [-c|--crash_bin FILENAME] filename to serialize crash bin class to - [-p|--proc_name NAME] process name to search for and attach to - [-i|--ignore_pid PID] PID to ignore when searching for target process - [-l|--log_level LEVEL] log level: default 1, increase for more verbosity - [-P|--port PORT] TCP port to bind this agent to - """ +DEFAULT_PORT = 26002 class DebuggerThread(threading.Thread): @@ -274,7 +266,7 @@ def post_send(self): self.crash_bin.export_file(self.crash_filename) # for binary in self.crash_bin.bins.keys(): # crashes += len(self.crash_bin.bins[binary]) - for binary, crash_list in self.crash_bin.bins.iteritems(): + for binary, crash_list in self.crash_bin.bins.items(): crashes += len(crash_list) return not av @@ -365,47 +357,38 @@ def set_stop_commands(self, new_stop_commands): self.stop_commands = new_stop_commands -def main(): - global PORT - opts = None - # parse command line options. - try: - # TODO: Refactor to use something less dumb - opts, args = getopt.getopt( - sys.argv[1:], - "c:i:l:p:", - [ - "crash_bin=", - "ignore_pid=", - "log_level=", - "proc_name=", - "port=" - ] - ) - except getopt.GetoptError: - ERR(USAGE) - - crash_bin = ignore_pid = proc_name = None - log_level = 1 - - for opt, arg in opts: - if opt in ("-c", "--crash_bin"): - crash_bin = arg - if opt in ("-i", "--ignore_pid"): - ignore_pid = int(arg) - if opt in ("-l", "--log_level"): - log_level = int(arg) - if opt in ("-p", "--proc_name"): - proc_name = arg - if opt in ("-P", "--port"): - PORT = int(arg) - - if not crash_bin: - crash_bin = 'crash-bin' - - with ProcessMonitorPedrpcServer("0.0.0.0", PORT, crash_bin, proc_name, ignore_pid, log_level) as servlet: +def serve_procmon(port, crash_bin, proc_name, ignore_pid, log_level): + with ProcessMonitorPedrpcServer(host="0.0.0.0", port=port, crash_filename=crash_bin, proc=proc_name, + pid_to_ignore=ignore_pid, level=log_level) as servlet: servlet.serve_forever() +class ProcessMonitorApp(CementApp): + class Meta: + label = 'procmon' + + +def main(): + with ProcessMonitorApp() as app: + app.args.add_argument('-f', '--foo', action='store', metavar='STR', + help='the notorious foo option') + app.args.add_argument("-c", "--crash_bin", help='filename to serialize crash bin class to', default='crash-bin', + metavar='FILENAME') + app.args.add_argument("-i", "--ignore_pid", help='PID to ignore when searching for target process', type=int, + metavar='PID') + app.args.add_argument("-l", "--log_level", help='log level: default 1, increase for more verbosity', type=int, + default=1, metavar='LEVEL') + app.args.add_argument("-p", "--proc_name", help='process name to search for and attach to', metavar='NAME') + app.args.add_argument("-P", "--port", help='TCP port to bind this agent to', type=int, default=DEFAULT_PORT) + + app.run() # parses args + + serve_procmon(port=app.pargs.port, + crash_bin=app.pargs.crash_bin, + proc_name=app.pargs.proc_name, + ignore_pid=app.pargs.ignore_pid, + log_level=app.pargs.log_level) + + if __name__ == "__main__": main() diff --git a/setup.py b/setup.py index b4626a7d..d1b3dd1c 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ def find_version(*path_elements): package_data={'boofuzz': ['web/templates/*', 'web/static/css/*']}, install_requires=[ 'future', 'pyserial', 'pydot', 'tornado==4.0.2', - 'Flask==0.10.1', 'impacket', 'colorama'], + 'Flask==0.10.1', 'impacket', 'colorama', 'cement'], extras_require={ # This list is duplicated in tox.ini. Make sure to change both! 'dev': ['check-manifest', 'mock', 'pytest', 'pytest-bdd', 'netifaces', 'ipaddress'], From 3156b864f926038970605f48ffc19a111d50237a Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 22 Apr 2017 18:48:08 -0700 Subject: [PATCH 06/11] Add restart_target method to procmons... Simplifying the interface between Session and procmon and fixing a bug in which the target would not be properly stopped if it failed without crashing. --- CHANGELOG.rst | 1 + boofuzz/sessions.py | 13 ++++--------- process_monitor.py | 11 ++++++++++- process_monitor_unix.py | 9 +++++++++ 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ae109dcb..dadce74e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,7 @@ Fixes - Made process_monitor.py --crash_bin optional (as documented). - Improved procmon behavior when certain parameters aren't given. - Improved procmon error handling. +- Fixed a bug in which the procmon would not properly restart a target that had failed without crashing. 0.0.7 ===== diff --git a/boofuzz/sessions.py b/boofuzz/sessions.py index 312f740f..7d91f8b6 100644 --- a/boofuzz/sessions.py +++ b/boofuzz/sessions.py @@ -599,7 +599,7 @@ def _process_failures(self, target): self.total_mutant_index += skipped self.fuzz_node.mutant_index += skipped - self.restart_target(target, stop_first=False) + self.restart_target(target) # noinspection PyUnusedLocal def post_send(self, target, fuzz_data_logger, session, sock, *args, **kwargs): @@ -654,7 +654,7 @@ def pre_send(self, sock): # default to doing nothing. pass - def restart_target(self, target, stop_first=True): + def restart_target(self, target): """ Restart the fuzz target. If a VMControl is available revert the snapshot, if a process monitor is available restart the target process. Otherwise, do nothing. @@ -662,9 +662,6 @@ def restart_target(self, target, stop_first=True): @type target: session.target @param target: Target we are restarting - @type stop_first: bool - @param stop_first: Set to True to stop the target before starting. - @raise sex.BoofuzzRestartFailedError if restart fails. """ @@ -681,13 +678,11 @@ def restart_target(self, target, stop_first=True): # if we have a connected process monitor, restart the target process. elif target.procmon: self._fuzz_data_logger.log_info("restarting target process") - if stop_first: - target.procmon.stop_target() - if not target.procmon.start_target(): + if not target.procmon.restart_target(): raise sex.BoofuzzRestartFailedError() - # give the process a few seconds to settle in. + self._fuzz_data_logger.log_info("giving the process 3 seconds to settle in ") time.sleep(3) # otherwise all we can do is wait a while for the target to recover on its own. diff --git a/process_monitor.py b/process_monitor.py index ccd57547..f845f86e 100644 --- a/process_monitor.py +++ b/process_monitor.py @@ -293,7 +293,7 @@ def start_target(self): """ Start up the target process by issuing the commands in self.start_commands. - @returns True if successful. No failure detection yet. + @returns True if successful. """ # if we don't already have a debugger thread, instantiate and start one now. if not self.debugger_thread or not self.debugger_thread.isAlive(): @@ -344,6 +344,15 @@ def stop_target(self): else: os.system(command) + def restart_target(self): + """ + Stop and start the target process. + + @returns True if successful. + """ + self.stop_target() + return self.start_target() + def set_proc_name(self, new_proc_name): self.log("updating target process name to '%s'" % new_proc_name) self.proc_name = new_proc_name diff --git a/process_monitor_unix.py b/process_monitor_unix.py index 4c3eb758..41a496cc 100644 --- a/process_monitor_unix.py +++ b/process_monitor_unix.py @@ -215,6 +215,15 @@ def stop_target(self): else: os.system(command) + def restart_target(self): + """ + Stop and start the target process. + + @returns True if successful. + """ + self.stop_target() + return self.start_target() + def set_start_commands(self, start_commands): """ We expect start_commands to be a list with one element for example From e4723204d43bd758077f56df419af1c7c7424f14 Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 22 Apr 2017 21:45:32 -0700 Subject: [PATCH 07/11] Fix overly broad exception handle in unix procmon --- boofuzz/pedrpc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boofuzz/pedrpc.py b/boofuzz/pedrpc.py index 7e4a2d96..d0ca844d 100644 --- a/boofuzz/pedrpc.py +++ b/boofuzz/pedrpc.py @@ -277,11 +277,11 @@ def serve_forever(self): try: method = getattr(self, method_name) - ret = method(*args, **kwargs) except AttributeError: # if the method can't be found notify the user and raise an error - sys.stderr.write("PED-RPC> remote method %s cannot be found\n" % method_name) + sys.stderr.write('PED-RPC> remote method "{0}" of {1} cannot be found\n'.format(method_name, self)) raise + ret = method(*args, **kwargs) # transmit the return value to the client, continue on socket disconnect. try: From 64b6dd66d20477a952dd039af0436411976572e0 Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 22 Apr 2017 21:46:00 -0700 Subject: [PATCH 08/11] Make unix procmon more compatible with Windows Make stop_commands optional and allow using lists in start_commands --- CHANGELOG.rst | 1 + process_monitor_unix.py | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index dadce74e..631b18b5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,7 @@ Features and proc_name are no longer required. - Added --help parameter to process monitor. - SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py (usually). +- Made Unix procmon more compatible with Windows. Fixes ----- diff --git a/process_monitor_unix.py b/process_monitor_unix.py index 41a496cc..299c58ea 100644 --- a/process_monitor_unix.py +++ b/process_monitor_unix.py @@ -56,7 +56,10 @@ def __init__(self, start_command): """ self.start_command = start_command - self.tokens = start_command.split(' ') + if isinstance(start_command, basestring): + self.tokens = start_command.split(' ') + else: + self.tokens = start_command self.cmd_args = [] self.pid = None self.exit_status = None @@ -108,8 +111,8 @@ def __init__(self, host, port, cbin, level=1): self.dbg = None self.last_synopsis = None self.test_number = 0 - self.start_commands = None - self.stop_commands = None + self.start_commands = [] + self.stop_commands = [] self.proc_name = None self.log("Process Monitor PED-RPC server initialized:") self.log("Listening on %s:%s" % (host, port)) @@ -209,11 +212,14 @@ def stop_target(self): self.log("stopping target process") - for command in self.stop_commands: - if command == "TERMINATE_PID": - self.dbg.stop_target() - else: - os.system(command) + if len(self.stop_commands) < 1: + self.dbg.stop_target() + else: + for command in self.stop_commands: + if command == "TERMINATE_PID": + self.dbg.stop_target() + else: + os.system(command) def restart_target(self): """ From 214201700ec79af58b81676507109d28ea81a91e Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 22 Apr 2017 22:28:49 -0700 Subject: [PATCH 09/11] Make Target take procmon, procmon_options in constructor --- CHANGELOG.rst | 3 ++- boofuzz/sessions.py | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 631b18b5..fe8b5ab8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,9 +5,10 @@ Features - Console output - now with colors! - The process monitor (procmon) now tracks processes by PID rather than searching by name. Therefore, stop_commands and proc_name are no longer required. -- Added --help parameter to process monitor. +- Added `--help` parameter to process monitor. - SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py (usually). - Made Unix procmon more compatible with Windows. +- Target class now takes `procmon` and `procmon_options` in constructor. Fixes ----- diff --git a/boofuzz/sessions.py b/boofuzz/sessions.py index 7d91f8b6..91ea0f4a 100644 --- a/boofuzz/sessions.py +++ b/boofuzz/sessions.py @@ -36,7 +36,7 @@ class Target(object): tcp_target = Target(SocketConnection(host='127.0.0.1', port=17971)) """ - def __init__(self, connection): + def __init__(self, connection, procmon=None, procmon_options=None): """ @type connection: itarget_connection.ITargetConnection @param connection: Connection to system under test. @@ -44,13 +44,15 @@ def __init__(self, connection): self._fuzz_data_logger = None self._target_connection = connection + self.procmon = procmon # set these manually once target is instantiated. self.netmon = None - self.procmon = None self.vmcontrol = None self.netmon_options = {} - self.procmon_options = {} + if procmon_options is None: + procmon_options = {} + self.procmon_options = procmon_options self.vmcontrol_options = {} def close(self): From 27b3d733633056f9e37b81a50aea65a463e033f4 Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 29 Apr 2017 14:52:30 -0700 Subject: [PATCH 10/11] make procmon work when pydbg fails --- CHANGELOG.rst | 2 ++ boofuzz/__init__.py | 1 + process_monitor.py | 23 ++++++++++------------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fe8b5ab8..97f78401 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,6 +8,8 @@ Features - Added `--help` parameter to process monitor. - SIGINT (AKA Ctrl+C) now works to close both boofuzz and process_monitor.py (usually). - Made Unix procmon more compatible with Windows. +- Improved procmon debugger error handling, e.g., when running 64-bit apps. +- Windows procmon now runs even if pydbg fails. - Target class now takes `procmon` and `procmon_options` in constructor. Fixes diff --git a/boofuzz/__init__.py b/boofuzz/__init__.py index 4f8b9ecb..28b2de1a 100644 --- a/boofuzz/__init__.py +++ b/boofuzz/__init__.py @@ -173,6 +173,7 @@ def __exit__(self, type, value, traceback): return ScopedBlock(block) + def s_block_start(name, *args, **kwargs): """ Open a new block under the current request. This routine always returns an instance so you can make your fuzzer pretty diff --git a/process_monitor.py b/process_monitor.py index f845f86e..f91ba911 100644 --- a/process_monitor.py +++ b/process_monitor.py @@ -92,17 +92,14 @@ def run(self): if self.proc_name is not None or self.pid is not None: # watch for and try attaching to the process. - try: - if self.pid is None and self.proc_name is not None: - self.process_monitor.log( - "debugger thread-%s looking for process name: %s" % (self.getName(), self.proc_name)) - self.watch() - self.process_monitor.log("debugger thread-%s attaching to pid: %s" % (self.getName(), self.pid)) - self.dbg.attach(self.pid) - self.dbg.run() - self.process_monitor.log("debugger thread-%s exiting" % self.getName()) - except Exception: - pass + if self.pid is None and self.proc_name is not None: + self.process_monitor.log( + "debugger thread-%s looking for process name: %s" % (self.getName(), self.proc_name)) + self.watch() + self.process_monitor.log("debugger thread-%s attaching to pid: %s" % (self.getName(), self.pid)) + self.dbg.attach(self.pid) + self.dbg.run() + self.process_monitor.log("debugger thread-%s exiting" % self.getName()) # TODO: removing the following line appears to cause some concurrency issues. del self.dbg @@ -295,8 +292,8 @@ def start_target(self): @returns True if successful. """ - # if we don't already have a debugger thread, instantiate and start one now. - if not self.debugger_thread or not self.debugger_thread.isAlive(): + # if we don't already have a debugger thread or process, start one now. + if (not self.debugger_thread or not self.debugger_thread.isAlive()) and (self._process is None or self._process.poll() is not None): if len(self.start_commands) > 0: self.log("starting target process") From 234c29150d00891af9b10ebd6a255c81169775ed Mon Sep 17 00:00:00 2001 From: Joshua Pereyda Date: Sat, 29 Apr 2017 17:07:40 -0700 Subject: [PATCH 11/11] Improve ECONNABORTED error message --- boofuzz/sessions.py | 9 ++++++--- boofuzz/sex.py | 8 +++++++- boofuzz/socket_connection.py | 5 +++-- setup.py | 2 +- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/boofuzz/sessions.py b/boofuzz/sessions.py index 91ea0f4a..abaf407c 100644 --- a/boofuzz/sessions.py +++ b/boofuzz/sessions.py @@ -747,9 +747,12 @@ def transmit(self, sock, node, edge): self._fuzz_data_logger.log_pass("Some data received from target.") except sex.BoofuzzTargetConnectionReset: self._fuzz_data_logger.log_fail("Target connection reset.") - except sex.BoofuzzTargetConnectionAborted: - self._fuzz_data_logger.log_fail("Target connection lost: Software caused connection abort. You may have a" - " network issue, or an issue with firewalls or anti-virus.") + except sex.BoofuzzTargetConnectionAborted as e: + self._fuzz_data_logger.log_fail("Target connection lost (socket error: {0} {1}): You may have a network " + "issue, or an issue with firewalls or anti-virus. Try disabling your" + "firewall." + .format(e.socket_errno, e.socket_errmsg)) + pass def build_webapp_thread(self, port=26000): app.session = self diff --git a/boofuzz/sex.py b/boofuzz/sex.py index a2104ed7..0a5291e7 100644 --- a/boofuzz/sex.py +++ b/boofuzz/sex.py @@ -1,3 +1,4 @@ +import attr # Sulley EXception Class @@ -17,8 +18,13 @@ class BoofuzzTargetConnectionReset(BoofuzzError): pass +@attr.s class BoofuzzTargetConnectionAborted(BoofuzzError): - pass + """ + Raised on `errno.ECONNABORTED`. + """ + socket_errno = attr.ib() + socket_errmsg = attr.ib() class SullyRuntimeError(Exception): diff --git a/boofuzz/socket_connection.py b/boofuzz/socket_connection.py index 06915341..642d416f 100644 --- a/boofuzz/socket_connection.py +++ b/boofuzz/socket_connection.py @@ -170,7 +170,7 @@ def recv(self, max_bytes): data = bytes('') except socket.error as e: if e.errno == errno.ECONNABORTED: - raise_(sex.BoofuzzTargetConnectionAborted, None, sys.exc_info()[2]) + raise_(sex.BoofuzzTargetConnectionAborted(socket_errno=e.errno, socket_errmsg=e.strerror), None, sys.exc_info()[2]) elif (e.errno == errno.ECONNRESET) or \ (e.errno == errno.ENETRESET) or \ (e.errno == errno.ETIMEDOUT): @@ -214,7 +214,8 @@ def send(self, data): raise sex.SullyRuntimeError("INVALID PROTOCOL SPECIFIED: %s" % self.proto) except socket.error as e: if e.errno == errno.ECONNABORTED: - raise_(sex.BoofuzzTargetConnectionAborted, None, sys.exc_info()[2]) + raise_(sex.BoofuzzTargetConnectionAborted(socket_errno=e.errno, socket_errmsg=e.strerror), + None, sys.exc_info()[2]) elif (e.errno == errno.ECONNRESET) or \ (e.errno == errno.ENETRESET) or \ (e.errno == errno.ETIMEDOUT): diff --git a/setup.py b/setup.py index d1b3dd1c..8350442b 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ def find_version(*path_elements): package_data={'boofuzz': ['web/templates/*', 'web/static/css/*']}, install_requires=[ 'future', 'pyserial', 'pydot', 'tornado==4.0.2', - 'Flask==0.10.1', 'impacket', 'colorama', 'cement'], + 'Flask==0.10.1', 'impacket', 'colorama', 'cement', 'attrs'], extras_require={ # This list is duplicated in tox.ini. Make sure to change both! 'dev': ['check-manifest', 'mock', 'pytest', 'pytest-bdd', 'netifaces', 'ipaddress'],