Add Support for fuzzing by attaching to running processes on Windows. #38

parikhakshat · 2022-08-19T20:31:08Z

I have made some changes to Jackalope that allow someone to fuzz processes that are already running on the system by attaching to them. The current limitations of this new mode are that it only supports file delivery and single threads. The reason for only supporting one thread is that attaching multiple threads to the same process to fuzz results in unreliable. I also plan to add a recovery_script option to this mode which allows the user to specify a script that can be used to restart the running process to a fresh state. Currently, I have tested this mode on processes that have an automatic restart feature after each time they are killed, so I didn't need the recovery_script option. Please let me know if you have any questions or concerns about my commit.

To support this new mode, I needed to make some minor changes to TinyInst as well. Those changes are in a separate pull request on that repository.

google-cla · 2022-08-19T20:31:14Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

ifratric

Thanks for submitting! Please see the comments on the TinyInst PR first. Some general comments on the Jackalope part are below.

ifratric · 2022-08-24T08:55:17Z

tinyinstinstrumentation.cpp

@@ -136,6 +135,117 @@ RunResult TinyInstInstrumentation::RunWithCrashAnalysis(int argc, char** argv, u
  return ret;
 }

+RunResult TinyInstInstrumentation::Attach(char * script, char * process_name, uint32_t init_timeout, uint32_t timeout) {


This function duplicates a lot of code from TinyInstInstrumentation::Run. Would be better if this was implemented by changing TinyInstInstrumentation::Run where appropriate. Probably the same thing with TinyInstInstrumentation::AttachWithCrashAnalysis and TinyInstInstrumentation::RunWithCrashAnalysis.

fuzzer.cpp

parikhakshat · 2022-09-06T02:17:40Z

I have updated my commit with some of the code that used to be in the Tinyinst repository. I have tested the tool and it works pretty well with some of the targets. Let me know if there is anymore work needed to be done. Note: there is some redundancy of code involving process_name in both fuzzer.cpp and tinyinstrumentation.cpp. I wasn't able to move that code outside of fuzzer.cpp because I need that code to set the number of threads to one, which only happens in fuzzer.cpp. When attaching to processes and getting coverage, I've found that the coverage and stability of the target is only really stable when only one thread is attached to it.

ifratric

I apologize for the delay, I was out of office for a while due to conference travel and it slipped my mind. Comments below.

ifratric · 2022-09-19T08:15:56Z

README.md

@@ -127,6 +127,8 @@ The following command line arguments are supported:

 `-dict <path>` - Provides a dictionary to be used during mutation. The dictionary should be a text file with every entry on a separate line. `\xXX` escape sequences can be used.

+`-process-name <name of running process> - Enables fuzzing for processes that are already running in the operating system such as Windows services. The fuzzer will attach to the specified running process and get coverage from there. Then, the script specified after `--` will be opened in a new thread and will send the current testcase to running process. Note: this mode only allows for one thread and file delivery through the script. 


I'm not sure windows services is a good example here as afaik all windows services run under the same process name (svchost.exe)

ifratric · 2022-09-19T08:20:29Z

fuzzer.cpp

+
+  if (process_name != NULL) {
+    //set threads to one as multiple threads attaching to one process does not work well
+    num_threads = 1;


Perhaps if num_threads is not 1 already it would be a good idea to notify the user that Jackalope is changing the thread count, e.g.
WARN("Using a single fuzzing thread when process_name is set");

ifratric · 2022-09-19T08:21:00Z

fuzzer.cpp

@@ -325,8 +332,9 @@ RunResult Fuzzer::TryReproduceCrash(ThreadContext* tc, Sample* sample, uint32_t
        FATAL("Repeatedly failed to deliver sample");
      }
    }
-
+    


There are some changes to spacing that should be reverted.

ifratric · 2022-09-19T08:21:32Z

instrumentation.h

@@ -31,7 +31,7 @@ class Instrumentation {
  virtual RunResult RunWithCrashAnalysis(int argc, char** argv, uint32_t init_timeout, uint32_t timeout) {
    return Run(argc, argv, init_timeout, timeout);
  }
-
+  


Unnecessary changes to spacing

ifratric · 2022-09-19T08:37:22Z

tinyinstinstrumentation.cpp

 void TinyInstInstrumentation::Init(int argc, char **argv) {
  instrumentation = new LiteCov();
  instrumentation->Init(argc, argv);

  persist = GetBinaryOption("-persist", argc, argv, false);
  num_iterations = GetIntOption("-iterations", argc, argv, 1);
+  process_name = GetOption("-process_name", argc, argv);
+  script = ArgvToCmd(argc, argv);


Running a script is a part of sample delivery so I don't think it should go in this class. Instead, my proposal is to

Create a new class ExternalSampleDelivery that subclasses SampleDelivery (see sampledelivery.h and sampledelivery.cpp for existing examples)

In Fuzzer::CreateSampleDelivery (

Jackalope/fuzzer.cpp

Line 987 in 6134ff1

SampleDelivery *Fuzzer::CreateSampleDelivery(int argc, char **argv, ThreadContext *tc) {

) if the right option is set (e.g. -delivery external), create the ExternalSampleDelivery object and set its params. You can get the value of target argc/argv from tc->target_argc/tc->target_argv

In ExternalSampleDelivery::DeliverSample create a thread a run the script

ifratric · 2022-09-19T08:40:30Z

tinyinstinstrumentation.cpp

+        Sleep(init_timeout);
+        processID = FindProcessId(process_name);
+        if (script != NULL) {
+          HANDLE thread_handle = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)system, script, 0, NULL);


CreateThread here is a Windows function so it will only work on Windows. LPTHREAD_START_ROUTINE is also Windows specific. Jackalope has its own function for creating threads that works cross-platform, see https://github.com/googleprojectzero/Jackalope/blob/main/thread.h For an example of use see e.g.

Jackalope/server.cpp

Line 667 in 46c1c9b

CreateThread(StartStatusThread, this);

ifratric · 2022-09-19T08:40:51Z

tinyinstinstrumentation.h

@@ -32,7 +32,7 @@ class TinyInstInstrumentation : public Instrumentation {

  RunResult Run(int argc, char** argv, uint32_t init_timeout, uint32_t timeout) override;
  RunResult RunWithCrashAnalysis(int argc, char** argv, uint32_t init_timeout, uint32_t timeout) override;
-
+  


Unnecessary changes to spacing.

ifratric · 2022-09-19T08:55:09Z

tinyinstinstrumentation.cpp

+          HANDLE thread_handle = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)system, script, 0, NULL);
+          CloseHandle(thread_handle);
+        }
+        status = instrumentation->Attach(processID, timeout1);


Here in attach mode you call instrumentation->Attach instead of instrumentation->Run (good!), but there is another place above where instrumentation->Run is called that might behave incorrectly in attach mode. You should do the same thing there as well.

ifratric · 2022-09-19T08:59:35Z

tinyinstinstrumentation.cpp

@@ -68,7 +74,17 @@ RunResult TinyInstInstrumentation::Run(int argc, char **argv, uint32_t init_time
      WARN("Target function not reached, retrying with a clean process\n");
      instrumentation->Kill();
      cur_iteration = 0;
-      status = instrumentation->Run(argc, argv, init_timeout);
+      if (attach_mode) {
+        Sleep(init_timeout);


Sleep is a Windows function, see e.g.

Jackalope/fuzzer.cpp

Line 202 in 6134ff1

#if defined(WIN32) || defined(_WIN32) || defined(__WIN32)

ifratric · 2022-09-19T09:04:35Z

tinyinstinstrumentation.cpp

-      status = instrumentation->Run(argc, argv, init_timeout);
+      if (attach_mode) {
+        Sleep(init_timeout);
+        processID = FindProcessId(process_name);


There should be a check here if FindProcessId failed. Instead of calling Sleep first and then FindProcessId, maybe you can do something like

while(1) { processID = FindProcessId(process_name); if(processID) break; WARN('Could not find target process, retrying after timeout'); Sleep(init_timeout); }

parikhakshat added 2 commits August 19, 2022 13:13

Add initial support for fuzzing attached processes

ed85ce0

Update README.md

7351697

parikhakshat mentioned this pull request Aug 19, 2022

Add Support Functions for Fuzzing Attached Processes and Fix a False Hang issue in attached processes googleprojectzero/TinyInst#61

Open

ifratric reviewed Aug 24, 2022

View reviewed changes

Moved some of the Tinyinst Functionality to the Jackalope codebase

9c7ba20

ifratric reviewed Sep 19, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Support for fuzzing by attaching to running processes on Windows. #38

Add Support for fuzzing by attaching to running processes on Windows. #38

parikhakshat commented Aug 19, 2022 •

edited

Loading

google-cla bot commented Aug 19, 2022

ifratric left a comment

ifratric Aug 24, 2022

parikhakshat commented Sep 6, 2022

ifratric left a comment

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

ifratric Sep 19, 2022

		@@ -127,6 +127,8 @@ The following command line arguments are supported:

		`-dict <path>` - Provides a dictionary to be used during mutation. The dictionary should be a text file with every entry on a separate line. `\xXX` escape sequences can be used.

		`-process-name <name of running process> - Enables fuzzing for processes that are already running in the operating system such as Windows services. The fuzzer will attach to the specified running process and get coverage from there. Then, the script specified after `--` will be opened in a new thread and will send the current testcase to running process. Note: this mode only allows for one thread and file delivery through the script.

		@@ -32,7 +32,7 @@ class TinyInstInstrumentation : public Instrumentation {

		RunResult Run(int argc, char** argv, uint32_t init_timeout, uint32_t timeout) override;
		RunResult RunWithCrashAnalysis(int argc, char** argv, uint32_t init_timeout, uint32_t timeout) override;

Add Support for fuzzing by attaching to running processes on Windows. #38

Are you sure you want to change the base?

Add Support for fuzzing by attaching to running processes on Windows. #38

Conversation

parikhakshat commented Aug 19, 2022 • edited Loading

google-cla bot commented Aug 19, 2022

ifratric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parikhakshat commented Sep 6, 2022

ifratric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parikhakshat commented Aug 19, 2022 •

edited

Loading