From 284476c070cb071ba3da2bf842a7ec79e195a132 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 8 Feb 2021 21:07:05 +0300 Subject: [PATCH] manifest: add Permission and Permissions validity checks Refs. #1699. --- pkg/smartcontract/manifest/manifest.go | 4 +- pkg/smartcontract/manifest/manifest_test.go | 11 +++ pkg/smartcontract/manifest/parameter.go | 24 ++++-- pkg/smartcontract/manifest/permission.go | 80 +++++++++++++++++++ pkg/smartcontract/manifest/permission_test.go | 56 +++++++++++++ 5 files changed, 165 insertions(+), 10 deletions(-) diff --git a/pkg/smartcontract/manifest/manifest.go b/pkg/smartcontract/manifest/manifest.go index 45f300e67..10dddf9ea 100644 --- a/pkg/smartcontract/manifest/manifest.go +++ b/pkg/smartcontract/manifest/manifest.go @@ -82,10 +82,10 @@ func (m *Manifest) IsValid(hash util.Uint160) error { for _, g := range m.Groups { err = g.IsValid(hash) if err != nil { - break + return err } } - return err + return Permissions(m.Permissions).AreValid() } // ToStackItem converts Manifest to stackitem.Item. diff --git a/pkg/smartcontract/manifest/manifest_test.go b/pkg/smartcontract/manifest/manifest_test.go index 834be1b4e..3e8239f30 100644 --- a/pkg/smartcontract/manifest/manifest_test.go +++ b/pkg/smartcontract/manifest/manifest_test.go @@ -147,6 +147,17 @@ func TestIsValid(t *testing.T) { }) m.ABI.Events = m.ABI.Events[:1] + m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3})) + t.Run("valid, with permissions", func(t *testing.T) { + require.NoError(t, m.IsValid(contractHash)) + }) + + m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3})) + t.Run("invalid, with permissions", func(t *testing.T) { + require.Error(t, m.IsValid(contractHash)) + }) + m.Permissions = m.Permissions[:1] + t.Run("with groups", func(t *testing.T) { m.Groups = make([]Group, 3) pks := make([]*keys.PrivateKey, 3) diff --git a/pkg/smartcontract/manifest/parameter.go b/pkg/smartcontract/manifest/parameter.go index b12e55a7c..e739a52f5 100644 --- a/pkg/smartcontract/manifest/parameter.go +++ b/pkg/smartcontract/manifest/parameter.go @@ -85,14 +85,22 @@ func (p Parameters) AreValid() error { for i := range p { names[i] = p[i].Name } - sort.Strings(names) - for i := range names { - if i == 0 { - continue - } - if names[i] == names[i-1] { - return errors.New("duplicate parameter name") - } + if stringsHaveDups(names) { + return errors.New("duplicate parameter name") } return nil } + +// stringsHaveDups checks given set of strings for duplicates. It modifies the slice given! +func stringsHaveDups(strings []string) bool { + sort.Strings(strings) + for i := range strings { + if i == 0 { + continue + } + if strings[i] == strings[i-1] { + return true + } + } + return false +} diff --git a/pkg/smartcontract/manifest/permission.go b/pkg/smartcontract/manifest/permission.go index 831229cf6..e1c7cd6b1 100644 --- a/pkg/smartcontract/manifest/permission.go +++ b/pkg/smartcontract/manifest/permission.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "sort" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/util" @@ -36,6 +37,9 @@ type Permission struct { Methods WildStrings `json:"methods"` } +// Permissions is just an array of Permission. +type Permissions []Permission + type permissionAux struct { Contract PermissionDesc `json:"contract"` Methods WildStrings `json:"methods"` @@ -85,6 +89,82 @@ func (d *PermissionDesc) Group() *keys.PublicKey { return d.Value.(*keys.PublicKey) } +// IsValid checks if Permission is correct. +func (p *Permission) IsValid() error { + for i := range p.Methods.Value { + if p.Methods.Value[i] == "" { + return errors.New("empty method name") + } + } + if len(p.Methods.Value) < 2 { + return nil + } + names := make([]string, len(p.Methods.Value)) + copy(names, p.Methods.Value) + if stringsHaveDups(names) { + return errors.New("duplicate method names") + } + return nil +} + +// AreValid checks each Permission and ensures there are no duplicates. +func (ps Permissions) AreValid() error { + for i := range ps { + err := ps[i].IsValid() + if err != nil { + return err + } + } + if len(ps) < 2 { + return nil + } + contracts := make([]PermissionDesc, 0, len(ps)) + for i := range ps { + contracts = append(contracts, ps[i].Contract) + } + sort.Slice(contracts, func(i, j int) bool { + if contracts[i].Type < contracts[j].Type { + return true + } + if contracts[i].Type != contracts[j].Type { + return false + } + switch contracts[i].Type { + case PermissionHash: + return contracts[i].Hash().Less(contracts[j].Hash()) + case PermissionGroup: + return contracts[i].Group().Cmp(contracts[j].Group()) < 0 + } + return false + }) + for i := range contracts { + if i == 0 { + continue + } + j := i - 1 + if contracts[i].Type != contracts[j].Type { + continue + } + var bad bool + switch contracts[i].Type { + case PermissionWildcard: + bad = true + case PermissionHash: + if contracts[i].Hash() == contracts[j].Hash() { + bad = true + } + case PermissionGroup: + if contracts[i].Group().Cmp(contracts[j].Group()) == 0 { + bad = true + } + } + if bad { + return errors.New("duplicate contracts") + } + } + return nil +} + // IsAllowed checks if method is allowed to be executed. func (p *Permission) IsAllowed(hash util.Uint160, m *Manifest, method string) bool { switch p.Contract.Type { diff --git a/pkg/smartcontract/manifest/permission_test.go b/pkg/smartcontract/manifest/permission_test.go index 97eebb007..1016bb542 100644 --- a/pkg/smartcontract/manifest/permission_test.go +++ b/pkg/smartcontract/manifest/permission_test.go @@ -21,6 +21,62 @@ func TestNewPermission(t *testing.T) { require.Panics(t, func() { NewPermission(PermissionGroup, util.Uint160{}) }) } +func TestPermissionIsValid(t *testing.T) { + p := Permission{} + require.NoError(t, p.IsValid()) + + p.Methods.Add("") + require.Error(t, p.IsValid()) + + p.Methods.Value = nil + p.Methods.Add("qwerty") + require.NoError(t, p.IsValid()) + + p.Methods.Add("poiuyt") + require.NoError(t, p.IsValid()) + + p.Methods.Add("qwerty") + require.Error(t, p.IsValid()) +} + +func TestPermissionsAreValid(t *testing.T) { + p := Permissions{} + require.NoError(t, p.AreValid()) + + p = append(p, Permission{Methods: WildStrings{Value: []string{""}}}) + require.Error(t, p.AreValid()) + + p = p[:0] + p = append(p, *NewPermission(PermissionHash, util.Uint160{1, 2, 3})) + require.NoError(t, p.AreValid()) + + priv0, err := keys.NewPrivateKey() + require.NoError(t, err) + priv1, err := keys.NewPrivateKey() + require.NoError(t, err) + + p = append(p, *NewPermission(PermissionGroup, priv0.PublicKey())) + require.NoError(t, p.AreValid()) + + p = append(p, *NewPermission(PermissionGroup, priv1.PublicKey())) + require.NoError(t, p.AreValid()) + + p = append(p, *NewPermission(PermissionWildcard)) + require.NoError(t, p.AreValid()) + + p = append(p, *NewPermission(PermissionHash, util.Uint160{3, 2, 1})) + require.NoError(t, p.AreValid()) + + p = append(p, *NewPermission(PermissionWildcard)) + require.Error(t, p.AreValid()) + + p = append(p[:len(p)-1], *NewPermission(PermissionHash, util.Uint160{1, 2, 3})) + require.Error(t, p.AreValid()) + + p = append(p[:len(p)-1], *NewPermission(PermissionGroup, priv0.PublicKey())) + require.Error(t, p.AreValid()) +} + func TestPermission_MarshalJSON(t *testing.T) { t.Run("wildcard", func(t *testing.T) { expected := NewPermission(PermissionWildcard)