Merge pull request #1454 from nspcc-dev/core/policy_checks

core: add validation to policy methods

pkg/consensus/prepare_request.go

@@ -2,6 +2,7 @@ package consensus
 
 import (
 	"github.com/nspcc-dev/dbft/payload"
+	"github.com/nspcc-dev/neo-go/pkg/core/block"
 	"github.com/nspcc-dev/neo-go/pkg/io"
 	"github.com/nspcc-dev/neo-go/pkg/util"
 )
@@ -26,7 +27,7 @@ func (p *prepareRequest) EncodeBinary(w *io.BinWriter) {
 func (p *prepareRequest) DecodeBinary(r *io.BinReader) {
 	p.timestamp = r.ReadU64LE()
 	p.nonce = r.ReadU64LE()
-	r.ReadArray(&p.transactionHashes)
+	r.ReadArray(&p.transactionHashes, block.MaxTransactionsPerBlock)
 }
 
 // Timestamp implements payload.PrepareRequest interface.

pkg/consensus/prepare_request_test.go

@@ -3,7 +3,9 @@ package consensus
 import (
 	"testing"
 
+	"github.com/nspcc-dev/neo-go/pkg/core/block"
 	"github.com/nspcc-dev/neo-go/pkg/internal/random"
+	"github.com/nspcc-dev/neo-go/pkg/internal/testserdes"
 	"github.com/nspcc-dev/neo-go/pkg/util"
 	"github.com/stretchr/testify/require"
 )
@@ -30,3 +32,32 @@ func TestPrepareRequest_Setters(t *testing.T) {
 	p.SetTransactionHashes(hashes[:])
 	require.Equal(t, hashes[:], p.TransactionHashes())
 }
+
+func TestPrepareRequest_EncodeDecodeBinary(t *testing.T) {
+	t.Run("positive", func(t *testing.T) {
+		expected := &prepareRequest{
+			timestamp: 112,
+			nonce:     1325,
+			transactionHashes: []util.Uint256{
+				random.Uint256(),
+				random.Uint256(),
+			},
+		}
+		testserdes.EncodeDecodeBinary(t, expected, new(prepareRequest))
+	})
+
+	t.Run("bad hashes count", func(t *testing.T) {
+		hashes := make([]util.Uint256, block.MaxTransactionsPerBlock+1)
+		for i := range hashes {
+			hashes[i] = random.Uint256()
+		}
+		expected := &prepareRequest{
+			timestamp:         112,
+			nonce:             1325,
+			transactionHashes: hashes,
+		}
+		data, err := testserdes.EncodeBinary(expected)
+		require.NoError(t, err)
+		require.Error(t, testserdes.DecodeBinary(data, new(prepareRequest)))
+	})
+}

pkg/core/block/block.go

@@ -3,6 +3,7 @@ package block
 import (
 	"encoding/json"
 	"errors"
+	"math"
 
 	"github.com/Workiva/go-datastructures/queue"
 	"github.com/nspcc-dev/neo-go/pkg/config/netmode"
@@ -12,6 +13,16 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/util"
 )
 
+const (
+	// MaxContentsPerBlock is the maximum number of contents (transactions + consensus data) per block.
+	MaxContentsPerBlock = math.MaxUint16
+	// MaxTransactionsPerBlock is the maximum number of transactions per block.
+	MaxTransactionsPerBlock = MaxContentsPerBlock - 1
+)
+
+// ErrMaxContentsPerBlock is returned when the maximum number of contents per block is reached.
+var ErrMaxContentsPerBlock = errors.New("the number of contents exceeds the maximum number of contents per block")
+
 // Block represents one block in the chain.
 type Block struct {
 	// The base of the block.
@@ -82,6 +93,9 @@ func NewBlockFromTrimmedBytes(network netmode.Magic, b []byte) (*Block, error) {
 	block.Script.DecodeBinary(br)
 
 	lenHashes := br.ReadVarUint()
+	if lenHashes > MaxContentsPerBlock {
+		return nil, ErrMaxContentsPerBlock
+	}
 	if lenHashes > 0 {
 		var consensusDataHash util.Uint256
 		consensusDataHash.DecodeBinary(br)
@@ -142,6 +156,10 @@ func (b *Block) DecodeBinary(br *io.BinReader) {
 		br.Err = errors.New("invalid block format")
 		return
 	}
+	if contentsCount > MaxContentsPerBlock {
+		br.Err = ErrMaxContentsPerBlock
+		return
+	}
 	b.ConsensusData.DecodeBinary(br)
 	txes := make([]*transaction.Transaction, contentsCount-1)
 	for i := 0; i < int(contentsCount)-1; i++ {

pkg/core/block/block_test.go

@@ -3,6 +3,7 @@ package block
 import (
 	"encoding/base64"
 	"encoding/hex"
+	"errors"
 	"strings"
 	"testing"
 
@@ -218,3 +219,27 @@ func TestBlockCompare(t *testing.T) {
 	assert.Equal(t, 0, b2.Compare(&b2))
 	assert.Equal(t, -1, b2.Compare(&b3))
 }
+
+func TestBlockEncodeDecode(t *testing.T) {
+	t.Run("positive", func(t *testing.T) {
+		b := newDumbBlock()
+		b.Transactions = []*transaction.Transaction{}
+		_ = b.Hash()
+		testserdes.EncodeDecodeBinary(t, b, new(Block))
+	})
+
+	t.Run("bad contents count", func(t *testing.T) {
+		b := newDumbBlock()
+		b.Transactions = make([]*transaction.Transaction, MaxContentsPerBlock)
+		for i := range b.Transactions {
+			b.Transactions[i] = &transaction.Transaction{
+				Script: []byte("my_pretty_script"),
+			}
+		}
+		_ = b.Hash()
+		data, err := testserdes.EncodeBinary(b)
+		require.NoError(t, err)
+
+		require.True(t, errors.Is(testserdes.DecodeBinary(data, new(Block)), ErrMaxContentsPerBlock))
+	})
+}

pkg/core/native/policy.go

@@ -8,6 +8,7 @@ import (
 	"sort"
 	"sync"
 
+	"github.com/nspcc-dev/neo-go/pkg/core/block"
 	"github.com/nspcc-dev/neo-go/pkg/core/dao"
 	"github.com/nspcc-dev/neo-go/pkg/core/interop"
 	"github.com/nspcc-dev/neo-go/pkg/core/interop/runtime"
@@ -30,6 +31,8 @@ const (
 	defaultMaxBlockSystemFee       = 9000 * GASFactor
 	// minBlockSystemFee is the minimum allowed system fee per block.
 	minBlockSystemFee = 4007600
+	// maxFeePerByte is the maximum allowed fee per byte value.
+	maxFeePerByte = 100_000_000
 )
 
 var (
@@ -311,6 +314,10 @@ func (p *Policy) GetBlockedAccountsInternal(dao dao.DAO) (BlockedAccounts, error
 // setMaxTransactionsPerBlock is Policy contract method and  sets the upper limit
 // of transactions per block.
 func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackitem.Item) stackitem.Item {
+	value := uint32(toBigInt(args[0]).Int64())
+	if value > block.MaxTransactionsPerBlock {
+		panic(fmt.Errorf("MaxTransactionsPerBlock cannot exceed the maximum allowed transactions per block = %d", block.MaxTransactionsPerBlock))
+	}
 	ok, err := p.checkValidators(ic)
 	if err != nil {
 		panic(err)
@@ -318,7 +325,6 @@ func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackite
 	if !ok {
 		return stackitem.NewBool(false)
 	}
-	value := uint32(toBigInt(args[0]).Int64())
 	p.lock.Lock()
 	defer p.lock.Unlock()
 	err = p.setUint32WithKey(ic.DAO, maxTransactionsPerBlockKey, value)
@@ -331,6 +337,10 @@ func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackite
 
 // setMaxBlockSize is Policy contract method and sets maximum block size.
 func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) stackitem.Item {
+	value := uint32(toBigInt(args[0]).Int64())
+	if value > payload.MaxSize {
+		panic(fmt.Errorf("MaxBlockSize cannot be more than the maximum payload size = %d", payload.MaxSize))
+	}
 	ok, err := p.checkValidators(ic)
 	if err != nil {
 		panic(err)
@@ -338,10 +348,6 @@ func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) sta
 	if !ok {
 		return stackitem.NewBool(false)
 	}
-	value := uint32(toBigInt(args[0]).Int64())
-	if payload.MaxSize <= value {
-		return stackitem.NewBool(false)
-	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
 	err = p.setUint32WithKey(ic.DAO, maxBlockSizeKey, value)
@@ -354,6 +360,10 @@ func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) sta
 
 // setFeePerByte is Policy contract method and sets transaction's fee per byte.
 func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stackitem.Item {
+	value := toBigInt(args[0]).Int64()
+	if value < 0 || value > maxFeePerByte {
+		panic(fmt.Errorf("FeePerByte shouldn't be negative or greater than %d", maxFeePerByte))
+	}
 	ok, err := p.checkValidators(ic)
 	if err != nil {
 		panic(err)
@@ -361,7 +371,6 @@ func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stack
 	if !ok {
 		return stackitem.NewBool(false)
 	}
-	value := toBigInt(args[0]).Int64()
 	p.lock.Lock()
 	defer p.lock.Unlock()
 	err = p.setInt64WithKey(ic.DAO, feePerByteKey, value)
@@ -374,6 +383,10 @@ func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stack
 
 // setMaxBlockSystemFee is Policy contract method and sets the maximum system fee per block.
 func (p *Policy) setMaxBlockSystemFee(ic *interop.Context, args []stackitem.Item) stackitem.Item {
+	value := toBigInt(args[0]).Int64()
+	if value <= minBlockSystemFee {
+		panic(fmt.Errorf("MaxBlockSystemFee cannot be less then %d", minBlockSystemFee))
+	}
 	ok, err := p.checkValidators(ic)
 	if err != nil {
 		panic(err)
@@ -381,10 +394,6 @@ func (p *Policy) setMaxBlockSystemFee(ic *interop.Context, args []stackitem.Item
 	if !ok {
 		return stackitem.NewBool(false)
 	}
-	value := toBigInt(args[0]).Int64()
-	if value <= minBlockSystemFee {
-		return stackitem.NewBool(false)
-	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
 	err = p.setInt64WithKey(ic.DAO, maxBlockSystemFeeKey, value)

pkg/core/native_policy_test.go

@@ -5,11 +5,13 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/nspcc-dev/neo-go/pkg/core/block"
 	"github.com/nspcc-dev/neo-go/pkg/core/native"
 	"github.com/nspcc-dev/neo-go/pkg/core/state"
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/encoding/bigint"
 	"github.com/nspcc-dev/neo-go/pkg/io"
+	"github.com/nspcc-dev/neo-go/pkg/network/payload"
 	"github.com/nspcc-dev/neo-go/pkg/util"
 	"github.com/nspcc-dev/neo-go/pkg/vm"
 	"github.com/nspcc-dev/neo-go/pkg/vm/emit"
@@ -41,6 +43,12 @@ func TestMaxTransactionsPerBlock(t *testing.T) {
 		n := chain.contracts.Policy.GetMaxTransactionsPerBlockInternal(chain.dao)
 		require.Equal(t, 1024, int(n))
 	})
+
+	t.Run("set, too big value", func(t *testing.T) {
+		res, err := invokeNativePolicyMethod(chain, "setMaxTransactionsPerBlock", bigint.ToBytes(big.NewInt(block.MaxContentsPerBlock)))
+		require.NoError(t, err)
+		checkFAULTState(t, res)
+	})
 }
 
 func TestMaxBlockSize(t *testing.T) {
@@ -69,6 +77,12 @@ func TestMaxBlockSize(t *testing.T) {
 		checkResult(t, res, stackitem.NewBigInteger(big.NewInt(102400)))
 		require.NoError(t, chain.persist())
 	})
+
+	t.Run("set, too big value", func(t *testing.T) {
+		res, err := invokeNativePolicyMethod(chain, "setMaxBlockSize", bigint.ToBytes(big.NewInt(payload.MaxSize+1)))
+		require.NoError(t, err)
+		checkFAULTState(t, res)
+	})
 }
 
 func TestFeePerByte(t *testing.T) {
@@ -95,6 +109,18 @@ func TestFeePerByte(t *testing.T) {
 		n := chain.contracts.Policy.GetFeePerByteInternal(chain.dao)
 		require.Equal(t, 1024, int(n))
 	})
+
+	t.Run("set, negative value", func(t *testing.T) {
+		res, err := invokeNativePolicyMethod(chain, "setFeePerByte", bigint.ToBytes(big.NewInt(-1)))
+		require.NoError(t, err)
+		checkFAULTState(t, res)
+	})
+
+	t.Run("set, too big value", func(t *testing.T) {
+		res, err := invokeNativePolicyMethod(chain, "setFeePerByte", bigint.ToBytes(big.NewInt(100_000_000+1)))
+		require.NoError(t, err)
+		checkFAULTState(t, res)
+	})
 }
 
 func TestBlockSystemFee(t *testing.T) {
@@ -116,7 +142,7 @@ func TestBlockSystemFee(t *testing.T) {
 	t.Run("set, too low fee", func(t *testing.T) {
 		res, err := invokeNativePolicyMethod(chain, "setMaxBlockSystemFee", bigint.ToBytes(big.NewInt(4007600)))
 		require.NoError(t, err)
-		checkResult(t, res, stackitem.NewBool(false))
+		checkFAULTState(t, res)
 	})
 
 	t.Run("set, success", func(t *testing.T) {
@@ -251,3 +277,7 @@ func checkResult(t *testing.T, result *state.AppExecResult, expected stackitem.I
 	require.Equal(t, 1, len(result.Stack))
 	require.Equal(t, expected, result.Stack[0])
 }
+
+func checkFAULTState(t *testing.T, result *state.AppExecResult) {
+	require.Equal(t, vm.FaultState, result.VMState)
+}

pkg/network/payload/merkleblock.go

@@ -19,14 +19,18 @@ func (m *MerkleBlock) DecodeBinary(br *io.BinReader) {
 	m.Base = &block.Base{}
 	m.Base.DecodeBinary(br)
 
-	m.TxCount = int(br.ReadVarUint())
+	txCount := int(br.ReadVarUint())
-	br.ReadArray(&m.Hashes)
+	if txCount > block.MaxContentsPerBlock {
-	m.Flags = br.ReadVarBytes()
+		br.Err = block.ErrMaxContentsPerBlock
+		return
+	}
+	m.TxCount = txCount
+	br.ReadArray(&m.Hashes, m.TxCount)
+	m.Flags = br.ReadVarBytes((txCount + 7) / 8)
 }
 
 // EncodeBinary implements Serializable interface.
 func (m *MerkleBlock) EncodeBinary(bw *io.BinWriter) {
-	m.Base = &block.Base{}
 	m.Base.EncodeBinary(bw)
 
 	bw.WriteVarUint(uint64(m.TxCount))

pkg/network/payload/merkleblock_test.go

@@ -0,0 +1,71 @@
+package payload
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/nspcc-dev/neo-go/pkg/core/block"
+	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
+	"github.com/nspcc-dev/neo-go/pkg/crypto/hash"
+	"github.com/nspcc-dev/neo-go/pkg/internal/testserdes"
+	"github.com/nspcc-dev/neo-go/pkg/util"
+)
+
+func newDumbBlock() *block.Base {
+	return &block.Base{
+		Version:       0,
+		PrevHash:      hash.Sha256([]byte("a")),
+		MerkleRoot:    hash.Sha256([]byte("b")),
+		Timestamp:     100500,
+		Index:         1,
+		NextConsensus: hash.Hash160([]byte("a")),
+		Script: transaction.Witness{
+			VerificationScript: []byte{0x51}, // PUSH1
+			InvocationScript:   []byte{0x61}, // NOP
+		},
+	}
+}
+
+func TestMerkleBlock_EncodeDecodeBinary(t *testing.T) {
+	t.Run("positive", func(t *testing.T) {
+		b := newDumbBlock()
+		_ = b.Hash()
+		expected := &MerkleBlock{
+			Base:    b,
+			TxCount: 0,
+			Hashes:  []util.Uint256{},
+			Flags:   []byte{},
+		}
+		testserdes.EncodeDecodeBinary(t, expected, new(MerkleBlock))
+	})
+
+	t.Run("bad contents count", func(t *testing.T) {
+		b := newDumbBlock()
+		_ = b.Hash()
+		expected := &MerkleBlock{
+			Base:    b,
+			TxCount: block.MaxContentsPerBlock + 1,
+			Hashes:  make([]util.Uint256, block.MaxContentsPerBlock),
+			Flags:   []byte{},
+		}
+		data, err := testserdes.EncodeBinary(expected)
+		require.NoError(t, err)
+		require.True(t, errors.Is(block.ErrMaxContentsPerBlock, testserdes.DecodeBinary(data, new(MerkleBlock))))
+	})
+
+	t.Run("bad flags size", func(t *testing.T) {
+		b := newDumbBlock()
+		_ = b.Hash()
+		expected := &MerkleBlock{
+			Base:    b,
+			TxCount: 0,
+			Hashes:  []util.Uint256{},
+			Flags:   []byte{1, 2, 3, 4, 5},
+		}
+		data, err := testserdes.EncodeBinary(expected)
+		require.NoError(t, err)
+		require.Error(t, testserdes.DecodeBinary(data, new(MerkleBlock)))
+	})
+}