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

Panic on malformed floats #32

Closed
stevenjohnstone opened this issue May 22, 2018 · 3 comments
Closed

Panic on malformed floats #32

stevenjohnstone opened this issue May 22, 2018 · 3 comments

Comments

@stevenjohnstone
Copy link

Did some fuzzing on ca0442d with:

package fuzz

import (
	"errors"

	"github.com/francoispqt/gojay"
)

type jsonFloat float32

var badKey = errors.New("bad key")

func (f *jsonFloat) UnmarshalJSONObject(dec *gojay.Decoder, key string) error {
	if key == "n" {
		return dec.Float32((*float32)(f))
	}
	return badKey
}
func (f *jsonFloat) NKeys() int {
	return 1
}

func Fuzz(input []byte) int {
	var f jsonFloat
	err := gojay.UnmarshalJSONObject(input, &f)
	if err != nil {
		return 0
	}
	return 1
}

Found cases like

{"n":-5e-80

caused panics in float parsing

panic: runtime error: index out of range

goroutine 1 [running]:
github.com/francoispqt/gojay.(*Decoder).getFloat32(0xc42005a060, 0x200000003, 0xc420000180, 0xc420020000)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_number_float.go:266 +0xb06
github.com/francoispqt/gojay.(*Decoder).getFloat32Negative(0xc42005a060, 0x40d989, 0x4, 0xc420047ed0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_number_float.go:194 +0xa9
github.com/francoispqt/gojay.(*Decoder).decodeFloat32(0xc42005a060, 0xc420014108, 0x4, 0x0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_number_float.go:164 +0x1e9
github.com/francoispqt/gojay.(*Decoder).Float32(0xc42005a060, 0xc420014108, 0x1, 0x9)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode.go:442 +0x51
github.com/francoispqt/gojay/fuzz.(*jsonFloat).UnmarshalJSONObject(0xc420014108, 0xc42005a060, 0xc420014112, 0x1, 0x0, 0x0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/fuzz/fuzz.go:15 +0xd2
github.com/francoispqt/gojay.(*Decoder).decodeObject(0xc42005a060, 0x4f5460, 0xc420014108, 0xc420014110, 0xb, 0xb)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_object.go:58 +0x6df
github.com/francoispqt/gojay.UnmarshalJSONObject(0x7fd3e1e19000, 0xb, 0x200000, 0x4f5460, 0xc420014108, 0x0, 0x0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode.go:43 +0x128
github.com/francoispqt/gojay/fuzz.Fuzz(0x7fd3e1e19000, 0xb, 0x200000, 0x3)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/fuzz/fuzz.go:25 +0x81
go-fuzz-dep.Main(0x4e9588)
	/tmp/go-fuzz-build454068747/goroot/src/go-fuzz-dep/main.go:49 +0xad
main.main()
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/fuzz/go.fuzz.main/main.go:10 +0x2d

Looks like the check https://github.com/francoispqt/gojay/blob/master/decode_number_float.go#L261 needs to use the magnitude of exp to handle the negative case?

Also, the following input

{"n":0.

gave

panic: runtime error: index out of range

goroutine 1 [running]:
github.com/francoispqt/gojay.(*Decoder).atoi32(0xc42005a060, 0x7, 0x5, 0xc400000000)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_number_int.go:812 +0x440
github.com/francoispqt/gojay.(*Decoder).getFloat32(0xc42005a060, 0x40d989, 0x4, 0xc420047ed0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_number_float.go:248 +0x8d1
github.com/francoispqt/gojay.(*Decoder).decodeFloat32(0xc42005a060, 0xc4200b07fc, 0x4, 0x0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_number_float.go:156 +0x359
github.com/francoispqt/gojay.(*Decoder).Float32(0xc42005a060, 0xc4200b07fc, 0x1, 0x5)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode.go:442 +0x51
github.com/francoispqt/gojay/fuzz.(*jsonFloat).UnmarshalJSONObject(0xc4200b07fc, 0xc42005a060, 0xc4200b0802, 0x1, 0x0, 0x0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/fuzz/fuzz.go:15 +0xd2
github.com/francoispqt/gojay.(*Decoder).decodeObject(0xc42005a060, 0x4f5460, 0xc4200b07fc, 0xc4200b0800, 0x7, 0x7)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode_object.go:58 +0x6df
github.com/francoispqt/gojay.UnmarshalJSONObject(0x7fc76d580000, 0x7, 0x200000, 0x4f5460, 0xc4200b07fc, 0x0, 0x0)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/decode.go:43 +0x128
github.com/francoispqt/gojay/fuzz.Fuzz(0x7fc76d580000, 0x7, 0x200000, 0x3)
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/fuzz/fuzz.go:25 +0x81
go-fuzz-dep.Main(0x4e9588)
	/tmp/go-fuzz-build454068747/goroot/src/go-fuzz-dep/main.go:49 +0xad
main.main()
	/tmp/go-fuzz-build454068747/gopath/src/github.com/francoispqt/gojay/fuzz/go.fuzz.main/main.go:10 +0x2d

@francoispqt
Copy link
Owner

Hi thanks again for raising issue. Will be fixed today and run more tests with go-fuzz.

@francoispqt
Copy link
Owner

I opened a pull request: #33
Crashers for floats have been fixed. Have found new potential ones for ints which I have fixed. Will run more tests before merging. Don't want to release a patch before making sure I've ran enough tests. Thanks!

@francoispqt
Copy link
Owner

Pull request is merged. Closing the issue. Let me know if you find any other, thank you!

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

No branches or pull requests

2 participants