From aa76383fa7ba7cd9c07ba57a8b2da68411dcf13a Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Fri, 25 Jun 2021 10:54:00 +0300 Subject: [PATCH] compiler: extend permission check to runtime hashes If a method is known at compile time we can still check if it is present in the list of methods of at least one contract. Signed-off-by: Evgeniy Stratonikov --- pkg/compiler/compiler.go | 13 ++++++-- pkg/compiler/compiler_test.go | 17 ++++++---- pkg/compiler/inline.go | 39 ++++++++++------------- pkg/rpc/server/testdata/test_contract.yml | 2 ++ 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index d73ad9e84..b930827ed 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -16,6 +16,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest/standard" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" + "github.com/nspcc-dev/neo-go/pkg/util" "golang.org/x/tools/go/loader" ) @@ -304,11 +305,13 @@ func CreateManifest(di *DebugInfo, o *Options) (*manifest.Manifest, error) { // Thus only basic checks are performed. for h, methods := range di.InvokedContracts { + knownHash := !h.Equals(util.Uint160{}) + methodLoop: for _, m := range methods { for _, p := range o.Permissions { // Group or wildcard permission is ok to try. - if p.Contract.Type == manifest.PermissionHash && !p.Contract.Hash().Equals(h) { + if knownHash && p.Contract.Type == manifest.PermissionHash && !p.Contract.Hash().Equals(h) { continue } @@ -317,8 +320,12 @@ func CreateManifest(di *DebugInfo, o *Options) (*manifest.Manifest, error) { } } - return nil, fmt.Errorf("method '%s' of contract %s is invoked but"+ - " corresponding permission is missing", m, h.StringLE()) + if knownHash { + return nil, fmt.Errorf("method '%s' of contract %s is invoked but"+ + " corresponding permission is missing", m, h.StringLE()) + } + return nil, fmt.Errorf("method '%s' is invoked but"+ + " corresponding permission is missing", m) } } } diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index 3188e6812..0fed351a4 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/nspcc-dev/neo-go/internal/random" "github.com/nspcc-dev/neo-go/pkg/compiler" "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" @@ -263,9 +264,7 @@ func TestInvokedContractsPermissons(t *testing.T) { contract.Call(interop.Hash160(hash), runtimeMethod, contract.All) contract.Call(runtimeHash, "someMethod", contract.All) contract.Call(interop.Hash160(runtimeHash), "someMethod", contract.All) - contract.Call(runh.RuntimeHash(), "method3", contract.All) - contract.Call(runh.RuntimeHashArgs(hash), "method3", contract.All) - contract.Call(invoke(hash), "method3", contract.All) + contract.Call(runh.RuntimeHash(), "method4", contract.All) }`, hashStr) _, di, err := compiler.CompileWithOptions("permissionTest", strings.NewReader(src), &compiler.Options{}) @@ -281,12 +280,17 @@ func TestInvokedContractsPermissons(t *testing.T) { require.Error(t, testCompile(t, di, false, *p)) require.NoError(t, testCompile(t, di, true, *p)) + pr := manifest.NewPermission(manifest.PermissionHash, random.Uint160()) + pr.Methods.Add("someMethod") + pr.Methods.Add("method4") + t.Run("wildcard", func(t *testing.T) { pw := manifest.NewPermission(manifest.PermissionWildcard) require.NoError(t, testCompile(t, di, false, *p, *pw)) pw.Methods.Add("method2") - require.NoError(t, testCompile(t, di, false, *p, *pw)) + require.Error(t, testCompile(t, di, false, *p, *pw)) + require.NoError(t, testCompile(t, di, false, *p, *pw, *pr)) }) t.Run("group", func(t *testing.T) { @@ -295,10 +299,11 @@ func TestInvokedContractsPermissons(t *testing.T) { require.NoError(t, testCompile(t, di, false, *p, *pw)) pw.Methods.Add("invalid") - require.Error(t, testCompile(t, di, false, *p, *pw)) + require.Error(t, testCompile(t, di, false, *p, *pw, *pr)) pw.Methods.Add("method2") - require.NoError(t, testCompile(t, di, false, *p, *pw)) + require.Error(t, testCompile(t, di, false, *p, *pw)) + require.NoError(t, testCompile(t, di, false, *p, *pw, *pr)) }) }) } diff --git a/pkg/compiler/inline.go b/pkg/compiler/inline.go index 9e2cc2079..31329ddae 100644 --- a/pkg/compiler/inline.go +++ b/pkg/compiler/inline.go @@ -159,34 +159,27 @@ func (c *codegen) processNotify(f *funcScope, args []ast.Expr) { } func (c *codegen) processContractCall(f *funcScope, args []ast.Expr) { + var u util.Uint160 + // For stdlib calls it is `interop.Hash160(constHash)` // so we can determine hash at compile-time. ce, ok := args[0].(*ast.CallExpr) - if !ok || len(ce.Args) != 1 { - return + if ok && len(ce.Args) == 1 { + // Ensure this is a type conversion, not a simple invoke. + se, ok := ce.Fun.(*ast.SelectorExpr) + if ok { + name, _ := c.getFuncNameFromSelector(se) + if _, ok := c.funcs[name]; !ok { + value := c.typeAndValueOf(ce.Args[0]).Value + if value != nil { + s := constant.StringVal(value) + copy(u[:], s) // constant must be big-endian + } + } + } } - // Ensure this is a type conversion, not a simple invoke. - se, ok := ce.Fun.(*ast.SelectorExpr) - if !ok { - return - } - - name, _ := c.getFuncNameFromSelector(se) - if _, ok := c.funcs[name]; ok { - return - } - - value := c.typeAndValueOf(ce.Args[0]).Value - if value == nil { - return - } - - s := constant.StringVal(value) - var u util.Uint160 - copy(u[:], s) // constant must be big-endian - - value = c.typeAndValueOf(args[1]).Value + value := c.typeAndValueOf(args[1]).Value if value == nil { return } diff --git a/pkg/rpc/server/testdata/test_contract.yml b/pkg/rpc/server/testdata/test_contract.yml index 604bc1b0d..2761f0c7e 100644 --- a/pkg/rpc/server/testdata/test_contract.yml +++ b/pkg/rpc/server/testdata/test_contract.yml @@ -10,3 +10,5 @@ events: type: Hash160 - name: amount type: Integer +permissions: + - methods: ["onNEP17Payment"] \ No newline at end of file