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

[bug/fuzzing] "panic: runtime error: slice bounds out of range" when parsing SSZ #6083

Closed
pventuzelo opened this issue Jun 2, 2020 · 5 comments · Fixed by prysmaticlabs/go-ssz#112
Assignees
Labels
Bug Something isn't working Fuzz Anything fuzz testing related!

Comments

@pventuzelo
Copy link

pventuzelo commented Jun 2, 2020

🐞 Bug Report

Description

During fuzzing with beaconfuzz, I found the following bug:

  • what: a panic: runtime error: slice bounds out of range
  • where: in prysm go-ssz library
  • how: triggered during SSZ parsing with ssz.Unmarshal.

🔬 Minimal Reproduction

Install:

$ go get github.com/prysmaticlabs/ethereumapis/eth/v1alpha1
$ go get github.com/prysmaticlabs/go-ssz

Testing program with Attestation parsing:

package main
	
import (
    "fmt"
    "io/ioutil"
    "os"
)

import (
	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
	"github.com/prysmaticlabs/go-ssz"
)

func check(e error) {
    if e != nil {
        fmt.Println("Error:", e)
    }
}

func main() {

	file_name := os.Args[1]

    data, err := ioutil.ReadFile(file_name)
    check(err)
    fmt.Println("ReadFile OK: ", file_name)

	att := &ethpb.Attestation{}
	if err := ssz.Unmarshal(data, att); err != nil {
		fmt.Println("Failed to unmarshal: %v", err)
	}

	fmt.Println("OK")
}

Compilation:

$ go build test.go

🔥 Error

Download:
panic_slice_out_range_prysm.zip

Run:

./test panic_slice_out_range_prysm.ssz
ReadFile OK:  panic_slice_out_range_prysm.ssz
panic: runtime error: slice bounds out of range [228:227]

goroutine 1 [running]:
github.com/prysmaticlabs/go-ssz/types.(*structSSZ).Unmarshal(0x1160b70, 0xb29f80, 0xc0000893e0, 0x199, 0xc71c80, 0xb29f80, 0xc00042c000, 0xe3, 0x2e3, 0x0, ...)
	/XXX/test_prysm/src/github.com/prysmaticlabs/go-ssz/types/struct.go:243 +0x134f
github.com/prysmaticlabs/go-ssz.Unmarshal(0xc00042c000, 0xe3, 0x2e3, 0xb56160, 0xc0000893e0, 0x78, 0x0)
	/XXX/test_prysm/src/github.com/prysmaticlabs/go-ssz/ssz.go:106 +0x335
main.main()
	/XXX/test_prysm/debug_prysm_attestation.go:34 +0x17e

🌍 Your Environment

Operating System:

OS: Ubuntu 18.04

What version of Prysm are you running? (Which release)

master

@pventuzelo
Copy link
Author

Additional info (zcli):

$ zcli panic_slice_out_range_prysm.ssz
cannot load input
cannot decode ssz: expected object length is larger than given bytesLen

@pventuzelo pventuzelo changed the title [bug/fuzzing] "panic: runtime error: slice bounds out of range" when parsing Attestation [bug/fuzzing] "panic: runtime error: slice bounds out of range" when parsing SSZ Jun 2, 2020
@prestonvanloon prestonvanloon added Bug Something isn't working Fuzz Anything fuzz testing related! labels Jun 2, 2020
@prestonvanloon
Copy link
Member

We are in the process of deprecating go-ssz in favor of generated ssz methods.

Please prefer att.UnmarshalSSZ(data)

https://pkg.go.dev/github.com/prysmaticlabs/ethereumapis/eth/v1alpha1?tab=doc#Attestation.UnmarshalSSZ

We should fix this panic nonetheless. Thanks for reporting

@rauljordan
Copy link
Contributor

@prestonvanloon please triage

@prestonvanloon prestonvanloon self-assigned this Jun 11, 2020
rauljordan added a commit to prysmaticlabs/go-ssz that referenced this issue Jun 12, 2020
* Check struct offset range. Bug prysmaticlabs/prysm#6083

* add secondary check

* fix build

* Update types/struct.go

* Update types/struct.go

Co-authored-by: Raul Jordan <[email protected]>
@farazdagi
Copy link
Contributor

farazdagi commented Sep 10, 2020

The fix provided resolves only one branch of code (see line 244):

https://github.com/prysmaticlabs/go-ssz/blob/6d5c9aa213ae42b6b1b2f45d623b535e96922416/types/struct.go#L228-L253

and the one within the first condition's body (containing line 233 - where panic occurs), doesn't seem to check for the bounds.

To reproduce (reported by @pventuzelo):

package main


import (

    "fmt"
    
    "github.com/prysmaticlabs/prysm/shared/params"
    "github.com/prysmaticlabs/prysm/beacon-chain/p2p/encoder"
    testpb "github.com/prysmaticlabs/prysm/proto/testing"
)


func DecodeTestSimpleMessageCrash() {
    data := []byte("\x01\x00\x8f")
    params.UseMainnetConfig()
    input := &testpb.TestSimpleMessage{}
    e := encoder.SszNetworkEncoder{}
    if err := e.DecodeGossip(data, input); err != nil {
        _ = err
        return
    }
    return
}


func main() {
    fmt.Println("prysm: Crash reproducer")
    // change the following function to trigger different bugs
    DecodeTestSimpleMessageCrash()
}


// Run with:
// go run main.go

Error:

go run main.go                       
prysm: Crash reproducer
panic: runtime error: slice bounds out of range [:12] with capacity 1

goroutine 1 [running]:
github.com/prysmaticlabs/go-ssz/types.(*structSSZ).Unmarshal(0x8f4738, 0x6d4ae0, 0xc000138400, 0x199, 0x742cc0, 0x6d4ae0, 0xc00012aed3, 0x1, 0x1, 0x0, ...)
    /home/scop/Documents/consulting/sigmaprime/prysm/src/github.com/prysmaticlabs/go-ssz/types/struct.go:233 +0x14e7
github.com/prysmaticlabs/go-ssz.Unmarshal(0xc00012aed3, 0x1, 0x1, 0x6df500, 0xc000138400, 0x0, 0x1)
    /home/scop/Documents/consulting/sigmaprime/prysm/src/github.com/prysmaticlabs/go-ssz/ssz.go:115 +0x391
github.com/prysmaticlabs/prysm/beacon-chain/p2p/encoder.SszNetworkEncoder.doDecode(0xc00012aed3, 0x1, 0x1, 0x6df500, 0xc000138400, 0x3, 0xc00012aed3)
    /home/scop/Documents/consulting/sigmaprime/prysm/src/github.com/prysmaticlabs/prysm/beacon-chain/p2p/encoder/ssz.go:85 +0xd2
github.com/prysmaticlabs/prysm/beacon-chain/p2p/encoder.SszNetworkEncoder.DecodeGossip(0xc00012aed0, 0x3, 0x3, 0x6df500, 0xc000138400, 0x18, 0xc000116dd0)
    /home/scop/Documents/consulting/sigmaprime/prysm/src/github.com/prysmaticlabs/prysm/beacon-chain/p2p/encoder/ssz.go:98 +0xff
main.DecodeTestSimpleMessageCrash()
    /home/scop/Documents/consulting/sigmaprime/prysm/crashrepro/main.go:19 +0xa5
main.main()
    /home/scop/Documents/consulting/sigmaprime/prysm/crashrepro/main.go:30 +0x7a
exit status 2

❗ Even if we do not fix this issue in go-ssz repo (due to it being deprecated), we still can benefit from a regression test in /beacon-chain/p2p/encoder/ssz_test.go:

func TestSszNetworkEncoder_EmptyMessage(t *testing.T) {
	data := []byte("\x01\x00\x8f")
	params.UseMainnetConfig()
	input := &testpb.TestSimpleMessage{}
	e := encoder.SszNetworkEncoder{}
	assert.NoError(t, e.DecodeGossip(data, input))
}

@farazdagi farazdagi reopened this Sep 10, 2020
@rauljordan
Copy link
Contributor

No longer using go-ssz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Fuzz Anything fuzz testing related!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants