nil checks on ToConsensus() functions (#14867)

* adding more safety checks and associated tests * changelog * Update api/server/structs/conversions.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * radek's feedback * fixing tests * gaz * Update api/server/structs/conversions.go * Update api/server/structs/conversions.go * Update api/server/structs/conversions.go * Update api/server/structs/conversions.go --------- Co-authored-by: Radosław Kapka <rkapka@wp.pl>
2026-01-09 15:37:56 -05:00 · 2025-02-05 08:59:57 -06:00
parent e331d5b371
commit 3dc00816fb
3 changed files with 213 additions and 20 deletions
--- a/api/server/structs/conversions.go
+++ b/api/server/structs/conversions.go
@@ -52,6 +52,9 @@ func HistoricalSummaryFromConsensus(s *eth.HistoricalSummary) *HistoricalSummary
 }

 func (s *SignedBLSToExecutionChange) ToConsensus() (*eth.SignedBLSToExecutionChange, error) {
+	if s.Message == nil {
+		return nil, server.NewDecodeError(errNilValue, "Message")
+	}
 	change, err := s.Message.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Message")
@@ -103,14 +106,17 @@ func SignedBLSChangeFromConsensus(ch *eth.SignedBLSToExecutionChange) *SignedBLS

 func SignedBLSChangesToConsensus(src []*SignedBLSToExecutionChange) ([]*eth.SignedBLSToExecutionChange, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "SignedBLSToExecutionChanges")
 	}
 	err := slice.VerifyMaxLength(src, 16)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "SignedBLSToExecutionChanges")
 	}
 	changes := make([]*eth.SignedBLSToExecutionChange, len(src))
 	for i, ch := range src {
+		if ch == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d]", i))
+		}
 		changes[i], err = ch.ToConsensus()
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d]", i))
@@ -156,6 +162,9 @@ func ForkFromConsensus(f *eth.Fork) *Fork {
 }

 func (s *SignedValidatorRegistration) ToConsensus() (*eth.SignedValidatorRegistrationV1, error) {
+	if s.Message == nil {
+		return nil, server.NewDecodeError(errNilValue, "Message")
+	}
 	msg, err := s.Message.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Message")
@@ -212,6 +221,9 @@ func SignedValidatorRegistrationFromConsensus(vr *eth.SignedValidatorRegistratio
 }

 func (s *SignedContributionAndProof) ToConsensus() (*eth.SignedContributionAndProof, error) {
+	if s.Message == nil {
+		return nil, server.NewDecodeError(errNilValue, "Message")
+	}
 	msg, err := s.Message.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Message")
@@ -236,6 +248,9 @@ func SignedContributionAndProofFromConsensus(c *eth.SignedContributionAndProof)
 }

 func (c *ContributionAndProof) ToConsensus() (*eth.ContributionAndProof, error) {
+	if c.Contribution == nil {
+		return nil, server.NewDecodeError(errNilValue, "Contribution")
+	}
 	contribution, err := c.Contribution.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Contribution")
@@ -307,6 +322,9 @@ func SyncCommitteeContributionFromConsensus(c *eth.SyncCommitteeContribution) *S
 }

 func (s *SignedAggregateAttestationAndProof) ToConsensus() (*eth.SignedAggregateAttestationAndProof, error) {
+	if s.Message == nil {
+		return nil, server.NewDecodeError(errNilValue, "Message")
+	}
 	msg, err := s.Message.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Message")
@@ -327,6 +345,9 @@ func (a *AggregateAttestationAndProof) ToConsensus() (*eth.AggregateAttestationA
 	if err != nil {
 		return nil, server.NewDecodeError(err, "AggregatorIndex")
 	}
+	if a.Aggregate == nil {
+		return nil, server.NewDecodeError(errNilValue, "Aggregate")
+	}
 	agg, err := a.Aggregate.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Aggregate")
@@ -343,6 +364,9 @@ func (a *AggregateAttestationAndProof) ToConsensus() (*eth.AggregateAttestationA
 }

 func (s *SignedAggregateAttestationAndProofElectra) ToConsensus() (*eth.SignedAggregateAttestationAndProofElectra, error) {
+	if s.Message == nil {
+		return nil, server.NewDecodeError(errNilValue, "Message")
+	}
 	msg, err := s.Message.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Message")
@@ -363,6 +387,9 @@ func (a *AggregateAttestationAndProofElectra) ToConsensus() (*eth.AggregateAttes
 	if err != nil {
 		return nil, server.NewDecodeError(err, "AggregatorIndex")
 	}
+	if a.Aggregate == nil {
+		return nil, server.NewDecodeError(errNilValue, "Aggregate")
+	}
 	agg, err := a.Aggregate.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Aggregate")
@@ -383,6 +410,9 @@ func (a *Attestation) ToConsensus() (*eth.Attestation, error) {
 	if err != nil {
 		return nil, server.NewDecodeError(err, "AggregationBits")
 	}
+	if a.Data == nil {
+		return nil, server.NewDecodeError(errNilValue, "Data")
+	}
 	data, err := a.Data.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Data")
@@ -412,6 +442,9 @@ func (a *AttestationElectra) ToConsensus() (*eth.AttestationElectra, error) {
 	if err != nil {
 		return nil, server.NewDecodeError(err, "AggregationBits")
 	}
+	if a.Data == nil {
+		return nil, server.NewDecodeError(errNilValue, "Data")
+	}
 	data, err := a.Data.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Data")
@@ -451,6 +484,9 @@ func (a *SingleAttestation) ToConsensus() (*eth.SingleAttestation, error) {
 	if err != nil {
 		return nil, server.NewDecodeError(err, "AttesterIndex")
 	}
+	if a.Data == nil {
+		return nil, server.NewDecodeError(errNilValue, "Data")
+	}
 	data, err := a.Data.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Data")
@@ -490,10 +526,16 @@ func (a *AttestationData) ToConsensus() (*eth.AttestationData, error) {
 	if err != nil {
 		return nil, server.NewDecodeError(err, "BeaconBlockRoot")
 	}
+	if a.Source == nil {
+		return nil, server.NewDecodeError(errNilValue, "Source")
+	}
 	source, err := a.Source.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Source")
 	}
+	if a.Target == nil {
+		return nil, server.NewDecodeError(errNilValue, "Target")
+	}
 	target, err := a.Target.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Target")
@@ -593,15 +635,17 @@ func (b *BeaconCommitteeSubscription) ToConsensus() (*validator.BeaconCommitteeS
 }

 func (e *SignedVoluntaryExit) ToConsensus() (*eth.SignedVoluntaryExit, error) {
-	sig, err := bytesutil.DecodeHexWithLength(e.Signature, fieldparams.BLSSignatureLength)
-	if err != nil {
-		return nil, server.NewDecodeError(err, "Signature")
+	if e.Message == nil {
+		return nil, server.NewDecodeError(errNilValue, "Message")
 	}
 	exit, err := e.Message.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Message")
 	}
-
+	sig, err := bytesutil.DecodeHexWithLength(e.Signature, fieldparams.BLSSignatureLength)
+	if err != nil {
+		return nil, server.NewDecodeError(err, "Signature")
+	}
 	return &eth.SignedVoluntaryExit{
 		Exit:      exit,
 		Signature: sig,
@@ -704,10 +748,16 @@ func Eth1DataFromConsensus(e1d *eth.Eth1Data) *Eth1Data {
 }

 func (s *ProposerSlashing) ToConsensus() (*eth.ProposerSlashing, error) {
+	if s.SignedHeader1 == nil {
+		return nil, server.NewDecodeError(errNilValue, "SignedHeader1")
+	}
 	h1, err := s.SignedHeader1.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "SignedHeader1")
 	}
+	if s.SignedHeader2 == nil {
+		return nil, server.NewDecodeError(errNilValue, "SignedHeader2")
+	}
 	h2, err := s.SignedHeader2.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "SignedHeader2")
@@ -720,10 +770,16 @@ func (s *ProposerSlashing) ToConsensus() (*eth.ProposerSlashing, error) {
 }

 func (s *AttesterSlashing) ToConsensus() (*eth.AttesterSlashing, error) {
+	if s.Attestation1 == nil {
+		return nil, server.NewDecodeError(errNilValue, "Attestation1")
+	}
 	att1, err := s.Attestation1.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Attestation1")
 	}
+	if s.Attestation2 == nil {
+		return nil, server.NewDecodeError(errNilValue, "Attestation2")
+	}
 	att2, err := s.Attestation2.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Attestation2")
@@ -732,10 +788,16 @@ func (s *AttesterSlashing) ToConsensus() (*eth.AttesterSlashing, error) {
 }

 func (s *AttesterSlashingElectra) ToConsensus() (*eth.AttesterSlashingElectra, error) {
+	if s.Attestation1 == nil {
+		return nil, server.NewDecodeError(errNilValue, "Attestation1")
+	}
 	att1, err := s.Attestation1.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Attestation1")
 	}
+	if s.Attestation2 == nil {
+		return nil, server.NewDecodeError(errNilValue, "Attestation2")
+	}
 	att2, err := s.Attestation2.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Attestation2")
@@ -756,6 +818,9 @@ func (a *IndexedAttestation) ToConsensus() (*eth.IndexedAttestation, error) {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("AttestingIndices[%d]", i))
 		}
 	}
+	if a.Data == nil {
+		return nil, server.NewDecodeError(errNilValue, "Data")
+	}
 	data, err := a.Data.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Data")
@@ -788,6 +853,9 @@ func (a *IndexedAttestationElectra) ToConsensus() (*eth.IndexedAttestationElectr
 			return nil, server.NewDecodeError(err, fmt.Sprintf("AttestingIndices[%d]", i))
 		}
 	}
+	if a.Data == nil {
+		return nil, server.NewDecodeError(errNilValue, "Data")
+	}
 	data, err := a.Data.ToConsensus()
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Data")
@@ -943,11 +1011,11 @@ func (d *DepositRequest) ToConsensus() (*enginev1.DepositRequest, error) {

 func ProposerSlashingsToConsensus(src []*ProposerSlashing) ([]*eth.ProposerSlashing, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "ProposerSlashings")
 	}
 	err := slice.VerifyMaxLength(src, 16)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "ProposerSlashings")
 	}
 	proposerSlashings := make([]*eth.ProposerSlashing, len(src))
 	for i, s := range src {
@@ -1076,11 +1144,11 @@ func ProposerSlashingFromConsensus(src *eth.ProposerSlashing) *ProposerSlashing

 func AttesterSlashingsToConsensus(src []*AttesterSlashing) ([]*eth.AttesterSlashing, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "AttesterSlashings")
 	}
 	err := slice.VerifyMaxLength(src, 2)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "AttesterSlashings")
 	}

 	attesterSlashings := make([]*eth.AttesterSlashing, len(src))
@@ -1091,10 +1159,19 @@ func AttesterSlashingsToConsensus(src []*AttesterSlashing) ([]*eth.AttesterSlash
 		if s.Attestation1 == nil {
 			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation1", i))
 		}
+
+		if s.Attestation1.Data == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation1.Data", i))
+		}
+
 		if s.Attestation2 == nil {
 			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation2", i))
 		}

+		if s.Attestation2.Data == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation2.Data", i))
+		}
+
 		a1Sig, err := bytesutil.DecodeHexWithLength(s.Attestation1.Signature, fieldparams.BLSSignatureLength)
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d].Attestation1.Signature", i))
@@ -1111,6 +1188,7 @@ func AttesterSlashingsToConsensus(src []*AttesterSlashing) ([]*eth.AttesterSlash
 			}
 			a1AttestingIndices[j] = attestingIndex
 		}
+
 		a1Data, err := s.Attestation1.Data.ToConsensus()
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d].Attestation1.Data", i))
@@ -1208,11 +1286,11 @@ func AttesterSlashingFromConsensus(src *eth.AttesterSlashing) *AttesterSlashing

 func AttesterSlashingsElectraToConsensus(src []*AttesterSlashingElectra) ([]*eth.AttesterSlashingElectra, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "AttesterSlashingsElectra")
 	}
 	err := slice.VerifyMaxLength(src, fieldparams.MaxAttesterSlashingsElectra)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "AttesterSlashingsElectra")
 	}

 	attesterSlashings := make([]*eth.AttesterSlashingElectra, len(src))
@@ -1220,13 +1298,23 @@ func AttesterSlashingsElectraToConsensus(src []*AttesterSlashingElectra) ([]*eth
 		if s == nil {
 			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d]", i))
 		}
+
 		if s.Attestation1 == nil {
 			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation1", i))
 		}
+
+		if s.Attestation1.Data == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation1.Data", i))
+		}
+
 		if s.Attestation2 == nil {
 			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation2", i))
 		}

+		if s.Attestation2.Data == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d].Attestation2.Data", i))
+		}
+
 		a1Sig, err := bytesutil.DecodeHexWithLength(s.Attestation1.Signature, fieldparams.BLSSignatureLength)
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d].Attestation1.Signature", i))
@@ -1340,15 +1428,18 @@ func AttesterSlashingElectraFromConsensus(src *eth.AttesterSlashingElectra) *Att

 func AttsToConsensus(src []*Attestation) ([]*eth.Attestation, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "Attestations")
 	}
 	err := slice.VerifyMaxLength(src, 128)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "Attestations")
 	}

 	atts := make([]*eth.Attestation, len(src))
 	for i, a := range src {
+		if a == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d]", i))
+		}
 		atts[i], err = a.ToConsensus()
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d]", i))
@@ -1367,15 +1458,18 @@ func AttsFromConsensus(src []*eth.Attestation) []*Attestation {

 func AttsElectraToConsensus(src []*AttestationElectra) ([]*eth.AttestationElectra, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "AttestationsElectra")
 	}
 	err := slice.VerifyMaxLength(src, 8)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "AttestationsElectra")
 	}

 	atts := make([]*eth.AttestationElectra, len(src))
 	for i, a := range src {
+		if a == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d]", i))
+		}
 		atts[i], err = a.ToConsensus()
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d]", i))
@@ -1394,11 +1488,11 @@ func AttsElectraFromConsensus(src []*eth.AttestationElectra) []*AttestationElect

 func DepositsToConsensus(src []*Deposit) ([]*eth.Deposit, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "Deposits")
 	}
 	err := slice.VerifyMaxLength(src, 16)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "Deposits")
 	}

 	deposits := make([]*eth.Deposit, len(src))
@@ -1470,15 +1564,18 @@ func DepositsFromConsensus(src []*eth.Deposit) []*Deposit {

 func SignedExitsToConsensus(src []*SignedVoluntaryExit) ([]*eth.SignedVoluntaryExit, error) {
 	if src == nil {
-		return nil, errNilValue
+		return nil, server.NewDecodeError(errNilValue, "SignedVoluntaryExits")
 	}
 	err := slice.VerifyMaxLength(src, 16)
 	if err != nil {
-		return nil, err
+		return nil, server.NewDecodeError(err, "SignedVoluntaryExits")
 	}

 	exits := make([]*eth.SignedVoluntaryExit, len(src))
 	for i, e := range src {
+		if e == nil {
+			return nil, server.NewDecodeError(errNilValue, fmt.Sprintf("[%d]", i))
+		}
 		exits[i], err = e.ToConsensus()
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d]", i))
--- a/api/server/structs/conversions_test.go
+++ b/api/server/structs/conversions_test.go
@@ -24,3 +24,96 @@ func TestDepositSnapshotFromConsensus(t *testing.T) {
 	require.Equal(t, "0x1234", res.ExecutionBlockHash)
 	require.Equal(t, "67890", res.ExecutionBlockHeight)
 }
+
+func TestSignedBLSToExecutionChange_ToConsensus(t *testing.T) {
+	s := &SignedBLSToExecutionChange{Message: nil, Signature: ""}
+	_, err := s.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestSignedValidatorRegistration_ToConsensus(t *testing.T) {
+	s := &SignedValidatorRegistration{Message: nil, Signature: ""}
+	_, err := s.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestSignedContributionAndProof_ToConsensus(t *testing.T) {
+	s := &SignedContributionAndProof{Message: nil, Signature: ""}
+	_, err := s.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestContributionAndProof_ToConsensus(t *testing.T) {
+	c := &ContributionAndProof{
+		Contribution:    nil,
+		AggregatorIndex: "invalid",
+		SelectionProof:  "",
+	}
+	_, err := c.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestSignedAggregateAttestationAndProof_ToConsensus(t *testing.T) {
+	s := &SignedAggregateAttestationAndProof{Message: nil, Signature: ""}
+	_, err := s.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestAggregateAttestationAndProof_ToConsensus(t *testing.T) {
+	a := &AggregateAttestationAndProof{
+		AggregatorIndex: "1",
+		Aggregate:       nil,
+		SelectionProof:  "",
+	}
+	_, err := a.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestAttestation_ToConsensus(t *testing.T) {
+	a := &Attestation{
+		AggregationBits: "0x10",
+		Data:            nil,
+		Signature:       "",
+	}
+	_, err := a.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestSingleAttestation_ToConsensus(t *testing.T) {
+	s := &SingleAttestation{
+		CommitteeIndex: "1",
+		AttesterIndex:  "1",
+		Data:           nil,
+		Signature:      "",
+	}
+	_, err := s.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestSignedVoluntaryExit_ToConsensus(t *testing.T) {
+	s := &SignedVoluntaryExit{Message: nil, Signature: ""}
+	_, err := s.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestProposerSlashing_ToConsensus(t *testing.T) {
+	p := &ProposerSlashing{SignedHeader1: nil, SignedHeader2: nil}
+	_, err := p.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestAttesterSlashing_ToConsensus(t *testing.T) {
+	a := &AttesterSlashing{Attestation1: nil, Attestation2: nil}
+	_, err := a.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}
+
+func TestIndexedAttestation_ToConsensus(t *testing.T) {
+	a := &IndexedAttestation{
+		AttestingIndices: []string{"1"},
+		Data:             nil,
+		Signature:        "invalid",
+	}
+	_, err := a.ToConsensus()
+	require.ErrorContains(t, errNilValue.Error(), err)
+}