Fuzz test with `byte[]` parameter with mutation annotation is treated as classic fuzz test / annotations are ignored

[Marcono1234]

Version

jazzer-junit 0.28.0

Description

When a @FuzzTest takes only a single byte[] parameter but that parameter is annotated with mutation framework annotations (e.g. @WithLength), it is nonetheless treated as "classic" fuzz test and the annotations are ignored.

This can be quite irritating, especially if a user is unaware of the "classic" fuzz test mode, or when they change the signature of a mutation framework fuzz test to only have a byte[] parameter and suddenly the fuzz test turns into a "classic" one.

How to reproduce

Run this fuzz test in regression mode:

@FuzzTest
void test(byte @NotNull @WithLength(min = 1) [] b) {
    if (b.length < 1) throw new AssertionError();
}

:x: Problem: The test fails for the "<empty input>" run because the @WithLength annotation was ignored and the array is empty.

Expected behavior

If a fuzz test takes only a single byte[] parameter but that parameter is annotated (potentially nested, e.g. for arrays or parameterized types) with mutation framework annotations, then the fuzz test should be executed by the mutation framework.

Might also need some tweaks to the https://github.com/CodeIntelligenceTesting/jazzer/blob/main/docs/mutation-framework.md documentation.

[oetr]

So far we have avoided addressing this, because solving this properly will be a breaking change for most OSS-Fuzz fuzz tests.

Your proposal will not cause breaking change in OSS-Fuzz, but will have the following consequences:

• adding a @NotNull annotation suddenly also forces the byte array into the range of 0-1000 bytes--the default in the mutation framework
• adding only @WithLength suddenly allows the byte array to be null (in raw mode the byte array is never null).
• switching between both raw/mutation framework might invalidate the corpus and set back the coverage

I think, it's better to be consistent and always use mutation framework by default for any number of parameters. Users can still opt-out to raw fuzzing through a configuration option

This will 1) break most fuzz tests in OSS-Fuzz because in raw mode the byte array is never null, but in the mutation framework it can, unless annotated with @NotNull; 2) adding @NotNull as a countermeasure will invalidate all the corpus so far (because of write vs. writeExclusive that is used for the last param)

The mutation framework is currently already inconsistent:

• the most annoying for me personally: adding another parameter will switch to using mutation framework that will now force its limits on the first parameter (byte [] can suddenly be null and only have length between 0-1000 bytes)
• adding another parameter invalidates the corpus for the butlast parameter (mutation framework writes the last param with writeExclusive, and all others with write, which leads to this inconsistency).
• adding @NotNull annotation to the last parameter invalidates the corpus entries for it because of the same write /writeExclusive issue

To work around these issues, I use a dummy boolean as last parameter in most of my fuzz tests, such that adding annotations/additional parameters will not invalidate my corpus.

[fmeum]

You could add a compatibility mode and enable it globally in OSS-Fuzz via a flag in the Jazzer wrapper. But you would need to be careful and monitor Monorail 🙂

[Marcono1234]

Your proposal will not cause breaking change in OSS-Fuzz, but will have the following consequences: [...]

But it would be a deliberate decision then by the user, since they explicitly added mutation framework annotations.

---

adding a @NotNull annotation suddenly also forces the byte array into the range of 0-1000 bytes

There might not be any need for the user to do this though since (as you mention) the array is always non-null in classic mode. So adding @NotNull might only be relevant if the user wants to add other annotations as well.

---

switching between both raw/mutation framework might invalidate the corpus and set back the coverage

Isn't that a general problem when you change the signature of a fuzz test?

---

Though you have a good point regarding the remaining inconsistencies, and I guess you both have more insight into the internals and which solution might be best.

Maybe as first step the proposed solution of detecting the annotations could work though? And the other inconsistencies / larger issues could be solved afterwards?

[oetr]

Just looking at the two functions below, I would expect the fuzzer to give me values from the same domain for the two byte arrays. But surprisingly, their domains are completely different. Your proposal would leave this behavior the same.

@FuzzTest
public fuzzMe(byte[] data){}

@FuzzTest
public fuzzMe2(byte[] data,
               String format){}

And here, assuming we implement your proposal, the byte array of the first fuzz test cannot be null, but in the second it can. It is again very surprising.

@FuzzTest
public fuzzMe(byte[] data){}

@FuzzTest
public fuzzMe(byte @WithLength(max=10) [] data){}

Users have to learn these surprises the hard way. IMO the domain shouldn't depend on the number of parameters in the fuzz test, with and without annotations.

The deliberate decision to use raw byte[] fuzzing should be done by setting an already available option/env var/command line arg instead mutator_framework=false

[oetr]

add a compatibility mode and enable it globally in OSS-Fuzz via a flag in the Jazzer wrapper.

@fmeum How would this work for projects that already use the mutation framework?

[fmeum]

If they have fuzz tests with a classic byte[] signature they would be affected, but I'm hoping that these are easy enough to detect that you could set the compatibility flag for them and keep the current behavior for them.

The length handling for a classic signature vs any other is just inconsistent from the perspective of someone not familiar with libFuzzer-style targets. writeExclusive would still be useful for fuzz tests that just consume binary data, but if I remember correctly they could adopt @NotNull byte[] with an appropriately long max length to keep the same corpus representation below that max size.

I may very well be missing an unintended consequence though, it's been a long time.