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

Address panics in parseBinaryFloat, add fuzzer #41

Merged
merged 3 commits into from
Sep 14, 2023

Conversation

TomSellers
Copy link
Contributor

This PR addresses two indexing related panics in parseBinaryFloat. It also adds a fuzzer for DecodePacketErr to help identify similar issues in the future.

The fuzzer can be run using the following command:
go test -v -fuzz ^FuzzDecodePacket$ .

The following reproducer will trigger the panics. Uncomment the second data= block to trigger that one.

package main

import (
	ber "github.com/go-asn1-ber/asn1-ber"
)

func main() {
	// panic: runtime error: slice bounds out of range [:2] with capacity 1
	// parseBinaryFloat - [email protected]/real.go:98
	// https://github.com/go-asn1-ber/asn1-ber/blob/master/real.go#L98-L99
	// Root cause: failure to check length of 'v' before before indexing into it
	data := []byte{0x09, 0x02, 0x85, 0x30}

	// panic: runtime error: index out of range [0] with length 0
	// parseBinaryFloat - [email protected]/real.go:92
	// https://github.com/go-asn1-ber/asn1-ber/blob/3c992f0d6bf4482d85368da48d2a37fa6a5a120b/real.go#L92
	// Root cause: failure to check length of slice 'v' before indexing into it
	//data = []byte{0x09, 0x01, 0xcf}

	_ = ber.DecodePacket(data)
}

@TomSellers
Copy link
Contributor Author

I've updated the logic to add build tags so that the fuzz tests won't generate errors when being processed by older versions of Go.

@johnweldon johnweldon merged commit 04301b4 into go-asn1-ber:master Sep 14, 2023
9 checks passed
@johnweldon
Copy link
Contributor

Thank you @TomSellers

@TomSellers TomSellers deleted the bug/panic_parseBinaryFloat branch September 14, 2023 17:50
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.

2 participants