Migrate to Go 1.18+ native fuzzing, fix issues found by fuzzing#34
Open
tklauser wants to merge 3 commits into
Open
Migrate to Go 1.18+ native fuzzing, fix issues found by fuzzing#34tklauser wants to merge 3 commits into
tklauser wants to merge 3 commits into
Conversation
tklauser
changed the title
Use Go native fuzzing instead of gofuzz. This requires bumping the Go version in go.mod to 1.18. This is anyway needed since this modules uses the net/netip package which was introduced in Go 1.18[^1] since mdlayher#27. [^1]: https://go.dev/doc/go1.18#netip
tklauser
force-pushed
the
go-native-fuzzing
branch
2 times, most recently
from
09fb48f to
185c07f
Compare
In (*Packet).MarshalBinary, the buffer size is currently computed as:
make([]byte, 2+2+1+1+2+(p.IPLength*2)+(p.HardwareAddrLength*2))
p.IPLength and p.HardwareAddrLength are uint8. In the expression
calculating the buffer length, the untyped constant 2 takes the type of
the other operand, so both multiplications are evaluated as uint8
arithmetic. Any value ≥ 128 thus wraps:
uint8(128) * 2 == 0
uint8(200) * 2 == 144
Thus, the resulting buffer is too small and the subsequent copy calls
index out of bounds and panic.
The fix is to cast to int before multiplying:
make([]byte, 8+int(p.IPLength)*2+int(p.HardwareAddrLength)*2)
Also return the ErrInvalidIP sentinel error on invalid IP as done
elsewhere in the package.
MarshalBinary calls p.SenderIP.As4() and p.TargetIP.As4() unconditionally. netip.Addr.As4 panics if the address for some reason is not an IPv4 address. The fix replaces As4 with As16 (which is safe for any address) and slices the trailing pl bytes.
tklauser
force-pushed
the
go-native-fuzzing
branch
from
185c07f to
3e905e8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first commit changes the module to use Go native fuzzing instead of gofuzz. This requires bumping the Go version in
go.modto 1.18. This is anyway needed since this modules uses the net/netip package which was introduced in Go 1.181 since #27.The second and third commit fix two issues in
(*Packet).MarshalBinaryfound by the fuzzing test.See commit messages for details.
Footnotes
https://go.dev/doc/go1.18#netip ↩