From e9ea89b4e39931b98f84173df936eadabd8133f4 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Mon, 8 Feb 2021 15:11:31 +0300
Subject: [PATCH] manifest: add group signature length check

Refs. #1699.
---
 pkg/core/interop_system_test.go             |  2 +-
 pkg/crypto/keys/publickey.go                |  5 ++++-
 pkg/smartcontract/manifest/group.go         |  6 ++++++
 pkg/smartcontract/manifest/group_test.go    | 18 ++++++++++++++++++
 pkg/smartcontract/manifest/manifest_test.go |  2 +-
 pkg/smartcontract/manifest/method_test.go   |  5 +++--
 6 files changed, 33 insertions(+), 5 deletions(-)
 create mode 100644 pkg/smartcontract/manifest/group_test.go

diff --git a/pkg/core/interop_system_test.go b/pkg/core/interop_system_test.go
index 7bbe96958..c7e893467 100644
--- a/pkg/core/interop_system_test.go
+++ b/pkg/core/interop_system_test.go
@@ -910,7 +910,7 @@ func TestRuntimeCheckWitness(t *testing.T) {
 						Hash: contractScriptHash,
 						NEF:  *ne,
 						Manifest: manifest.Manifest{
-							Groups: []manifest.Group{{PublicKey: pk.PublicKey()}},
+							Groups: []manifest.Group{{PublicKey: pk.PublicKey(), Signature: make([]byte, keys.SignatureLen)}},
 						},
 					}
 					require.NoError(t, bc.contracts.Management.PutContractState(ic.DAO, contractState))
diff --git a/pkg/crypto/keys/publickey.go b/pkg/crypto/keys/publickey.go
index 7eac8931f..eb941fd88 100644
--- a/pkg/crypto/keys/publickey.go
+++ b/pkg/crypto/keys/publickey.go
@@ -24,6 +24,9 @@ import (
 // coordLen is the number of bytes in serialized X or Y coordinate.
 const coordLen = 32
 
+// SignatureLen is the length of standard signature for 256-bit EC key.
+const SignatureLen = 64
+
 // PublicKeys is a list of public keys.
 type PublicKeys []*PublicKey
 
@@ -333,7 +336,7 @@ func (p *PublicKey) Address() string {
 // Verify returns true if the signature is valid and corresponds
 // to the hash and public key.
 func (p *PublicKey) Verify(signature []byte, hash []byte) bool {
-	if p.X == nil || p.Y == nil || len(signature) != 64 {
+	if p.X == nil || p.Y == nil || len(signature) != SignatureLen {
 		return false
 	}
 	rBytes := new(big.Int).SetBytes(signature[0:32])
diff --git a/pkg/smartcontract/manifest/group.go b/pkg/smartcontract/manifest/group.go
index 96d02eaf0..30b39dfee 100644
--- a/pkg/smartcontract/manifest/group.go
+++ b/pkg/smartcontract/manifest/group.go
@@ -57,6 +57,9 @@ func (g *Group) UnmarshalJSON(data []byte) error {
 		return err
 	}
 	g.PublicKey = pub
+	if len(aux.Signature) != keys.SignatureLen {
+		return errors.New("wrong signature length")
+	}
 	g.Signature = aux.Signature
 	return nil
 }
@@ -90,6 +93,9 @@ func (g *Group) FromStackItem(item stackitem.Item) error {
 	if err != nil {
 		return err
 	}
+	if len(sig) != keys.SignatureLen {
+		return errors.New("wrong signature length")
+	}
 	g.Signature = sig
 	return nil
 }
diff --git a/pkg/smartcontract/manifest/group_test.go b/pkg/smartcontract/manifest/group_test.go
new file mode 100644
index 000000000..833e2ae9f
--- /dev/null
+++ b/pkg/smartcontract/manifest/group_test.go
@@ -0,0 +1,18 @@
+package manifest
+
+import (
+	"testing"
+
+	"github.com/nspcc-dev/neo-go/internal/testserdes"
+	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGroupJSONInOut(t *testing.T) {
+	priv, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+	pub := priv.PublicKey()
+	sig := make([]byte, keys.SignatureLen)
+	g := Group{pub, sig}
+	testserdes.MarshalUnmarshalJSON(t, &g, new(Group))
+}
diff --git a/pkg/smartcontract/manifest/manifest_test.go b/pkg/smartcontract/manifest/manifest_test.go
index fdf594d32..f5d03ca4d 100644
--- a/pkg/smartcontract/manifest/manifest_test.go
+++ b/pkg/smartcontract/manifest/manifest_test.go
@@ -199,7 +199,7 @@ func TestManifestToStackItem(t *testing.T) {
 			},
 			Groups: []Group{{
 				PublicKey: pk.PublicKey(),
-				Signature: []byte{1, 2, 3},
+				Signature: make([]byte, keys.SignatureLen),
 			}},
 			Permissions:        []Permission{*NewPermission(PermissionWildcard)},
 			SupportedStandards: []string{"NEP-17"},
diff --git a/pkg/smartcontract/manifest/method_test.go b/pkg/smartcontract/manifest/method_test.go
index 658b2894a..c05e517f8 100644
--- a/pkg/smartcontract/manifest/method_test.go
+++ b/pkg/smartcontract/manifest/method_test.go
@@ -117,11 +117,11 @@ func TestGroup_ToStackItemFromStackItem(t *testing.T) {
 	pk, _ := keys.NewPrivateKey()
 	g := &Group{
 		PublicKey: pk.PublicKey(),
-		Signature: []byte{1, 2, 3},
+		Signature: make([]byte, keys.SignatureLen),
 	}
 	expected := stackitem.NewStruct([]stackitem.Item{
 		stackitem.NewByteArray(pk.PublicKey().Bytes()),
-		stackitem.NewByteArray([]byte{1, 2, 3}),
+		stackitem.NewByteArray(make([]byte, keys.SignatureLen)),
 	})
 	CheckToFromStackItem(t, g, expected)
 }
@@ -134,6 +134,7 @@ func TestGroup_FromStackItemErrors(t *testing.T) {
 		"invalid pub type":  stackitem.NewStruct([]stackitem.Item{stackitem.NewInterop(nil), stackitem.Null{}}),
 		"invalid pub bytes": stackitem.NewStruct([]stackitem.Item{stackitem.NewByteArray([]byte{1}), stackitem.Null{}}),
 		"invalid sig type":  stackitem.NewStruct([]stackitem.Item{stackitem.NewByteArray(pk.Bytes()), stackitem.NewInterop(nil)}),
+		"invalid sig len":   stackitem.NewStruct([]stackitem.Item{stackitem.NewByteArray(pk.Bytes()), stackitem.NewByteArray([]byte{1})}),
 	}
 	for name, errCase := range errCases {
 		t.Run(name, func(t *testing.T) {