-
Notifications
You must be signed in to change notification settings - Fork 131
Description
Context
This library makes a strong point on testing, and that can be seen with the 97%+ code coverage.
However, as I was looking for bugs in an unrelated app and while running some fuzz test, it encountered a panic, where the root cause was this library.
I would like to propose creating a fuzz harness to further improve the reliability of this library.
As far as I can see, the package is explicitly non thread safe, this is thus out of scope of this fuzzer.
In order to simplify the proposal and avoid putting more work on volunteers, I'm accompanying this proposal with a possible implementation of a fuzzer.
The proposed implementation can be found here: #207.
That proposed implementation makes a bunch of tradeoff in order to be "extensible" and try to lower the cost of maintenance for a slightly higher complexity (due to reflection).
In order to make this fuzzer work, I'd like to also open the discussion around 2 issues: use of go 1.18 and CI integration.
Go 1.18+ requirements
Go has native fuzz capabilities since go 1.18.
Your minimal Go version (from the go.mod) is set to 1.13, which makes it to old to accept go native fuzz standard library.
However, the CI version uses the "latest" Go version which at the time of writing is 1.24.
I believe one of the alternative would be to gate the use of the fuzz test behind a build flag
CI integration
I believe fuzz testing this, even for 30 seconds to 1 min, in CI, would be beneficial for future work on the library.
This would allow you and contributors to trust more that new published code will be reliable and not cause panic.
Fuzzing can be ran for an unlimited time frame, which, here, is probably unnecessary, so I would suggest (especially to keep your Github Action free tier) to run for a limited amount of time
The app has some complexity but is not a critical cryptography library :D.
I've not prepared any PR for this, but it should be a matter of go test -fuzz "$FuzzSelect^" -timeout 30sec.
Please note, the argument is a regexp, and you can only start a single fuzz. It'll run on all cores available by default.
Next steps
In addition, I've also reported 2 bugs, found with that fuzzer (in about a second of execution time). And a bug fix for one of them.
The first bug can be seen here #205 . And is a nil pointer dereference. (bugfix #204)
The second bug can be seen here #208. And is a stack overflow caused by an infinite loop.