francoispqt / gojay

high performance JSON encoder/decoder with stream API for Golang
MIT License
2.11k stars 112 forks source link

Panic on malformed floats #32

Closed stevenjohnstone closed 6 years ago

stevenjohnstone commented 6 years ago

Did some fuzzing on ca0442d6e33334128a2ee68f24c6c962de72ead3 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 commented 6 years ago

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

francoispqt commented 6 years ago

I opened a pull request: https://github.com/francoispqt/gojay/pull/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 commented 6 years ago

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