core: check transaction's scripts length during decoding

pkg/core/blockchain.go

@@ -1384,14 +1384,13 @@ func (bc *Blockchain) verifyHeader(currHeader, prevHeader *block.Header) error {
 
 // Various errors that could be returned upon verification.
 var (
-	ErrTxExpired           = errors.New("transaction has expired")
+	ErrTxExpired         = errors.New("transaction has expired")
-	ErrInsufficientFunds   = errors.New("insufficient funds")
+	ErrInsufficientFunds = errors.New("insufficient funds")
-	ErrTxSmallNetworkFee   = errors.New("too small network fee")
+	ErrTxSmallNetworkFee = errors.New("too small network fee")
-	ErrTxTooBig            = errors.New("too big transaction")
+	ErrTxTooBig          = errors.New("too big transaction")
-	ErrMemPoolConflict     = errors.New("invalid transaction due to conflicts with the memory pool")
+	ErrMemPoolConflict   = errors.New("invalid transaction due to conflicts with the memory pool")
-	ErrInvalidScript       = errors.New("invalid script")
+	ErrInvalidScript     = errors.New("invalid script")
-	ErrTxInvalidWitnessNum = errors.New("number of signers doesn't match witnesses")
+	ErrInvalidAttribute  = errors.New("invalid attribute")
-	ErrInvalidAttribute    = errors.New("invalid attribute")
 )
 
 // verifyAndPoolTx verifies whether a transaction is bonafide or not and tries
@@ -1856,9 +1855,6 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa
 // not yet added into any block.
 // Golang implementation of VerifyWitnesses method in C# (https://github.com/neo-project/neo/blob/master/neo/SmartContract/Helper.cs#L87).
 func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block.Block, isPartialTx bool) error {
-	if len(t.Signers) != len(t.Scripts) {
-		return fmt.Errorf("%w: %d vs %d", ErrTxInvalidWitnessNum, len(t.Signers), len(t.Scripts))
-	}
 	interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t)
 	gasLimit := t.NetworkFee - int64(t.Size())*bc.FeePerByte()
 	if bc.P2PSigExtensionsEnabled() {

pkg/core/blockchain_test.go

@@ -439,10 +439,6 @@ func TestVerifyTx(t *testing.T) {
 		err := bc.PoolTx(tx2)
 		require.True(t, errors.Is(err, ErrMemPoolConflict))
 	})
-	t.Run("NotEnoughWitnesses", func(t *testing.T) {
-		tx := bc.newTestTx(h, testScript)
-		checkErr(t, ErrTxInvalidWitnessNum, tx)
-	})
 	t.Run("InvalidWitnessHash", func(t *testing.T) {
 		tx := bc.newTestTx(h, testScript)
 		require.NoError(t, accs[0].SignTx(tx))

pkg/core/interop_neo_test.go

@@ -280,8 +280,8 @@ func createVMAndContractState(t *testing.T) (*vm.VM, *state.Contract, *interop.C
 func createVMAndTX(t *testing.T) (*vm.VM, *transaction.Transaction, *interop.Context, *Blockchain) {
 	script := []byte{byte(opcode.PUSH1), byte(opcode.RET)}
 	tx := transaction.New(netmode.UnitTestNet, script, 0)
-
 	tx.Signers = []transaction.Signer{{Account: util.Uint160{1, 2, 3, 4}}}
+	tx.Scripts = []transaction.Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	chain := newTestChain(t)
 	d := dao.NewSimple(storage.NewMemoryStore(), netmode.UnitTestNet, chain.config.StateRootInHeader)
 	context := chain.newInteropContext(trigger.Application, d, nil, tx)

pkg/core/transaction/transaction.go

@@ -31,6 +31,9 @@ const (
 	DummyVersion = 255
 )
 
+// ErrInvalidWitnessNum returns when the number of witnesses does not match signers.
+var ErrInvalidWitnessNum = errors.New("number of signers doesn't match witnesses")
+
 // Transaction is a process recorded in the NEO blockchain.
 type Transaction struct {
 	// The trading version which is currently 0.
@@ -170,7 +173,11 @@ func (t *Transaction) DecodeBinary(br *io.BinReader) {
 	if br.Err != nil {
 		return
 	}
-	br.ReadArray(&t.Scripts)
+	br.ReadArray(&t.Scripts, len(t.Signers))
+	if len(t.Signers) != len(t.Scripts) {
+		br.Err = fmt.Errorf("%w: %d vs %d", ErrInvalidWitnessNum, len(t.Signers), len(t.Scripts))
+		return
+	}
 
 	// Create the hash of the transaction at decode, so we dont need
 	// to do it anymore.

pkg/core/transaction/transaction_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/config/netmode"
 	"github.com/nspcc-dev/neo-go/pkg/encoding/fixedn"
 	"github.com/nspcc-dev/neo-go/pkg/util"
+	"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -77,6 +78,7 @@ func TestNew(t *testing.T) {
 	script := []byte{0x51}
 	tx := New(netmode.UnitTestNet, script, 1)
 	tx.Signers = []Signer{{Account: util.Uint160{1, 2, 3}}}
+	tx.Scripts = []Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	assert.Equal(t, int64(1), tx.SystemFee)
 	assert.Equal(t, script, tx.Script)
 	// Update hash fields to match tx2 that is gonna autoupdate them on decode.
@@ -90,6 +92,7 @@ func TestNewTransactionFromBytes(t *testing.T) {
 	tx := New(netmode.UnitTestNet, script, 1)
 	tx.NetworkFee = 123
 	tx.Signers = []Signer{{Account: util.Uint160{1, 2, 3}}}
+	tx.Scripts = []Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	data, err := testserdes.EncodeBinary(tx)
 	require.NoError(t, err)
 
@@ -118,6 +121,15 @@ func TestDecodingTXWithNoScript(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestDecodingTxWithInvalidWitnessesNumber(t *testing.T) {
+	tx := New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, 1)
+	tx.Signers = []Signer{{Account: util.Uint160{1, 2, 3}}}
+	tx.Scripts = []Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}, {InvocationScript: []byte{}, VerificationScript: []byte{}}}
+	data, err := testserdes.EncodeBinary(tx)
+	require.NoError(t, err)
+	require.True(t, errors.Is(testserdes.DecodeBinary(data, new(Transaction)), ErrInvalidWitnessNum))
+}
+
 func TestUnmarshalNeoFSTX(t *testing.T) {
 	txjson := []byte(`
 {

pkg/network/message_test.go

@@ -306,6 +306,7 @@ func newDummyBlock(height uint32, txCount int) *block.Block {
 func newDummyTx() *transaction.Transaction {
 	tx := transaction.New(netmode.UnitTestNet, random.Bytes(100), 123)
 	tx.Signers = []transaction.Signer{{Account: random.Uint160()}}
+	tx.Scripts = []transaction.Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	tx.Size()
 	tx.Hash()
 	return tx