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

Fuzzing add seed, new target and remove bloat #592

Closed
wants to merge 2 commits into from

Conversation

0x34d
Copy link
Contributor

@0x34d 0x34d commented Oct 11, 2023

Description

Add more go-fuzz tests, add input seed and remove bloat.

Bug found: #591

Run:

go test -fuzz=FuzzUnmarshalBinary       $(pwd)
go test -fuzz=FuzzUnmarshal             $(pwd)/pkg/protocol/extension
go test -fuzz=FuzzDtlsHandshake         $(pwd)/pkg/protocol/handshake
go test -fuzz=FuzzRecordLayer           $(pwd)/pkg/protocol/recordlayer
go test -fuzz=FuzzUnpackDatagram        $(pwd)/pkg/protocol/recordlayer

@Sean-Der
Copy link
Member

Fantastic, thank you so much @0x34d

Can you fix the golangci-lint errors? If you don't feel comfortable doing it I can help!

@0x34d
Copy link
Contributor Author

0x34d commented Oct 11, 2023

Can you fix the golangci-lint errors? If you don't feel comfortable doing it I can help!

My bad I didn't run the golangci-lint, now you can try,

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2597464) 77.97% compared to head (7696273) 78.02%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #592      +/-   ##
==========================================
+ Coverage   77.97%   78.02%   +0.04%     
==========================================
  Files         101      101              
  Lines        6444     6444              
==========================================
+ Hits         5025     5028       +3     
+ Misses       1047     1045       -2     
+ Partials      372      371       -1     
Flag Coverage Δ
go 78.05% <ø> (+0.04%) ⬆️
wasm 63.58% <ø> (+0.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@0x34d 0x34d changed the title [Fuzzing] update and and Fuzzing add seed, new target and remove bloat Oct 12, 2023
@0x34d
Copy link
Contributor Author

0x34d commented Nov 5, 2023

@Sean-Der ping, can you rerun the workflow?

@Sean-Der
Copy link
Member

Sean-Der commented Nov 5, 2023

@0x34d you are in the org now so you can run yourself :)

sorry for slowing you down

@0x34d 0x34d requested review from hasheddan and stv0g November 6, 2023 03:50
@0x34d
Copy link
Contributor Author

0x34d commented Nov 11, 2023

ping for merge @hasheddan @stv0g @Sean-Der

@stv0g
Copy link
Member

stv0g commented Nov 11, 2023

I think it would be nicer to put the fuzz corpus into files. The fuzzer reada them automatically from testdata/TestName

@0x34d
Copy link
Contributor Author

0x34d commented Nov 11, 2023

I think it would be nicer to put the fuzz corpus into files. The fuzzer reada them automatically from testdata/TestName

Done 816e084 and leave others as they are.

)

func FuzzUnmarshalBinary(f *testing.F) {
TestResumeClient, err := os.ReadFile("testdata/seed/TestResumeClient.raw")
Copy link
Member

Choose a reason for hiding this comment

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

Please have a look at the Go fuzzing tutorial

The Go fuzzing enging has built-in support for reading files from the testdata directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, My bad https://go.dev/security/fuzz/#glos-seed-corpus

and the files in the testdata/fuzz/{FuzzTestName} directory within the package

Copy link
Contributor Author

@0x34d 0x34d Nov 14, 2023

Choose a reason for hiding this comment

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

Oh, My bad https://go.dev/security/fuzz/#glos-seed-corpus

and the files in the testdata/fuzz/{FuzzTestName} directory within the package

For a minute, I thought I was wrong.
Anything within testdata/fuzz/{FuzzTestName} is a crash and cannot be accepted as a seed for fuzzing. So, yes, my previous commit was on point.

Copy link
Member

Choose a reason for hiding this comment

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

Hey @0x34d, I am not so sure about it.

According to the Go fuzzing documentation, both f.Add() and the files in testdata are used to seed the corpus:

seed corpus: A user-provided corpus for a fuzz test which can be used to guide the fuzzing engine. It is composed of the corpus entries provided by f.Add calls within the fuzz test, and the files in the testdata/fuzz/{FuzzTestName} directory within the package. These entries are run by default with go test, whether fuzzing or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the Go fuzzing documentation, both f.Add() and the files in testdata are used to seed the corpus:

No, it's not. I already tried it. This was the whole reason for this comment #592 (comment)

testdata is for reproducing crashes.

Signed-off-by: Arjun Singh <ajsinghyadav00@gmail.com>
@0x34d
Copy link
Contributor Author

0x34d commented Feb 15, 2024

ping

@0x34d 0x34d closed this Apr 28, 2024
@Sean-Der
Copy link
Member

Sean-Der commented Nov 4, 2024

Merged with d796437

Thank you so much @0x34d sorry this didn't happen quicker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants