-
Notifications
You must be signed in to change notification settings - Fork 20
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 fuzzer for FileSystemTufStore #439
Conversation
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
This PR should be rebased after #440 has been merged before merging in. |
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
mts.loadTargets(); | ||
break; | ||
case 4: | ||
mts.storeTargetFile(string, byteArray); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must be a json file. Do we need to fuzz it with invalid UTF-8 sequences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have the json contents been verified elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use Gson, and it is fuzzed at https://github.com/google/oss-fuzz/tree/master/projects/gson
We parse trusted root with protobuf json parser.
We might consider using structure-aware fuzzing instead of raw json inputs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is, you say it "must be a json file". I am wondering what "must" means here. Has the string been checked for valid json format in Sigstore-java before it reaches storeTargetFile
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering what "must" means here
It means we use JSON parser when reading information from the file. In that case, the only valid file that would parse without errors would be something like a valid JSON.
Currently, we do not validate the byte sequence to be a valid JSON, however, I'm not sure that is important.
In any case, I think it would be much better to add JSON validation rather than spend (oss-fuzz) time on random-fuzzing of storeTargetFile.
PS. storeTargetFile
uses just java.nio.file.Files#write
, so I'm not sure fuzzing the method would add much coverage
closing for now, we can increase fuzzing coverage at another time. These have been open for too long. |
Summary
Add fuzzer for dev.sigstore.tuf.FileSystemTufStore class.
Release Note
Documentation