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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug/fuzzing] "panic: runtime error: nil pointer dereference" when processing ProposerSlashing #6127

Closed
pventuzelo opened this issue Jun 4, 2020 · 2 comments 路 Fixed by #6218
Labels
Bug Fuzz Good First Issue Help Wanted

Comments

@pventuzelo
Copy link

@pventuzelo pventuzelo commented Jun 4, 2020

馃悶 Bug Report

Description

During fuzzing with beaconfuzz, I found the following bug:

what: panic: runtime error: invalid memory address or nil pointer dereference
where: in prysm
how: triggered during ProposerSlashing processing.

The bug is happening mainly in VerifyProposerSlashing function.

Here is some supposition:

proposer, err := beaconState.ValidatorAtIndexReadOnly(slashing.Header_1.Header.ProposerIndex)
if err != nil {
return err
}
if !helpers.IsSlashableValidatorUsingTrie(proposer, helpers.SlotToEpoch(beaconState.Slot())) {
return fmt.Errorf("validator with key %#x is not slashable", proposer.PublicKey())
}

  • line 426: proposer will be nil after the call to ValidatorAtIndexReadOnly
  • line 430: IsSlashableValidatorUsingTrie is called with proposer

func IsSlashableValidatorUsingTrie(val *stateTrie.ReadOnlyValidator, epoch uint64) bool {
return checkValidatorSlashable(val.ActivationEpoch(), val.WithdrawableEpoch(), val.Slashed(), epoch)

  • line 51: IsSlashableValidatorUsingTrie will execute val.Slashed() (val == proposer)

// Slashed returns the read only validator is slashed.
func (v *ReadOnlyValidator) Slashed() bool {
return v.validator.Slashed
}

  • line 78: Slashed try to dereference v.validator without having verify that v and v.validator are not nil

馃敟 Error

Download: panic_nil_deref_prysm_proposer.zip

./prysm_FuzzProposerSlashing2.libfuzzer panic_nil_deref_prysm_proposer.ssz
INFO: Seed: 1287398888
./prysm_FuzzProposerSlashing2.libfuzzer: Running 1 inputs 1 time(s) each.
Running: panic_nil_deref_prysm_proposer.ssz

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x38 pc=0x11d0360]

goroutine 17 [running, locked to thread]:
github.com/prysmaticlabs/prysm/beacon-chain/state.(*ReadOnlyValidator).Slashed(...)
	github.com/prysmaticlabs/prysm/beacon-chain/state/getters.go:78
github.com/prysmaticlabs/prysm/beacon-chain/core/helpers.IsSlashableValidatorUsingTrie(...)
	github.com/prysmaticlabs/prysm/beacon-chain/core/helpers/validators.go:51
github.com/prysmaticlabs/prysm/beacon-chain/core/blocks.VerifyProposerSlashing(0x10c000802f50, 0x10c0000edd50, 0x16, 0x10c000038fe0)
	github.com/prysmaticlabs/prysm/beacon-chain/core/blocks/block_operations.go:430 +0x400
github.com/prysmaticlabs/prysm/beacon-chain/core/blocks.ProcessProposerSlashings(0x18344c0, 0x10c000138030, 0x10c000802f50, 0x10c0000edd80, 0x0, 0x0, 0x0)
	github.com/prysmaticlabs/prysm/beacon-chain/core/blocks/block_operations.go:396 +0x102
github.com/prysmaticlabs/prysm.FuzzProposerSlashing(0x603000001900, 0x20, 0x20, 0x0)
	github.com/prysmaticlabs/prysm/fuzz.go:223 +0x1c5
main.LLVMFuzzerTestOneInput(...)
	command-line-arguments/main.132452089.go:21
main._cgoexpwrap_b2568cfea483_LLVMFuzzerTestOneInput(0x603000001900, 0x20, 0x5ed8ef0e)
	_cgo_gotypes.go:53 +0x56

馃敩 Minimal Reproduction

I can only reproduce the bug running my fuzzer with the previous crashing ssz file.
Maybe you will succeed to reproduce with your fuzzers as well.

Fuzzing function:

func FuzzProposerSlashing(b []byte) int {
	params.UseMainnetConfig()
	data := &ethpb.ProposerSlashing{}
	if err := data.UnmarshalSSZ(b); err != nil {
		return 0
	}
	// get a valid beaconstate
	st := getbeaconstate()
	s, err := stateTrie.InitializeFromProto(st)
	if err != nil {
		// should never happen
		panic("stateTrie InitializeFromProto")
	}
	// process the container
	post, err := blocks.ProcessProposerSlashings(context.Background(), s, &ethpb.BeaconBlockBody{ProposerSlashings: []*ethpb.ProposerSlashing{data}})
	if err != nil {
		return 0
	}
	if post == nil {
		return 0
	}
	return 1
}

I have nevertheless fix the bug (with the following patch) and the fuzzer is not crashing anymore.

Patch

A simple fix consist to check if v and v.validator are nil like in WithdrawableEpoch() and ExitEpoch() functions.

// Slashed returns the read only validator is slashed.
func (v *ReadOnlyValidator) Slashed() bool {
	if v == nil || v.validator == nil {
		return false
	}
	return v.validator.Slashed
}

instead of:

func (v *ReadOnlyValidator) Slashed() bool {
return v.validator.Slashed
}

馃實 Your Environment

Operating System:

OS: Ubuntu 18.04
Go: Go 1.14

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

master
commit: d152b48

@prestonvanloon prestonvanloon added Bug Fuzz Good First Issue Help Wanted labels Jun 4, 2020
@farazdagi
Copy link
Contributor

@farazdagi farazdagi commented Jun 10, 2020

@pventuzelo thank you for such a detailed report.
As you already have a patch, will you be willing to open a PR, resolving the issue?

@pventuzelo
Copy link
Author

@pventuzelo pventuzelo commented Jun 10, 2020

Hi @farazdagi,

Thanks for the proposal but I prefer to let you choose how to fix this since it is dependent of the business logic.

My fix was just for testing to verify that the nil pointer deref was coming from this function. It's not taking care if this function should actually return false in that case, or if you should verify v and v.validator in the caller function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fuzz Good First Issue Help Wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants