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

testing: sub fuzz targets #46780

Open
srikrsna opened this issue Jun 16, 2021 · 12 comments
Open

testing: sub fuzz targets #46780

srikrsna opened this issue Jun 16, 2021 · 12 comments
Labels
FeatureRequest fuzz Issues related to native fuzzing support NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@srikrsna
Copy link

It would be nice to be able to define sub fuzz targets like subtests and sub-benchmarks. Example:

for _, fc := range Cases {
    f.Run(fc.Name, func(f *testing.F) {
        f.Add(...)
        f.Fuzz(func(t *testing.T, data []byte) {
            t.Parallel()
            fz := fuzz.NewFromGoFuzz(data).Funcs(exfuzz.FuzzFuncs()...).Funcs(vanguardfz.FuzzFuncs()...)

            *r := fc.r
            fz.Fuzz(&r)

            u := fc.Permissions
            res, det, err := assert[tc.Method].Eval(map[string]interface{}{
                "r": r,
                "u": u,
            })
            // Assertions
        })
    })
}

This would be very much useful to fuzz functions with generic but defined data structures.

@cagedmantis cagedmantis added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest labels Jun 16, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Jun 16, 2021
@cagedmantis
Copy link
Contributor

/cc @golang/fuzzing

@jayconrod
Copy link
Contributor

It's not clear to me exactly what this would do or how it would be used. Could you elaborate?

This would be very much useful to fuzz functions with generic but defined data structures.

This is something we'd like to support eventually in any case. You wouldn't need a sub-target for it.

type Foo struct { ... }
func FuzzFoo(f *testing.F) {
  f.Fuzz(func(t *testing.T, foo Foo) {
    // check something with the Foo struct, without having to deal with []byte
  })
}

@katiehockman katiehockman added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 21, 2021
@rolandshoemaker
Copy link
Member

It's not clear to me exactly what this would do or how it would be used.

Agreed, this seems like it'd also only work when -fuzztime is specified, otherwise what is the behavior when the fuzzer is running indefinitely?

This would be very much useful to fuzz functions with generic but defined data structures.

When fuzzing a function that takes an interface{} (i.e. a JSON parser or something), a common approach is to test all data structures in the same fuzz target, rather than using a single target per structure, since they're likely to share significant logic.

func FuzzJSON(f *testing.F) {
	f.Fuzz(func(t *testing.T, b []byte) {
		for _, typ := range []interface{}{
			map[interface{}]interface{}{},
			[]interface{}{},
		} {
			json.Unmarshal(b, typ)
		}
	})
}

@srikrsna
Copy link
Author

The use case is similar to table driven tests. As an example, I want to Fuzz test my input validation logic in my gRPC service. In the current approach I will have to write a top level Fuzz target for each of the request methods.

In case of sub-targets, I would write it something like this,

func FuzzValidate(f *testing.F) {
   ff := []struct{
       Name string
       Seed proto.Message
       Request proto.Message
   }{
      {...},
      {...},
       ...
   }

   for _, fc := range ff {
    f.Run(fc.Name, func(f *testing.F) {
        // f.Parallel() to solve the fuzztime requirement
        f.Fuzz(func(t *testing.T, data []byte) {
            t.Parallel()
            fz := fuzz.NewFromGoFuzz(data)
            *r := fc.r
            fz.Fuzz(&r)

            r.Validate()            
        })
    })    
   }   
}

If we were to shift the loop inside, I wouldn't know which validation logic is failing.

@jayconrod
Copy link
Contributor

@srikrsna Thanks, I think that makes sense.

We'd need to nail down the exact semantics for this.

  • It should not be allowed to call F.Run and F.Fuzz on the same F, just as it's not allowed to call F.Fuzz more than once.
  • We'd probably want functions that call F.Run to finish before starting any function provided to F.Fuzz (which already runs in a goroutine). That way, we can ensure -test.fuzz matches exactly one target.

@srikrsna
Copy link
Author

My assumption is that the semantics would be similar to benchmarks, won't they?

@rolandshoemaker
Copy link
Member

If we were to shift the loop inside, I wouldn't know which validation logic is failing.

It seems like manual testing of the triggering input would give you this information, which is generally doing to be required anyway when a new crasher is found (additionally the trace should point you to where the problem is occurring).

@srikrsna
Copy link
Author

additionally the trace should point you to where the problem is occurring

The same applies for sub tests and sub benchmarks. Trace would useful to find out the exact line where the problem would be and how it can be fixed. Naming the targets will give an overview of which cases are failing.

@AlekSi

This comment has been minimized.

@AlekSi

This comment has been minimized.

@gopherbot gopherbot added the fuzz Issues related to native fuzzing support label Jul 26, 2021
@jayconrod
Copy link
Contributor

@AlekSi If the goal is to associate a name with each seed value for output, maybe open a new issue? As I understand it, this one is about having separate fuzz targets with their own fuzz separate functions, but perhaps with shared setup code.

@AlekSi
Copy link
Contributor

AlekSi commented Jul 27, 2021

Created #47413 for adding (*F) Run(string, func(t *T)) instead of (*F) Run(string, func(f *F)).

@rsc rsc changed the title [dev.fuzz] testing: sub fuzz targets testing: sub fuzz targets Sep 21, 2021
@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 12, 2022
@thanm thanm added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest fuzz Issues related to native fuzzing support NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Status: No status
Development

No branches or pull requests

9 participants