diff --git a/beacon-chain/cache/depositcache/deposits_cache.go b/beacon-chain/cache/depositcache/deposits_cache.go index ffc8d82f73..a00552ed6b 100644 --- a/beacon-chain/cache/depositcache/deposits_cache.go +++ b/beacon-chain/cache/depositcache/deposits_cache.go @@ -136,7 +136,10 @@ func (dc *DepositCache) InsertFinalizedDeposits(ctx context.Context, eth1Deposit log.WithError(err).Error("Could not hash deposit data. Finalized deposit cache not updated.") return } - depositTrie.Insert(depHash[:], insertIndex) + if err = depositTrie.Insert(depHash[:], insertIndex); err != nil { + log.WithError(err).Error("Could not insert deposit hash") + return + } insertIndex++ } diff --git a/beacon-chain/powchain/deposit_test.go b/beacon-chain/powchain/deposit_test.go index 9050233c7b..bbc1ee44f5 100644 --- a/beacon-chain/powchain/deposit_test.go +++ b/beacon-chain/powchain/deposit_test.go @@ -220,7 +220,7 @@ func TestProcessDeposit_IncompleteDeposit(t *testing.T) { factor := params.BeaconConfig().MaxEffectiveBalance / params.BeaconConfig().EffectiveBalanceIncrement // deposit till 31e9 for i := 0; i < int(factor-1); i++ { - trie.Insert(dataRoot[:], i) + assert.NoError(t, trie.Insert(dataRoot[:], i)) trieRoot := trie.HashTreeRoot() eth1Data.DepositRoot = trieRoot[:] diff --git a/beacon-chain/powchain/log_processing.go b/beacon-chain/powchain/log_processing.go index fffa3c8456..9c2f7cf094 100644 --- a/beacon-chain/powchain/log_processing.go +++ b/beacon-chain/powchain/log_processing.go @@ -138,7 +138,9 @@ func (s *Service) ProcessDepositLog(ctx context.Context, depositLog gethTypes.Lo if s.depositTrie.NumOfItems() != int(index) { return errors.Errorf("invalid deposit index received: wanted %d but got %d", s.depositTrie.NumOfItems(), index) } - s.depositTrie.Insert(depositHash[:], int(index)) + if err = s.depositTrie.Insert(depositHash[:], int(index)); err != nil { + return err + } deposit := ðpb.Deposit{ Data: depositData, diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go index 03f9a3e918..9808fd521a 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go @@ -697,7 +697,9 @@ func (vs *Server) depositTrie(ctx context.Context, canonicalEth1Data *ethpb.Eth1 if err != nil { return nil, errors.Wrap(err, "could not hash deposit data") } - depositTrie.Insert(depHash[:], int(insertIndex)) + if err = depositTrie.Insert(depHash[:], int(insertIndex)); err != nil { + return nil, err + } insertIndex++ } valid, err := vs.validateDepositTrie(depositTrie, canonicalEth1Data) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go index a3460db22d..88546220ca 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go @@ -486,7 +486,7 @@ func TestProposer_PendingDeposits_OutsideEth1FollowWindow(t *testing.T) { depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, dp.Eth1BlockHeight, dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { @@ -621,7 +621,7 @@ func TestProposer_PendingDeposits_FollowsCorrectEth1Block(t *testing.T) { depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, dp.Eth1BlockHeight, dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { @@ -720,7 +720,7 @@ func TestProposer_PendingDeposits_CantReturnBelowStateEth1DepositIndex(t *testin depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, uint64(dp.Index), dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { @@ -816,7 +816,7 @@ func TestProposer_PendingDeposits_CantReturnMoreThanMax(t *testing.T) { depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, height.Uint64(), dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { @@ -910,7 +910,7 @@ func TestProposer_PendingDeposits_CantReturnMoreThanDepositCount(t *testing.T) { depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, uint64(dp.Index), dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { @@ -1019,7 +1019,7 @@ func TestProposer_DepositTrie_UtilizesCachedFinalizedDeposits(t *testing.T) { depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, dp.Eth1BlockHeight, dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { @@ -1129,7 +1129,7 @@ func TestProposer_DepositTrie_RebuildTrie(t *testing.T) { depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, dp.Eth1BlockHeight, dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { @@ -1191,17 +1191,17 @@ func TestProposer_ValidateDepositTrie(t *testing.T) { eth1dataCreator: func() *ethpb.Eth1Data { trie, err := trie.NewTrie(params.BeaconConfig().DepositContractTreeDepth) assert.NoError(t, err) - trie.Insert([]byte{'a'}, 0) - trie.Insert([]byte{'b'}, 1) - trie.Insert([]byte{'c'}, 2) + assert.NoError(t, trie.Insert([]byte{'a'}, 0)) + assert.NoError(t, trie.Insert([]byte{'b'}, 1)) + assert.NoError(t, trie.Insert([]byte{'c'}, 2)) return ðpb.Eth1Data{DepositRoot: []byte{'B'}, DepositCount: 3, BlockHash: []byte{}} }, trieCreator: func() *trie.SparseMerkleTrie { trie, err := trie.NewTrie(params.BeaconConfig().DepositContractTreeDepth) assert.NoError(t, err) - trie.Insert([]byte{'a'}, 0) - trie.Insert([]byte{'b'}, 1) - trie.Insert([]byte{'c'}, 2) + assert.NoError(t, trie.Insert([]byte{'a'}, 0)) + assert.NoError(t, trie.Insert([]byte{'b'}, 1)) + assert.NoError(t, trie.Insert([]byte{'c'}, 2)) return trie }, success: false, @@ -1211,18 +1211,18 @@ func TestProposer_ValidateDepositTrie(t *testing.T) { eth1dataCreator: func() *ethpb.Eth1Data { trie, err := trie.NewTrie(params.BeaconConfig().DepositContractTreeDepth) assert.NoError(t, err) - trie.Insert([]byte{'a'}, 0) - trie.Insert([]byte{'b'}, 1) - trie.Insert([]byte{'c'}, 2) + assert.NoError(t, trie.Insert([]byte{'a'}, 0)) + assert.NoError(t, trie.Insert([]byte{'b'}, 1)) + assert.NoError(t, trie.Insert([]byte{'c'}, 2)) rt := trie.HashTreeRoot() return ðpb.Eth1Data{DepositRoot: rt[:], DepositCount: 3, BlockHash: []byte{}} }, trieCreator: func() *trie.SparseMerkleTrie { trie, err := trie.NewTrie(params.BeaconConfig().DepositContractTreeDepth) assert.NoError(t, err) - trie.Insert([]byte{'a'}, 0) - trie.Insert([]byte{'b'}, 1) - trie.Insert([]byte{'c'}, 2) + assert.NoError(t, trie.Insert([]byte{'a'}, 0)) + assert.NoError(t, trie.Insert([]byte{'b'}, 1)) + assert.NoError(t, trie.Insert([]byte{'c'}, 2)) return trie }, success: true, @@ -1960,7 +1960,7 @@ func TestProposer_Deposits_ReturnsEmptyList_IfLatestEth1DataEqGenesisEth1Block(t depositHash, err := dp.Deposit.Data.HashTreeRoot() require.NoError(t, err, "Unable to determine hashed value of deposit") - depositTrie.Insert(depositHash[:], int(dp.Index)) + assert.NoError(t, depositTrie.Insert(depositHash[:], int(dp.Index))) assert.NoError(t, depositCache.InsertDeposit(ctx, dp.Deposit, uint64(dp.Index), dp.Index, depositTrie.HashTreeRoot())) } for _, dp := range recentDeposits { diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/status_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/status_test.go index c7adf1a185..57eeef2661 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/status_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/status_test.go @@ -532,7 +532,7 @@ func TestActivationStatus_OK(t *testing.T) { assert.NoError(t, depositCache.InsertDeposit(ctx, dep, 10 /*blockNum*/, 0, depositTrie.HashTreeRoot())) dep = deposits[2] - depositTrie.Insert(dep.Data.Signature, 15) + assert.NoError(t, depositTrie.Insert(dep.Data.Signature, 15)) assert.NoError(t, depositCache.InsertDeposit(context.Background(), dep, 0, 1, depositTrie.HashTreeRoot())) vs := &Server{ @@ -736,7 +736,7 @@ func TestMultipleValidatorStatus_Pubkeys(t *testing.T) { dep := deposits[0] assert.NoError(t, depositCache.InsertDeposit(ctx, dep, 10 /*blockNum*/, 0, depositTrie.HashTreeRoot())) dep = deposits[2] - depositTrie.Insert(dep.Data.Signature, 15) + assert.NoError(t, depositTrie.Insert(dep.Data.Signature, 15)) assert.NoError(t, depositCache.InsertDeposit(context.Background(), dep, 0, 1, depositTrie.HashTreeRoot())) vs := &Server{ diff --git a/container/trie/BUILD.bazel b/container/trie/BUILD.bazel index 52e9494bdc..cca7875001 100644 --- a/container/trie/BUILD.bazel +++ b/container/trie/BUILD.bazel @@ -27,6 +27,7 @@ go_test( "//crypto/hash:go_default_library", "//encoding/bytesutil:go_default_library", "//proto/prysm/v1alpha1:go_default_library", + "//testing/assert:go_default_library", "//testing/require:go_default_library", "@com_github_ethereum_go_ethereum//accounts/abi/bind:go_default_library", ], diff --git a/container/trie/sparse_merkle.go b/container/trie/sparse_merkle.go index 2ad23e91a1..26815b29ef 100644 --- a/container/trie/sparse_merkle.go +++ b/container/trie/sparse_merkle.go @@ -93,7 +93,10 @@ func (m *SparseMerkleTrie) HashTreeRoot() [32]byte { } // Insert an item into the trie. -func (m *SparseMerkleTrie) Insert(item []byte, index int) { +func (m *SparseMerkleTrie) Insert(item []byte, index int) error { + if index < 0 { + return fmt.Errorf("negative index provided: %d", index) + } for index >= len(m.branches[0]) { m.branches[0] = append(m.branches[0], ZeroHashes[0][:]) } @@ -132,10 +135,14 @@ func (m *SparseMerkleTrie) Insert(item []byte, index int) { } currentIndex = parentIdx } + return nil } // MerkleProof computes a proof from a trie's branches using a Merkle index. func (m *SparseMerkleTrie) MerkleProof(index int) ([][]byte, error) { + if index < 0 { + return nil, fmt.Errorf("merkle index is negative: %d", index) + } merkleIndex := uint(index) leaves := m.branches[0] if index >= len(leaves) { diff --git a/container/trie/sparse_merkle_test.go b/container/trie/sparse_merkle_test.go index 41733528df..6aa0f179d2 100644 --- a/container/trie/sparse_merkle_test.go +++ b/container/trie/sparse_merkle_test.go @@ -11,6 +11,7 @@ import ( "github.com/prysmaticlabs/prysm/crypto/hash" "github.com/prysmaticlabs/prysm/encoding/bytesutil" ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1" + "github.com/prysmaticlabs/prysm/testing/assert" "github.com/prysmaticlabs/prysm/testing/require" ) @@ -112,6 +113,24 @@ func TestMerkleTrie_VerifyMerkleProof(t *testing.T) { require.Equal(t, false, trie.VerifyMerkleBranch(root[:], []byte("buzz"), 3, proof, params.BeaconConfig().DepositContractTreeDepth)) } +func TestMerkleTrie_NegativeIndexes(t *testing.T) { + items := [][]byte{ + []byte("A"), + []byte("B"), + []byte("C"), + []byte("D"), + []byte("E"), + []byte("F"), + []byte("G"), + []byte("H"), + } + m, err := trie.GenerateTrieFromItems(items, params.BeaconConfig().DepositContractTreeDepth) + require.NoError(t, err) + _, err = m.MerkleProof(-1) + require.ErrorContains(t, "merkle index is negative", err) + require.ErrorContains(t, "negative index provided", m.Insert([]byte{'J'}, -1)) +} + func TestMerkleTrie_VerifyMerkleProof_TrieUpdated(t *testing.T) { items := [][]byte{ {1}, @@ -128,7 +147,7 @@ func TestMerkleTrie_VerifyMerkleProof_TrieUpdated(t *testing.T) { require.Equal(t, true, trie.VerifyMerkleBranch(root[:], items[0], 0, proof, depth)) // Now we update the trie. - m.Insert([]byte{5}, 3) + assert.NoError(t, m.Insert([]byte{5}, 3)) proof, err = m.MerkleProof(3) require.NoError(t, err) root = m.HashTreeRoot() @@ -140,7 +159,7 @@ func TestMerkleTrie_VerifyMerkleProof_TrieUpdated(t *testing.T) { } // Now we update the trie at an index larger than the number of items. - m.Insert([]byte{6}, 15) + assert.NoError(t, m.Insert([]byte{6}, 15)) } func TestRoundtripProto_OK(t *testing.T) { @@ -209,7 +228,7 @@ func BenchmarkInsertTrie_Optimized(b *testing.B) { someItem := bytesutil.ToBytes32([]byte("hello-world")) b.StartTimer() for i := 0; i < b.N; i++ { - tr.Insert(someItem[:], i%numDeposits) + require.NoError(b, tr.Insert(someItem[:], i%numDeposits)) } } diff --git a/contracts/deposit/deposit_tree_test.go b/contracts/deposit/deposit_tree_test.go index b5ab74f3da..1c3ee8b612 100644 --- a/contracts/deposit/deposit_tree_test.go +++ b/contracts/deposit/deposit_tree_test.go @@ -44,7 +44,7 @@ func TestDepositTrieRoot_OK(t *testing.T) { item, err := data.HashTreeRoot() require.NoError(t, err) - localTrie.Insert(item[:], i) + assert.NoError(t, localTrie.Insert(item[:], i)) depRoot, err = testAcc.Contract.GetDepositRoot(&bind.CallOpts{}) require.NoError(t, err) assert.Equal(t, depRoot, localTrie.HashTreeRoot(), "Local deposit trie root and contract deposit trie root are not equal for index %d", i) @@ -84,7 +84,7 @@ func TestDepositTrieRoot_Fail(t *testing.T) { item, err := data.HashTreeRoot() require.NoError(t, err) - localTrie.Insert(item[:], i) + assert.NoError(t, localTrie.Insert(item[:], i)) depRoot, err = testAcc.Contract.GetDepositRoot(&bind.CallOpts{}) require.NoError(t, err) diff --git a/encoding/bytesutil/bytes.go b/encoding/bytesutil/bytes.go index 1ed97e9a98..deaace0d9c 100644 --- a/encoding/bytesutil/bytes.go +++ b/encoding/bytesutil/bytes.go @@ -16,6 +16,9 @@ var hexRegex = regexp.MustCompile("^0x[0-9a-fA-F]+$") // ToBytes returns integer x to bytes in little-endian format at the specified length. // Spec defines similar method uint_to_bytes(n: uint) -> bytes, which is equivalent to ToBytes(n, 8). func ToBytes(x uint64, length int) []byte { + if length < 0 { + length = 0 + } makeLength := length if length < 8 { makeLength = 8 @@ -70,6 +73,9 @@ func Bytes32(x uint64) []byte { // FromBytes4 returns an integer which is stored in the little-endian format(4, 'little') // from a byte array. func FromBytes4(x []byte) uint64 { + if len(x) < 4 { + return 0 + } empty4bytes := make([]byte, 4) return binary.LittleEndian.Uint64(append(x[:4], empty4bytes...)) } @@ -77,6 +83,9 @@ func FromBytes4(x []byte) uint64 { // FromBytes8 returns an integer which is stored in the little-endian format(8, 'little') // from a byte array. func FromBytes8(x []byte) uint64 { + if len(x) < 8 { + return 0 + } return binary.LittleEndian.Uint64(x) } @@ -134,6 +143,9 @@ func ToBool(x byte) bool { // FromBytes2 returns an integer which is stored in the little-endian format(2, 'little') // from a byte array. func FromBytes2(x []byte) uint16 { + if len(x) < 2 { + return 0 + } return binary.LittleEndian.Uint16(x[:2]) } @@ -172,9 +184,11 @@ func Trunc(x []byte) []byte { // ToLowInt64 returns the lowest 8 bytes interpreted as little endian. func ToLowInt64(x []byte) int64 { - if len(x) > 8 { - x = x[:8] + if len(x) < 8 { + return 0 } + // Use the first 8 bytes. + x = x[:8] return int64(binary.LittleEndian.Uint64(x)) } @@ -249,7 +263,7 @@ func SetBit(b []byte, i int) []byte { // Returns the original bitlist if the index `i` // is out of range. func ClearBit(b []byte, i int) []byte { - if i >= len(b)*8 { + if i >= len(b)*8 || i < 0 { return b } @@ -288,6 +302,9 @@ func HighestBitIndexAt(b []byte, index int) (int, error) { if b == nil || bLength == 0 { return 0, errors.New("input list can't be empty or nil") } + if index < 0 { + return 0, errors.Errorf("index is negative: %d", index) + } start := index / 8 if start >= bLength { diff --git a/encoding/bytesutil/bytes_test.go b/encoding/bytesutil/bytes_test.go index 78b2b544c3..5f73101e7c 100644 --- a/encoding/bytesutil/bytes_test.go +++ b/encoding/bytesutil/bytes_test.go @@ -471,3 +471,29 @@ func TestSafeCopy2dBytes(t *testing.T) { }) } } + +func TestBytesInvalidInputs(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Errorf("Test panicked: %v", r) + } + }() + rawBytes := bytesutil.ToBytes(100, -10) + assert.DeepEqual(t, []byte{}, rawBytes) + + _, err := bytesutil.HighestBitIndexAt([]byte{'A', 'B', 'C'}, -5) + assert.ErrorContains(t, "index is negative", err) + + // There should be no panic + _ = bytesutil.ClearBit([]byte{'C', 'D', 'E'}, -7) + res := bytesutil.FromBytes4([]byte{}) + assert.Equal(t, res, uint64(0)) + newRes := bytesutil.FromBytes2([]byte{}) + assert.Equal(t, newRes, uint16(0)) + res = bytesutil.FromBytes8([]byte{}) + assert.Equal(t, res, uint64(0)) + + intRes := bytesutil.ToLowInt64([]byte{}) + assert.Equal(t, intRes, int64(0)) + +} diff --git a/testing/util/deposits.go b/testing/util/deposits.go index 33b08c38c3..dd37a05894 100644 --- a/testing/util/deposits.go +++ b/testing/util/deposits.go @@ -69,7 +69,9 @@ func DeterministicDepositsAndKeys(numDeposits uint64) ([]*ethpb.Deposit, []bls.S return nil, nil, errors.Wrap(err, "could not tree hash deposit data") } - t.Insert(hashedDeposit[:], int(numExisting+i)) + if err = t.Insert(hashedDeposit[:], int(numExisting+i)); err != nil { + return nil, nil, err + } } } @@ -138,7 +140,9 @@ func DepositsWithBalance(balances []uint64) ([]*ethpb.Deposit, *trie.SparseMerkl return nil, nil, errors.Wrap(err, "could not tree hash deposit data") } - sparseTrie.Insert(hashedDeposit[:], int(i)) + if err = sparseTrie.Insert(hashedDeposit[:], int(i)); err != nil { + return nil, nil, err + } } depositTrie, _, err := DepositTrieSubset(sparseTrie, int(numDeposits)) @@ -364,7 +368,9 @@ func DeterministicDepositsAndKeysSameValidator(numDeposits uint64) ([]*ethpb.Dep return nil, nil, errors.Wrap(err, "could not tree hash deposit data") } - t.Insert(hashedDeposit[:], int(numExisting+i)) + if err = t.Insert(hashedDeposit[:], int(numExisting+i)); err != nil { + return nil, nil, err + } } }