attestantio / go-eth2-client

Apache License 2.0
102 stars 59 forks source link

`(g *Gwei) UnmarshalJSON()` `panic` on malformed input #122

Closed infosecual closed 3 months ago

infosecual commented 3 months ago

I am currently running Nosy on a broad set of ethereum client dependencies. It found a few panics while fuzzing go-eth2-client. This fixes an issues in (g *Gwei) UnmarshalJSON() where a panic: runtime error: slice bounds out of range is triggerable due to a malformed JSON token.

Summary:

(g *Gwei) UnmarshalJSON() has a few checks upon entry to prevent issues but a panic can still be triggered if a single " character is provided.

    if len(input) == 0 {
        return errors.New("input missing")
    }
    if !bytes.HasPrefix(input, []byte{'"'}) {
        return errors.New("invalid prefix")
    }
    if !bytes.HasSuffix(input, []byte{'"'}) {
        return errors.New("invalid suffix")
    }

This bypasses the len(input) == 0 check as well as the prefix and suffix checks, since the single " character matches.

The panic happens on the slice indexing shortly after:

    val, err := strconv.ParseUint(string(input[1:len(input)-1]), 10, 64)

This PR adds a minimum length check for a valid entry to (g *Gwei) UnmarshalJSON() as well as a test for the malformed input.

infosecual commented 3 months ago

For reference- here is the panic output with out the fix:

/usr/local/go/bin/go test -timeout 30s -run ^TestGweiUnmarshalJSON$ github.com/attestantio/go-eth2-client/spec/phase0

--- FAIL: TestGweiUnmarshalJSON (0.00s)
    --- FAIL: TestGweiUnmarshalJSON/Invalid_input (0.00s)
panic: runtime error: slice bounds out of range [1:0] [recovered]
    panic: runtime error: slice bounds out of range [1:0]

goroutine 9 [running]:
testing.tRunner.func1.2({0x6a43a0, 0xc00001e390})
    /usr/local/go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
    /usr/local/go/src/testing/testing.go:1634 +0x377
panic({0x6a43a0?, 0xc00001e390?})
    /usr/local/go/src/runtime/panic.go:770 +0x132
github.com/attestantio/go-eth2-client/spec/phase0.(*Gwei).UnmarshalJSON(0xc000072730, {0xc0000151d8, 0x1, 0x1})
    /home/user/repos/infosecual-go-eth2-client/spec/phase0/gwei.go:40 +0x2e5
github.com/attestantio/go-eth2-client/spec/phase0_test.TestGweiUnmarshalJSON.func1(0xc0000d1ba0)
    /home/user/repos/infosecual-go-eth2-client/spec/phase0/gwei_test.go:56 +0x3f
testing.tRunner(0xc0000d1ba0, 0xc00002ac10)
    /usr/local/go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 6
    /usr/local/go/src/testing/testing.go:1742 +0x390
FAIL    github.com/attestantio/go-eth2-client/spec/phase0   0.004s
FAIL