fuzz: Fix off by one error in sparse merkle trie item insertion (#10668)

* fuzz: Fix off by one error in sparse merkle trie item insertion

* remove new line

* Move validation to the proto constructor

* fix build

* Add a unit test because @nisdas is going to ask for it

* fix up

* gaz

* Update container/trie/sparse_merkle.go

* Update container/trie/sparse_merkle_test.go

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: nisdas <nishdas93@gmail.com>

beacon-chain/blockchain/BUILD.bazel

@@ -137,6 +137,7 @@ go_test(
         "//config/fieldparams:go_default_library",
         "//config/params:go_default_library",
         "//consensus-types/wrapper:go_default_library",
+        "//container/trie:go_default_library",
         "//encoding/bytesutil:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
         "//testing/assert:go_default_library",
@@ -188,6 +189,7 @@ go_test(
         "//beacon-chain/powchain/testing:go_default_library",
         "//config/params:go_default_library",
         "//consensus-types/wrapper:go_default_library",
+        "//container/trie:go_default_library",
         "//encoding/bytesutil:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
         "//testing/assert:go_default_library",

beacon-chain/blockchain/service_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/prysmaticlabs/prysm/config/params"
 	"github.com/prysmaticlabs/prysm/consensus-types/interfaces"
 	"github.com/prysmaticlabs/prysm/consensus-types/wrapper"
+	"github.com/prysmaticlabs/prysm/container/trie"
 	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
 	ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/testing/assert"
@@ -86,9 +87,11 @@ func setupBeaconChain(t *testing.T, beaconDB db.Database) *Service {
 	bState, _ := util.DeterministicGenesisState(t, 10)
 	pbState, err := v1.ProtobufBeaconState(bState.InnerStateUnsafe())
 	require.NoError(t, err)
+	mockTrie, err := trie.NewTrie(0)
+	require.NoError(t, err)
 	err = beaconDB.SavePowchainData(ctx, &ethpb.ETH1ChainData{
 		BeaconState: pbState,
-		Trie:        &ethpb.SparseMerkleTrie{},
+		Trie:        mockTrie.ToProto(),
 		CurrentEth1Data: &ethpb.LatestETH1Data{
 			BlockHash: make([]byte, 32),
 		},

beacon-chain/powchain/service.go

@@ -767,9 +767,12 @@ func (s *Service) initializeEth1Data(ctx context.Context, eth1DataInDB *ethpb.ET
 	if eth1DataInDB == nil {
 		return nil
 	}
-	s.depositTrie = trie.CreateTrieFromProto(eth1DataInDB.Trie)
-	s.chainStartData = eth1DataInDB.ChainstartData
 	var err error
+	s.depositTrie, err = trie.CreateTrieFromProto(eth1DataInDB.Trie)
+	if err != nil {
+		return err
+	}
+	s.chainStartData = eth1DataInDB.ChainstartData
 	if !reflect.ValueOf(eth1DataInDB.BeaconState).IsZero() {
 		s.preGenesisState, err = v1.InitializeFromProto(eth1DataInDB.BeaconState)
 		if err != nil {

container/trie/BUILD.bazel

@@ -13,6 +13,7 @@ go_library(
         "//encoding/bytesutil:go_default_library",
         "//math:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
+        "@com_github_pkg_errors//:go_default_library",
     ],
 )
 

container/trie/sparse_merkle.go

@@ -4,9 +4,9 @@ package trie
 import (
 	"bytes"
 	"encoding/binary"
-	"errors"
 	"fmt"
 
+	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/crypto/hash"
 	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
 	"github.com/prysmaticlabs/prysm/math"
@@ -29,7 +29,7 @@ func NewTrie(depth uint64) (*SparseMerkleTrie, error) {
 }
 
 // CreateTrieFromProto creates a Sparse Merkle Trie from its corresponding merkle trie.
-func CreateTrieFromProto(trieObj *protodb.SparseMerkleTrie) *SparseMerkleTrie {
+func CreateTrieFromProto(trieObj *protodb.SparseMerkleTrie) (*SparseMerkleTrie, error) {
 	trie := &SparseMerkleTrie{
 		depth:         uint(trieObj.Depth),
 		originalItems: trieObj.OriginalItems,
@@ -39,7 +39,29 @@ func CreateTrieFromProto(trieObj *protodb.SparseMerkleTrie) *SparseMerkleTrie {
 		branches[i] = layer.Layer
 	}
 	trie.branches = branches
-	return trie
+
+	if err := trie.validate(); err != nil {
+		return nil, errors.Wrap(err, "invalid sparse merkle trie")
+	}
+
+	return trie, nil
+}
+
+func (m *SparseMerkleTrie) validate() error {
+	if len(m.branches) == 0 {
+		return errors.New("no branches")
+	}
+	if len(m.branches[len(m.branches)-1]) == 0 {
+		return errors.New("invalid branches provided")
+	}
+	if m.depth >= uint(len(m.branches)) {
+		return errors.New("depth is greater than or equal to number of branches")
+	}
+	if m.depth >= 64 {
+		return errors.New("depth exceeds 64") // PowerOf2 would overflow.
+	}
+
+	return nil
 }
 
 // GenerateTrieFromItems constructs a Merkle trie from a sequence of byte slices.
@@ -82,10 +104,6 @@ func (m *SparseMerkleTrie) Items() [][]byte {
 //  Spec Definition:
 //   sha256(concat(node, self.to_little_endian_64(self.deposit_count), slice(zero_bytes32, start=0, len=24)))
 func (m *SparseMerkleTrie) HashTreeRoot() ([32]byte, error) {
-	if len(m.branches) == 0 || len(m.branches[len(m.branches)-1]) == 0 {
-		return [32]byte{}, errors.New("invalid branches provided to compute root")
-	}
-
 	enc := [32]byte{}
 	depositCount := uint64(len(m.originalItems))
 	if len(m.originalItems) == 1 && bytes.Equal(m.originalItems[0], ZeroHashes[0][:]) {
@@ -101,12 +119,6 @@ func (m *SparseMerkleTrie) Insert(item []byte, index int) error {
 	if index < 0 {
 		return fmt.Errorf("negative index provided: %d", index)
 	}
-	if len(m.branches) == 0 {
-		return errors.New("invalid trie: no branches")
-	}
-	if m.depth > uint(len(m.branches)) {
-		return errors.New("invalid trie: depth is greater than number of branches")
-	}
 	for index >= len(m.branches[0]) {
 		m.branches[0] = append(m.branches[0], ZeroHashes[0][:])
 	}
@@ -153,16 +165,10 @@ func (m *SparseMerkleTrie) MerkleProof(index int) ([][]byte, error) {
 	if index < 0 {
 		return nil, fmt.Errorf("merkle index is negative: %d", index)
 	}
-	if len(m.branches) == 0 {
-		return nil, errors.New("invalid trie: no branches")
-	}
 	leaves := m.branches[0]
 	if index >= len(leaves) {
 		return nil, fmt.Errorf("merkle index out of range in trie, max range: %d, received: %d", len(leaves), index)
 	}
-	if m.depth > uint(len(m.branches)) {
-		return nil, errors.New("invalid trie: depth is greater than number of branches")
-	}
 	merkleIndex := uint(index)
 	proof := make([][]byte, m.depth+1)
 	for i := uint(0); i < m.depth; i++ {

container/trie/sparse_merkle_test.go

@@ -16,6 +16,68 @@ import (
 	"github.com/prysmaticlabs/prysm/testing/require"
 )
 
+func TestCreateTrieFromProto_Validation(t *testing.T) {
+	h := hash.Hash([]byte("hi"))
+	genValidLayers := func(num int) []*ethpb.TrieLayer {
+		l := make([]*ethpb.TrieLayer, num)
+		for i := 0; i < num; i++ {
+			l[i] = &ethpb.TrieLayer{
+				Layer: [][]byte{h[:]},
+			}
+		}
+		return l
+	}
+	tests := []struct {
+		trie      *ethpb.SparseMerkleTrie
+		errString string
+	}{
+		{
+			trie: &ethpb.SparseMerkleTrie{
+				Layers: []*ethpb.TrieLayer{},
+				Depth:  0,
+			},
+			errString: "no branches",
+		},
+		{
+			trie: &ethpb.SparseMerkleTrie{
+				Layers: []*ethpb.TrieLayer{
+					{
+						Layer: [][]byte{h[:]},
+					},
+					{
+						Layer: [][]byte{h[:]},
+					},
+					{
+						Layer: [][]byte{},
+					},
+				},
+				Depth: 2,
+			},
+			errString: "invalid branches provided",
+		},
+		{
+			trie: &ethpb.SparseMerkleTrie{
+				Layers: genValidLayers(3),
+				Depth:  12,
+			},
+			errString: "depth is greater than or equal to number of branches",
+		},
+		{
+			trie: &ethpb.SparseMerkleTrie{
+				Layers: genValidLayers(66),
+				Depth:  65,
+			},
+			errString: "depth exceeds 64",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.errString, func(t *testing.T) {
+			_, err := trie.CreateTrieFromProto(tt.trie)
+			require.ErrorContains(t, tt.errString, err)
+		})
+	}
+}
+
 func TestMarshalDepositWithProof(t *testing.T) {
 	items := [][]byte{
 		[]byte("A"),
@@ -52,7 +114,7 @@ func TestMarshalDepositWithProof(t *testing.T) {
 
 func TestMerkleTrie_MerkleProofOutOfRange(t *testing.T) {
 	h := hash.Hash([]byte("hi"))
-	m := trie.CreateTrieFromProto(&ethpb.SparseMerkleTrie{
+	m, err := trie.CreateTrieFromProto(&ethpb.SparseMerkleTrie{
 		Layers: []*ethpb.TrieLayer{
 			{
 				Layer: [][]byte{h[:]},
@@ -61,11 +123,12 @@ func TestMerkleTrie_MerkleProofOutOfRange(t *testing.T) {
 				Layer: [][]byte{h[:]},
 			},
 			{
-				Layer: [][]byte{},
+				Layer: [][]byte{h[:]},
 			},
 		},
-		Depth: 4,
+		Depth: 2,
 	})
+	require.NoError(t, err)
 	if _, err := m.MerkleProof(6); err == nil {
 		t.Error("Expected out of range failure, received nil", err)
 	}
@@ -128,6 +191,7 @@ func TestMerkleTrie_VerifyMerkleProof(t *testing.T) {
 		[]byte("G"),
 		[]byte("H"),
 	}
+
 	m, err := trie.GenerateTrieFromItems(items, params.BeaconConfig().DepositContractTreeDepth)
 	require.NoError(t, err)
 	proof, err := m.MerkleProof(0)
@@ -209,7 +273,8 @@ func TestRoundtripProto_OK(t *testing.T) {
 	depositRoot, err := m.HashTreeRoot()
 	require.NoError(t, err)
 
-	newTrie := trie.CreateTrieFromProto(protoTrie)
+	newTrie, err := trie.CreateTrieFromProto(protoTrie)
+	require.NoError(t, err)
 	root, err := newTrie.HashTreeRoot()
 	require.NoError(t, err)
 	require.DeepEqual(t, depositRoot, root)

container/trie/sparse_merkle_trie_fuzz_test.go

@@ -22,10 +22,10 @@ func FuzzSparseMerkleTrie_HashTreeRoot(f *testing.F) {
 				Layer: [][]byte{h[:]},
 			},
 			{
-				Layer: [][]byte{},
+				Layer: [][]byte{h[:]},
 			},
 		},
-		Depth: 4,
+		Depth: 2,
 	}
 	b, err := proto.Marshal(pb)
 	require.NoError(f, err)
@@ -36,10 +36,13 @@ func FuzzSparseMerkleTrie_HashTreeRoot(f *testing.F) {
 		if err := proto.Unmarshal(b, pb); err != nil {
 			return
 		}
-		_, err := trie.CreateTrieFromProto(pb).HashTreeRoot()
+		smt, err := trie.CreateTrieFromProto(pb)
 		if err != nil {
 			return
 		}
+		if _, err := smt.HashTreeRoot(); err != nil {
+			return
+		}
 	})
 }
 
@@ -54,10 +57,10 @@ func FuzzSparseMerkleTrie_MerkleProof(f *testing.F) {
 				Layer: [][]byte{h[:]},
 			},
 			{
-				Layer: [][]byte{},
+				Layer: [][]byte{h[:]},
 			},
 		},
-		Depth: 4,
+		Depth: 2,
 	}
 	b, err := proto.Marshal(pb)
 	require.NoError(f, err)
@@ -68,10 +71,13 @@ func FuzzSparseMerkleTrie_MerkleProof(f *testing.F) {
 		if err := proto.Unmarshal(b, pb); err != nil {
 			return
 		}
-		_, err := trie.CreateTrieFromProto(pb).MerkleProof(i)
+		smt, err := trie.CreateTrieFromProto(pb)
 		if err != nil {
 			return
 		}
+		if _, err := smt.MerkleProof(i); err != nil {
+			return
+		}
 	})
 }
 
@@ -86,10 +92,10 @@ func FuzzSparseMerkleTrie_Insert(f *testing.F) {
 				Layer: [][]byte{h[:]},
 			},
 			{
-				Layer: [][]byte{},
+				Layer: [][]byte{h[:]},
 			},
 		},
-		Depth: 4,
+		Depth: 2,
 	}
 	b, err := proto.Marshal(pb)
 	require.NoError(f, err)
@@ -100,7 +106,11 @@ func FuzzSparseMerkleTrie_Insert(f *testing.F) {
 		if err := proto.Unmarshal(b, pb); err != nil {
 			return
 		}
-		if err := trie.CreateTrieFromProto(pb).Insert(item, i); err != nil {
+		smt, err := trie.CreateTrieFromProto(pb)
+		if err != nil {
+			return
+		}
+		if err := smt.Insert(item, i); err != nil {
 			return
 		}
 	})

container/trie/testdata/fuzz/FuzzSparseMerkleTrie_Insert/fuzzbuzz-97663548a1e2280a081745e5fa25c289f7121c0b

@@ -0,0 +1,4 @@
+go test fuzz v1
+[]byte("\b\x03\x12\"2 00000000000000000000000000000000\x12\"2 00000000000000000000000000000000\x12\x00")
+[]byte("")
+int(0)