From dcc58b7c445508885c332731e472b962e641f37e Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <evgeniy@nspcc.ru>
Date: Wed, 2 Dec 2020 14:49:22 +0300
Subject: [PATCH] core: validate manifest before incrementing ID in
 `Contract.Create`

Also add more tests.
---
 pkg/core/interop_neo.go         |  6 ++--
 pkg/core/interop_system_test.go | 51 +++++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go
index 67a8c3d6f..a65902903 100644
--- a/pkg/core/interop_neo.go
+++ b/pkg/core/interop_neo.go
@@ -115,13 +115,13 @@ func contractCreate(ic *interop.Context) error {
 	if contract != nil && err == nil {
 		return errors.New("contract already exists")
 	}
+	if !manif.IsValid(h) {
+		return errors.New("failed to check contract script hash against manifest")
+	}
 	id, err := ic.DAO.GetAndUpdateNextContractID()
 	if err != nil {
 		return err
 	}
-	if !manif.IsValid(h) {
-		return errors.New("failed to check contract script hash against manifest")
-	}
 	newcontract := &state.Contract{
 		ID:       id,
 		Hash:     h,
diff --git a/pkg/core/interop_system_test.go b/pkg/core/interop_system_test.go
index 42c62e6bf..b0e1788f6 100644
--- a/pkg/core/interop_system_test.go
+++ b/pkg/core/interop_system_test.go
@@ -589,14 +589,24 @@ func TestContractCreate(t *testing.T) {
 
 	// nef.NewFile() cares about version a lot.
 	config.Version = "0.90.0-test"
+
+	ne, err := nef.NewFile(cs.Script)
+	require.NoError(t, err)
+	neb, err := ne.Bytes()
+	require.NoError(t, err)
+	priv, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+	sender := util.Uint160{1, 2, 3}
+	h := state.CreateContractHash(sender, ne.Script)
+	sig := priv.Sign(h.BytesBE())
+	cs.Manifest.Groups = []manifest.Group{{
+		PublicKey: priv.PublicKey(),
+		Signature: sig,
+	}}
+	m, err := json.Marshal(cs.Manifest)
+	require.NoError(t, err)
 	putArgsOnStack := func() {
-		manifest, err := json.Marshal(cs.Manifest)
-		require.NoError(t, err)
-		ne, err := nef.NewFile(cs.Script)
-		require.NoError(t, err)
-		neb, err := ne.Bytes()
-		require.NoError(t, err)
-		v.Estack().PushVal(manifest)
+		v.Estack().PushVal(m)
 		v.Estack().PushVal(neb)
 	}
 
@@ -607,11 +617,36 @@ func TestContractCreate(t *testing.T) {
 	})
 
 	ic.Tx = transaction.New(netmode.UnitTestNet, []byte{1}, 0)
-	var sender = util.Uint160{1, 2, 3}
 	ic.Tx.Signers = append(ic.Tx.Signers, transaction.Signer{Account: sender})
 	cs.ID = 0
 	cs.Hash = state.CreateContractHash(sender, cs.Script)
 
+	t.Run("missing NEF", func(t *testing.T) {
+		v.Estack().PushVal(m)
+		v.Estack().PushVal(stackitem.Null{})
+		require.Error(t, contractCreate(ic))
+	})
+	t.Run("missing manifest", func(t *testing.T) {
+		v.Estack().PushVal(stackitem.Null{})
+		v.Estack().PushVal(neb)
+		require.Error(t, contractCreate(ic))
+	})
+	t.Run("invalid manifest (empty)", func(t *testing.T) {
+		v.Estack().PushVal([]byte{})
+		v.Estack().PushVal(neb)
+		require.Error(t, contractCreate(ic))
+	})
+
+	t.Run("invalid manifest (group signature)", func(t *testing.T) {
+		cs.Manifest.Groups[0].Signature = make([]byte, 11)
+		rawManif, err := json.Marshal(cs.Manifest)
+		require.NoError(t, err)
+		v.Estack().PushVal(rawManif)
+		v.Estack().PushVal(neb)
+		require.Error(t, contractCreate(ic))
+	})
+
+	cs.Manifest.Groups[0].Signature = sig
 	t.Run("positive", func(t *testing.T) {
 		putArgsOnStack()