Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


For TinyInst instrumentation command line arguments, refer to [TinyInst readme](https://github.com/googleprojectzero/TinyInst).

Example (macOS):
Expand Down
12 changes: 10 additions & 2 deletions fuzzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ void Fuzzer::ParseOptions(int argc, char **argv) {
incremental_coverage = GetBinaryOption("-incremental_coverage", argc, argv, true);

add_all_inputs = GetBinaryOption("-add_all_inputs", argc, argv, false);

process_name = GetOption("-process_name", argc, argv);
ifratric marked this conversation as resolved.
Show resolved Hide resolved

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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");

}
}

void Fuzzer::SetupDirectories() {
Expand Down Expand Up @@ -245,8 +252,8 @@ RunResult Fuzzer::RunSampleAndGetCoverage(ThreadContext *tc, Sample *sample, Cov
FATAL("Repeatedly failed to deliver sample");
}
}

RunResult result = tc->instrumentation->Run(tc->target_argc, tc->target_argv, init_timeout, timeout);

tc->instrumentation->GetCoverage(*coverage, true);

// save crashes and hangs immediately when they are detected
Expand Down Expand Up @@ -325,8 +332,9 @@ RunResult Fuzzer::TryReproduceCrash(ThreadContext* tc, Sample* sample, uint32_t
FATAL("Repeatedly failed to deliver sample");
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some changes to spacing that should be reverted.

result = tc->instrumentation->RunWithCrashAnalysis(tc->target_argc, tc->target_argv, init_timeout, timeout);

tc->instrumentation->ClearCoverage();

if (result == CRASH) return result;
Expand Down
2 changes: 2 additions & 0 deletions fuzzer.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ class Fuzzer {
uint64_t num_samples_discarded;
uint64_t num_threads;
uint64_t total_execs;

char * process_name;

void SaveState(ThreadContext *tc);
void RestoreState(ThreadContext *tc);
Expand Down
2 changes: 1 addition & 1 deletion instrumentation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary changes to spacing

virtual void CleanTarget() = 0;

virtual bool HasNewCoverage() = 0;
Expand Down
20 changes: 18 additions & 2 deletions tinyinstinstrumentation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,19 @@ limitations under the License.

#include <sstream>


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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (
    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


attach_mode = false;
if(process_name != NULL) {
attach_mode = true;
}
}

RunResult TinyInstInstrumentation::Run(int argc, char **argv, uint32_t init_timeout, uint32_t timeout) {
Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sleep is a Windows function, see e.g.

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

processID = FindProcessId(process_name);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}

if (script != NULL) {
HANDLE thread_handle = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)system, script, 0, NULL);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

CreateThread(StartStatusThread, this);

CloseHandle(thread_handle);
}
status = instrumentation->Attach(processID, timeout1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} else {
status = instrumentation->Run(argc, argv, init_timeout);
}
}

if (status != DEBUGGER_TARGET_START) {
Expand Down
6 changes: 5 additions & 1 deletion tinyinstinstrumentation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary changes to spacing.

void CleanTarget() override;

bool HasNewCoverage() override;
Expand All @@ -47,6 +47,10 @@ class TinyInstInstrumentation : public Instrumentation {
protected:
LiteCov * instrumentation;
bool persist;
boolean attach_mode;
int processID;
char * process_name;
char * script;
int num_iterations;
int cur_iteration;
};
Expand Down