Implement untrusted RPCS for fuzzing and reproduction. by oliverchang · Pull Request #888 · google/clusterfuzz

oliverchang · 2019-08-26T07:15:37Z

For the new fuzzing pipeline (#483).

Write integration tests.
Simplify testcase pulling pushing for testcase minimization/cleansing.
Make file_host.rebase_to_worker_root idempotent.

For the new fuzzing pipeline (#483).

oliverchang · 2019-08-26T07:15:43Z

/gcbrun

oliverchang · 2019-08-26T08:44:01Z

/gcbrun

oliverchang · 2019-08-26T09:14:06Z

/gcbrun

oliverchang · 2019-08-26T12:27:08Z

/gcbrun

oliverchang · 2019-08-26T13:15:42Z

/gcbrun

Dor1s

LGTM!!

Dor1s · 2019-08-26T14:29:47Z

@@ -123,7 +123,17 @@ func (s *UntrustedRunnerServer) PruneCorpus(context.Context, *pb.PruneCorpusRequ
 }

 // ProcessTestcase is processes a testcase with a fuzzing engine (e.g. minimization).


nit (not your change): "is " is a typo

Dor1s · 2019-08-26T14:34:48Z

  optional bytes output = 4;
 }

+message RunEngineFuzzerRequest {


a random thought: would it be better to name all of these starting with "Engine", e.g. "EngineRunFuzzerRequest", IMO that way it still sounds good, but kinda easier to attribute to the in-process fuzzing pipeline

Yes, or how about just EngineRunRequest, EngineRunResponse ?
I don't see you use Fuzzer in any of the other fields like EngineCrash or stuff.

Renamed to EngineFuzz{Request,Response}. Want to make it explicit that this is for fuzzing.

Dor1s · 2019-08-26T14:39:13Z

+  build_dir = environment.get_value('BUILD_DIR')
+  target_path = engine_common.find_fuzzer_path(build_dir, target_name)
+  if not target_path:
+    raise TargetNotFoundError('Failed to find target ' + target_name)


Why not leave this logic in the constructor? Isn't it better to fail earlier?

We can't call this in the constructor, which lives on the host and therefore doesn't have access to the build.

Dor1s · 2019-08-26T14:44:02Z

+      raise ValueError('Unknown stat type for ' + key)
+
+    packed_value.Unpack(value)
+    unpacked_stats[key] = value.value


Are these two lines doing what they're expected to do? Doesn't look like a typical Python code to me, but I'm not familiar with that Unpack method :)

Yeah, it's protobuf-generated code. See e.g. protocolbuffers/protobuf#4420 (comment)

Dor1s · 2019-08-26T14:57:55Z

+  engine_impl = engine.get(request.engine)
+  result = testcase_manager.engine_reproduce(
+      engine_impl, request.target_name, request.testcase_path,
+      list(request.arguments), request.timeout)


nit: for some reason, I don't like the conversion to the list here, maybe do it inside engine_reproduce? That way it will work with both lists and whatever protobuf object there is, without requiring the caller to cast the type epxlicitly

Good point! Moved!

inferno-chromium · 2019-08-26T20:39:02Z

  optional bytes output = 4;
 }

+message RunEngineFuzzerRequest {


Yes, or how about just EngineRunRequest, EngineRunResponse ?
I don't see you use Fuzzer in any of the other fields like EngineCrash or stuff.

inferno-chromium · 2019-08-26T20:41:23Z


  @classmethod
  def setUpClass(cls):
+    cls.saved_env = os.environ.copy()


Why not test_helpers.patch_environ(self) and then remove code in tearDownClass

That only works for setUp, not setUpClass

inferno-chromium · 2019-08-26T20:46:47Z

+    build_manager.setup_build()
+    with self.assertRaises(testcase_manager.TargetNotFoundError):
+      testcase_manager.engine_reproduce(libfuzzer_engine.LibFuzzerEngine(),
+                                        'not_found', testcase_file_path, [], 30)


s/not_found/not_exist ?

oliverchang

/gcbrun

oliverchang · 2019-08-27T00:30:32Z

+  build_dir = environment.get_value('BUILD_DIR')
+  target_path = engine_common.find_fuzzer_path(build_dir, target_name)
+  if not target_path:
+    raise TargetNotFoundError('Failed to find target ' + target_name)


We can't call this in the constructor, which lives on the host and therefore doesn't have access to the build.

oliverchang · 2019-08-27T00:32:16Z

+      raise ValueError('Unknown stat type for ' + key)
+
+    packed_value.Unpack(value)
+    unpacked_stats[key] = value.value


Yeah, it's protobuf-generated code. See e.g. protocolbuffers/protobuf#4420 (comment)

oliverchang · 2019-08-27T00:32:24Z

+  engine_impl = engine.get(request.engine)
+  result = testcase_manager.engine_reproduce(
+      engine_impl, request.target_name, request.testcase_path,
+      list(request.arguments), request.timeout)


Good point! Moved!

oliverchang · 2019-08-27T00:34:03Z

+    build_manager.setup_build()
+    with self.assertRaises(testcase_manager.TargetNotFoundError):
+      testcase_manager.engine_reproduce(libfuzzer_engine.LibFuzzerEngine(),
+                                        'not_found', testcase_file_path, [], 30)


oliverchang · 2019-08-27T00:34:33Z


  @classmethod
  def setUpClass(cls):
+    cls.saved_env = os.environ.copy()


That only works for setUp, not setUpClass

oliverchang · 2019-08-27T00:38:29Z

  optional bytes output = 4;
 }

+message RunEngineFuzzerRequest {


Renamed to EngineFuzz{Request,Response}. Want to make it explicit that this is for fuzzing.

oliverchang · 2019-08-27T00:40:27Z

@@ -123,7 +123,17 @@ func (s *UntrustedRunnerServer) PruneCorpus(context.Context, *pb.PruneCorpusRequ
 }

 // ProcessTestcase is processes a testcase with a fuzzing engine (e.g. minimization).


oliverchang · 2019-08-27T00:42:38Z

/gcbrun

oliverchang · 2019-08-27T01:11:39Z

/gcbrun

WIP: implement untrusted RPCS for fuzzing and reproduction.

6b7b5ad

For the new fuzzing pipeline (#483).

googlebot added the cla: yes CLA signed. label Aug 26, 2019

fix go

e3301a5

fix exception

aca2389

oliverchang changed the title ~~WIP: implement untrusted RPCS for fuzzing and reproduction.~~ Implement untrusted RPCS for fuzzing and reproduction. Aug 26, 2019

oliverchang requested review from Dor1s and inferno-chromium August 26, 2019 09:14

fix lint

018651e

fix lint again

37f9229

Dor1s approved these changes Aug 26, 2019

View reviewed changes

inferno-chromium approved these changes Aug 26, 2019

View reviewed changes

oliverchang added 2 commits August 27, 2019 10:28

speculative fix for test

93998a6

address comments.

e4d5499

oliverchang commented Aug 27, 2019

View reviewed changes

fix go

920772f

oliverchang merged commit 4cb3cb3 into master Aug 27, 2019

oliverchang deleted the untrusted_rpc branch August 27, 2019 01:29

stfsdr6sgs-eng mentioned this pull request Nov 25, 2025

Deema Boutiqué #5052

Closed

Sovann87 mentioned this pull request Mar 16, 2026

Mrr #5208

Closed

		@@ -123,7 +123,17 @@ func (s UntrustedRunnerServer) PruneCorpus(context.Context, pb.PruneCorpusRequ
		}

		// ProcessTestcase is processes a testcase with a fuzzing engine (e.g. minimization).

Conversation

oliverchang commented Aug 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliverchang commented Aug 26, 2019

Uh oh!

oliverchang commented Aug 26, 2019

Uh oh!

oliverchang commented Aug 26, 2019

Uh oh!

oliverchang commented Aug 26, 2019

Uh oh!

oliverchang commented Aug 26, 2019

Uh oh!

Dor1s left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliverchang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliverchang commented Aug 27, 2019

Uh oh!

oliverchang commented Aug 27, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

oliverchang commented Aug 26, 2019 •

edited

Loading