francoispqt / gojay

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

Panic on malformed integers #27

Closed stevenjohnstone closed 6 years ago

stevenjohnstone commented 6 years ago

Ran go-fuzz with the following fuzzer (based on an example in the README):

package fuzz

import (
    "github.com/francoispqt/gojay"
)

type user struct {
    id    int
    name  string
    email string
}

// implement gojay.UnmarshalerJSONObject
func (u *user) UnmarshalJSONObject(dec *gojay.Decoder, key string) error {
    switch key {
    case "id":
        return dec.Int(&u.id)
    case "name":
        return dec.String(&u.name)
    case "email":
        return dec.String(&u.email)
    }
    return nil
}
func (u *user) NKeys() int {
    return 3
}

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

This is meant to reflect a deployment where the input JSON comes from an untrusted source.

After a few minutes, the fuzzer found issues like the following: this

{\"id\":0.E----

causes

panic: runtime error: index out of range

goroutine 1 [running]:
github.com/francoispqt/gojay.(*Decoder).atoi64(0xc42005a060, 0xc, 0xc, 0x0)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_number_int.go:673 +0x485
github.com/francoispqt/gojay.(*Decoder).getExponent(0xc42005a060, 0x0)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_number.go:105 +0x260
github.com/francoispqt/gojay.(*Decoder).getExponent(0xc42005a060, 0x0)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_number.go:91 +0x1ab
github.com/francoispqt/gojay.(*Decoder).getExponent(0xc42005a060, 0x568880)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_number.go:91 +0x1ab
github.com/francoispqt/gojay.(*Decoder).getExponent(0xc42005a060, 0x8)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_number.go:91 +0x1ab
github.com/francoispqt/gojay.(*Decoder).getInt64(0xc42005a060, 0x300000030, 0xc420000180, 0xc420047d80, 0x42f089)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_number_int.go:611 +0x618
github.com/francoispqt/gojay.(*Decoder).decodeInt(0xc42005a060, 0xc42007a750, 0x5, 0x0)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_number_int.go:24 +0x4d1
github.com/francoispqt/gojay.(*Decoder).Int(0xc42005a060, 0xc42007a750, 0x2, 0xa)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode.go:332 +0x51
github.com/francoispqt/gojay/fuzz.(*user).UnmarshalJSONObject(0xc42007a750, 0xc42005a060, 0xc4200140e2, 0x2, 0x0, 0x0)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/fuzz/fuzz.go:17 +0x1a2
github.com/francoispqt/gojay.(*Decoder).decodeObject(0xc42005a060, 0x4f54a0, 0xc42007a750, 0xc4200140e0, 0xc, 0xc)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode_object.go:58 +0x6df
github.com/francoispqt/gojay.UnmarshalJSONObject(0x7f54c5bc5000, 0xc, 0x200000, 0x4f54a0, 0xc42007a750, 0x0, 0x0)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/decode.go:43 +0x128
github.com/francoispqt/gojay/fuzz.Fuzz(0x7f54c5bc5000, 0xc, 0x200000, 0x4ac3e7)
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/fuzz/fuzz.go:31 +0x81
go-fuzz-dep.Main(0x4e9598)
    /tmp/go-fuzz-build108984608/goroot/src/go-fuzz-dep/main.go:49 +0xad
main.main()
    /tmp/go-fuzz-build108984608/gopath/src/github.com/francoispqt/gojay/fuzz/go.fuzz.main/main.go:10 +0x2d

The problem here seems to be Decoder's Int() method isn't robust against this input? I'd expect the method to fail with a error indicating that the input isn't a well-formed string representation of an integer.

francoispqt commented 6 years ago

Hi,

Didn't know about go-fuzz. It's a great tool. A fix will be issued today for the crashers found. Thanks a lot!

francoispqt commented 6 years ago

Fixed crashers found. Ran go-fuzz, it ran more than 42000000 times and didn't find any crasher. Merging the branch and closing the issue. Thank you!