From dd05cae506d352c59f550d334b92b2ab0d73678b Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Tue, 9 Feb 2021 22:28:36 +0300
Subject: [PATCH] core: check all transaction scripts

Refs. #1699.
---
 pkg/core/blockchain.go      | 20 ++++++++++++++++++-
 pkg/core/blockchain_test.go | 38 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go
index 3765fe4ac..4fd1a5857 100644
--- a/pkg/core/blockchain.go
+++ b/pkg/core/blockchain.go
@@ -1380,6 +1380,7 @@ var (
 	ErrTxSmallNetworkFee   = errors.New("too small network fee")
 	ErrTxTooBig            = errors.New("too big transaction")
 	ErrMemPoolConflict     = errors.New("invalid transaction due to conflicts with the memory pool")
+	ErrInvalidScript       = errors.New("invalid script")
 	ErrTxInvalidWitnessNum = errors.New("number of signers doesn't match witnesses")
 	ErrInvalidAttribute    = errors.New("invalid attribute")
 )
@@ -1387,6 +1388,13 @@ var (
 // verifyAndPoolTx verifies whether a transaction is bonafide or not and tries
 // to add it to the mempool given.
 func (bc *Blockchain) verifyAndPoolTx(t *transaction.Transaction, pool *mempool.Pool, feer mempool.Feer, data ...interface{}) error {
+	// This code can technically be moved out of here, because it doesn't
+	// really require a chain lock.
+	err := vm.IsScriptCorrect(t.Script, nil)
+	if err != nil {
+		return fmt.Errorf("%w: %v", ErrInvalidScript, err)
+	}
+
 	height := bc.BlockHeight()
 	isPartialTx := data != nil
 	if t.ValidUntilBlock <= height || !isPartialTx && t.ValidUntilBlock > height+transaction.MaxValidUntilBlockIncrement {
@@ -1424,7 +1432,7 @@ func (bc *Blockchain) verifyAndPoolTx(t *transaction.Transaction, pool *mempool.
 			return err
 		}
 	}
-	err := bc.verifyTxWitnesses(t, nil, isPartialTx)
+	err = bc.verifyTxWitnesses(t, nil, isPartialTx)
 	if err != nil {
 		return err
 	}
@@ -1728,7 +1736,9 @@ var (
 	ErrWitnessHashMismatch         = errors.New("witness hash mismatch")
 	ErrNativeContractWitness       = errors.New("native contract witness must have empty verification script")
 	ErrVerificationFailed          = errors.New("signature check failed")
+	ErrInvalidInvocation           = errors.New("invalid invocation script")
 	ErrInvalidSignature            = fmt.Errorf("%w: invalid signature", ErrVerificationFailed)
+	ErrInvalidVerification         = errors.New("invalid verification script")
 	ErrUnknownVerificationContract = errors.New("unknown verification contract")
 	ErrInvalidVerificationContract = errors.New("verification contract is missing `verify` method")
 )
@@ -1744,6 +1754,10 @@ func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160,
 		if bc.contracts.ByHash(hash) != nil {
 			return ErrNativeContractWitness
 		}
+		err := vm.IsScriptCorrect(witness.VerificationScript, nil)
+		if err != nil {
+			return fmt.Errorf("%w: %v", ErrInvalidVerification, err)
+		}
 		v.LoadScriptWithFlags(witness.VerificationScript, callflag.ReadStates)
 	} else {
 		cs, err := ic.GetContract(hash)
@@ -1765,6 +1779,10 @@ func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160,
 		}
 	}
 	if len(witness.InvocationScript) != 0 {
+		err := vm.IsScriptCorrect(witness.InvocationScript, nil)
+		if err != nil {
+			return fmt.Errorf("%w: %v", ErrInvalidInvocation, err)
+		}
 		v.LoadScript(witness.InvocationScript)
 		if isNative {
 			if err := v.StepOut(); err != nil {
diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go
index 7d72223c1..bb6d2b4df 100644
--- a/pkg/core/blockchain_test.go
+++ b/pkg/core/blockchain_test.go
@@ -390,6 +390,42 @@ func TestVerifyTx(t *testing.T) {
 			require.Equal(t, expectedNetFee, bc.FeePerByte()*int64(actualSize)+gasConsumed)
 		})
 	})
+	t.Run("InvalidTxScript", func(t *testing.T) {
+		tx := bc.newTestTx(h, testScript)
+		tx.Script = append(tx.Script, 0xff)
+		require.NoError(t, accs[0].SignTx(tx))
+		checkErr(t, ErrInvalidScript, tx)
+	})
+	t.Run("InvalidVerificationScript", func(t *testing.T) {
+		tx := bc.newTestTx(h, testScript)
+		verif := []byte{byte(opcode.JMP), 3, 0xff, byte(opcode.PUSHT)}
+		tx.Signers = append(tx.Signers, transaction.Signer{
+			Account: hash.Hash160(verif),
+			Scopes:  transaction.Global,
+		})
+		tx.NetworkFee += 1000000
+		require.NoError(t, accs[0].SignTx(tx))
+		tx.Scripts = append(tx.Scripts, transaction.Witness{
+			InvocationScript:   []byte{},
+			VerificationScript: verif,
+		})
+		checkErr(t, ErrInvalidVerification, tx)
+	})
+	t.Run("InvalidInvocationScript", func(t *testing.T) {
+		tx := bc.newTestTx(h, testScript)
+		verif := []byte{byte(opcode.PUSHT)}
+		tx.Signers = append(tx.Signers, transaction.Signer{
+			Account: hash.Hash160(verif),
+			Scopes:  transaction.Global,
+		})
+		tx.NetworkFee += 1000000
+		require.NoError(t, accs[0].SignTx(tx))
+		tx.Scripts = append(tx.Scripts, transaction.Witness{
+			InvocationScript:   []byte{byte(opcode.JMP), 3, 0xff},
+			VerificationScript: verif,
+		})
+		checkErr(t, ErrInvalidInvocation, tx)
+	})
 	t.Run("Conflict", func(t *testing.T) {
 		balance := bc.GetUtilityTokenBalance(h).Int64()
 		tx := bc.newTestTx(h, testScript)
@@ -583,7 +619,7 @@ func TestVerifyTx(t *testing.T) {
 			})
 			t.Run("InvalidScript", func(t *testing.T) {
 				tx := getOracleTx(t)
-				tx.Script[0] = ^tx.Script[0]
+				tx.Script = append(tx.Script, byte(opcode.NOP))
 				require.NoError(t, oracleAcc.SignTx(tx))
 				checkErr(t, ErrInvalidAttribute, tx)
 			})