From 62b8e63a0acc1e8ac31321fc25e6d6055fb1aecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Kapka?= Date: Fri, 6 Sep 2024 15:39:33 -0400 Subject: [PATCH] Compare with nil before invoking `IsNil()` on an interface (#14431) * Compare with nil before invoking `IsNil()` on an interface * changelog * review --- CHANGELOG.md | 1 + beacon-chain/blockchain/execution_engine.go | 2 +- beacon-chain/blockchain/pow_block.go | 2 +- beacon-chain/execution/engine_client.go | 2 +- beacon-chain/execution/payload_body.go | 2 +- .../rpc/prysm/v1alpha1/validator/proposer_bellatrix.go | 8 ++++---- beacon-chain/rpc/prysm/v1alpha1/validator/status.go | 7 ++----- beacon-chain/sync/validate_beacon_blocks.go | 2 +- consensus-types/blocks/getters.go | 2 +- testing/endtoend/evaluators/validator.go | 4 ++-- 10 files changed, 15 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d48ebdb7fca..c467c7d8fb4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - E2E: fixed gas limit at genesis - Light client support: use LightClientHeader instead of BeaconBlockHeader. - Core: Fix process effective balance update to safe copy validator for Electra. +- `== nil` checks before calling `IsNil()` on interfaces to prevent panics. ### Security diff --git a/beacon-chain/blockchain/execution_engine.go b/beacon-chain/blockchain/execution_engine.go index 8cfcb385fdb7..387bc552e9cc 100644 --- a/beacon-chain/blockchain/execution_engine.go +++ b/beacon-chain/blockchain/execution_engine.go @@ -39,7 +39,7 @@ func (s *Service) notifyForkchoiceUpdate(ctx context.Context, arg *fcuConfig) (* ctx, span := trace.StartSpan(ctx, "blockChain.notifyForkchoiceUpdate") defer span.End() - if arg.headBlock.IsNil() { + if arg.headBlock == nil || arg.headBlock.IsNil() { log.Error("Head block is nil") return nil, nil } diff --git a/beacon-chain/blockchain/pow_block.go b/beacon-chain/blockchain/pow_block.go index 00f929d3c19f..3018d31d413d 100644 --- a/beacon-chain/blockchain/pow_block.go +++ b/beacon-chain/blockchain/pow_block.go @@ -46,7 +46,7 @@ func (s *Service) validateMergeBlock(ctx context.Context, b interfaces.ReadOnlyS if err != nil { return err } - if payload.IsNil() { + if payload == nil || payload.IsNil() { return errors.New("nil execution payload") } ok, err := canUseValidatedTerminalBlockHash(b.Block().Slot(), payload) diff --git a/beacon-chain/execution/engine_client.go b/beacon-chain/execution/engine_client.go index 12fdd0393b98..ae069440f96d 100644 --- a/beacon-chain/execution/engine_client.go +++ b/beacon-chain/execution/engine_client.go @@ -526,7 +526,7 @@ func (s *Service) ReconstructFullBellatrixBlockBatch( func fullPayloadFromPayloadBody( header interfaces.ExecutionData, body *pb.ExecutionPayloadBody, bVersion int, ) (interfaces.ExecutionData, error) { - if header.IsNil() || body == nil { + if header == nil || header.IsNil() || body == nil { return nil, errors.New("execution block and header cannot be nil") } diff --git a/beacon-chain/execution/payload_body.go b/beacon-chain/execution/payload_body.go index a895044319f4..763cfd214501 100644 --- a/beacon-chain/execution/payload_body.go +++ b/beacon-chain/execution/payload_body.go @@ -67,7 +67,7 @@ func (r *blindedBlockReconstructor) addToBatch(b interfaces.ReadOnlySignedBeacon if err != nil { return err } - if header.IsNil() { + if header == nil || header.IsNil() { return errors.New("execution payload header in blinded block was nil") } r.orderedBlocks = append(r.orderedBlocks, &blockWithHeader{block: b, header: header}) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go index 23379cb2e000..8f8a95fea6ea 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go @@ -198,7 +198,7 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitiv if err != nil { return nil, err } - if signedBid.IsNil() { + if signedBid == nil || signedBid.IsNil() { return nil, errors.New("builder returned nil bid") } fork, err := forks.Fork(slots.ToEpoch(slot)) @@ -217,7 +217,7 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitiv if err != nil { return nil, errors.Wrap(err, "could not get bid") } - if bid.IsNil() { + if bid == nil || bid.IsNil() { return nil, errors.New("builder returned nil bid") } @@ -309,14 +309,14 @@ func validateBuilderSignature(signedBid builder.SignedBid) error { if err != nil { return err } - if signedBid.IsNil() { + if signedBid == nil || signedBid.IsNil() { return errors.New("nil builder bid") } bid, err := signedBid.Message() if err != nil { return errors.Wrap(err, "could not get bid") } - if bid.IsNil() { + if bid == nil || bid.IsNil() { return errors.New("builder returned nil bid") } return signing.VerifySigningRoot(bid, bid.Pubkey(), signedBid.Signature(), d) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/status.go b/beacon-chain/rpc/prysm/v1alpha1/validator/status.go index 67a5799d65c7..6382fac75bf6 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/status.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/status.go @@ -402,16 +402,13 @@ func statusForPubKey(headState state.ReadOnlyBeaconState, pubKey []byte) (ethpb. func assignmentStatus(beaconState state.ReadOnlyBeaconState, validatorIndex primitives.ValidatorIndex) ethpb.ValidatorStatus { validator, err := beaconState.ValidatorAtIndexReadOnly(validatorIndex) - if err != nil { + if err != nil || validator.IsNil() { return ethpb.ValidatorStatus_UNKNOWN_STATUS } + currentEpoch := time.CurrentEpoch(beaconState) farFutureEpoch := params.BeaconConfig().FarFutureEpoch validatorBalance := validator.EffectiveBalance() - - if validator.IsNil() { - return ethpb.ValidatorStatus_UNKNOWN_STATUS - } if currentEpoch < validator.ActivationEligibilityEpoch() { return depositStatus(validatorBalance) } diff --git a/beacon-chain/sync/validate_beacon_blocks.go b/beacon-chain/sync/validate_beacon_blocks.go index 302107cbeb52..c12407e926f4 100644 --- a/beacon-chain/sync/validate_beacon_blocks.go +++ b/beacon-chain/sync/validate_beacon_blocks.go @@ -345,7 +345,7 @@ func (s *Service) validateBellatrixBeaconBlock(ctx context.Context, parentState if err != nil { return err } - if payload.IsNil() { + if payload == nil || payload.IsNil() { return errors.New("execution payload is nil") } if payload.Timestamp() != uint64(t.Unix()) { diff --git a/consensus-types/blocks/getters.go b/consensus-types/blocks/getters.go index 410e53317d6b..eda1a420017d 100644 --- a/consensus-types/blocks/getters.go +++ b/consensus-types/blocks/getters.go @@ -276,7 +276,7 @@ func (b *SignedBeaconBlock) ToBlinded() (interfaces.ReadOnlySignedBeaconBlock, e } func (b *SignedBeaconBlock) Unblind(e interfaces.ExecutionData) error { - if e.IsNil() { + if e == nil || e.IsNil() { return errors.New("cannot unblind with nil execution data") } if !b.IsBlinded() { diff --git a/testing/endtoend/evaluators/validator.go b/testing/endtoend/evaluators/validator.go index 02137d366d7a..e4c5c20b2422 100644 --- a/testing/endtoend/evaluators/validator.go +++ b/testing/endtoend/evaluators/validator.go @@ -233,7 +233,7 @@ func validatorsSyncParticipation(_ *types.EvaluationContext, conns ...*grpc.Clie return errors.Wrapf(err, "block type doesn't exist for block at epoch %d", lowestBound) } - if b.IsNil() { + if b == nil || b.IsNil() { return errors.New("nil block provided") } forkStartSlot, err := slots.EpochStart(params.BeaconConfig().AltairForkEpoch) @@ -274,7 +274,7 @@ func validatorsSyncParticipation(_ *types.EvaluationContext, conns ...*grpc.Clie return errors.Wrapf(err, "block type doesn't exist for block at epoch %d", lowestBound) } - if b.IsNil() { + if b == nil || b.IsNil() { return errors.New("nil block provided") } forkSlot, err := slots.EpochStart(params.BeaconConfig().AltairForkEpoch)