From 4249674ddceca990a99e2055adec1295c11d999f Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Thu, 24 Jun 2021 18:36:40 +0300 Subject: [PATCH] compiler: check for contract permissions On many occassions we can determine at compile-time if contract config lacks some properties it needs. This includes all native contract invocations through stdlib, as both hashes and methods are known at compile-time there. Signed-off-by: Evgeniy Stratonikov --- cli/smartcontract/smart_contract.go | 9 +- pkg/compiler/codegen.go | 9 +- pkg/compiler/compiler.go | 31 +++++++ pkg/compiler/compiler_test.go | 105 +++++++++++++++++++++++ pkg/compiler/debug.go | 4 + pkg/compiler/inline.go | 126 +++++++++++++++++++++------- pkg/compiler/testdata/runh/hash.go | 13 +++ 7 files changed, 261 insertions(+), 36 deletions(-) create mode 100644 pkg/compiler/testdata/runh/hash.go diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index 89e80a5b6..677a27d06 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -154,6 +154,10 @@ func NewCommands() []cli.Command { Name: "no-events", Usage: "do not check emitted events with the manifest", }, + cli.BoolFlag{ + Name: "no-permissions", + Usage: "do not check if invoked contracts are allowed in manifest", + }, }, }, { @@ -439,8 +443,9 @@ func contractCompile(ctx *cli.Context) error { DebugInfo: debugFile, ManifestFile: manifestFile, - NoStandardCheck: ctx.Bool("no-standards"), - NoEventsCheck: ctx.Bool("no-events"), + NoStandardCheck: ctx.Bool("no-standards"), + NoEventsCheck: ctx.Bool("no-events"), + NoPermissionsCheck: ctx.Bool("no-permissions"), } if len(confFile) != 0 { diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 53ac35249..28081596b 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -15,6 +15,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" @@ -108,6 +109,9 @@ type codegen struct { // emittedEvents contains all events emitted by contract. emittedEvents map[string][][]string + // invokedContracts contains invoked methods of other contracts. + invokedContracts map[util.Uint160][]string + // Label table for recording jump destinations. l []int } @@ -2058,8 +2062,9 @@ func newCodegen(info *buildInfo, pkg *loader.PackageInfo) *codegen { initEndOffset: -1, deployEndOffset: -1, - emittedEvents: make(map[string][][]string), - sequencePoints: make(map[string][]DebugSeqPoint), + emittedEvents: make(map[string][][]string), + invokedContracts: make(map[util.Uint160][]string), + sequencePoints: make(map[string][]DebugSeqPoint), } } diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index 01e99a14a..d73ad9e84 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -43,6 +43,11 @@ type Options struct { // This setting has effect only if manifest is emitted. NoStandardCheck bool + // NoPermissionsCheck specifies if permissions in YAML config need to be checked + // against invocations performed by the contract. + // This setting has effect only if manifest is emitted. + NoPermissionsCheck bool + // Name is contract's name to be written to manifest. Name string @@ -291,5 +296,31 @@ func CreateManifest(di *DebugInfo, o *Options) (*manifest.Manifest, error) { } } } + + if !o.NoPermissionsCheck { + // We can't perform full check for 2 reasons: + // 1. Contract hash may not be available at compile time. + // 2. Permission may be specified for a group of contracts by public key. + // Thus only basic checks are performed. + + for h, methods := range di.InvokedContracts { + 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) { + continue + } + + if p.Methods.Contains(m) { + continue methodLoop + } + } + + return nil, fmt.Errorf("method '%s' of contract %s is invoked but"+ + " corresponding permission is missing", m, h.StringLE()) + } + } + } return m, nil } diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index eb18a1b1e..3188e6812 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -10,8 +10,11 @@ import ( "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" + "github.com/nspcc-dev/neo-go/pkg/interop/native/neo" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/stretchr/testify/require" ) @@ -197,3 +200,105 @@ func TestNotifyInVerify(t *testing.T) { }) } } + +func TestInvokedContractsPermissons(t *testing.T) { + testCompile := func(t *testing.T, di *compiler.DebugInfo, disable bool, ps ...manifest.Permission) error { + o := &compiler.Options{ + NoPermissionsCheck: disable, + Permissions: ps, + } + + _, err := compiler.CreateManifest(di, o) + return err + } + + t.Run("native", func(t *testing.T) { + src := `package test + import "github.com/nspcc-dev/neo-go/pkg/interop/native/neo" + import "github.com/nspcc-dev/neo-go/pkg/interop/native/management" + func Main() int { + neo.Transfer(nil, nil, 10, nil) + management.GetContract(nil) // skip read-only + return 0 + }` + + _, di, err := compiler.CompileWithOptions("permissionTest", strings.NewReader(src), &compiler.Options{}) + require.NoError(t, err) + + var nh util.Uint160 + + p := manifest.NewPermission(manifest.PermissionHash, nh) + require.Error(t, testCompile(t, di, false, *p)) + require.NoError(t, testCompile(t, di, true, *p)) + + copy(nh[:], neo.Hash) + p.Contract.Value = nh + require.NoError(t, testCompile(t, di, false, *p)) + + p.Methods.Restrict() + require.Error(t, testCompile(t, di, false, *p)) + require.NoError(t, testCompile(t, di, true, *p)) + }) + + t.Run("custom", func(t *testing.T) { + hashStr := "aaaaaaaaaaaaaaaaaaaa" + src := fmt.Sprintf(`package test + import "github.com/nspcc-dev/neo-go/pkg/interop/contract" + import "github.com/nspcc-dev/neo-go/pkg/interop" + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/runh" + + const hash = "%s" + var runtimeHash interop.Hash160 + var runtimeMethod string + func invoke(h string) interop.Hash160 { return nil } + func Main() { + contract.Call(interop.Hash160(hash), "method1", contract.All) + contract.Call(interop.Hash160(hash), "method2", contract.All) + contract.Call(interop.Hash160(hash), "method2", contract.All) + + // skip read-only + contract.Call(interop.Hash160(hash), "method3", contract.ReadStates) + + // skip this + 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) + }`, hashStr) + + _, di, err := compiler.CompileWithOptions("permissionTest", strings.NewReader(src), &compiler.Options{}) + require.NoError(t, err) + + var h util.Uint160 + copy(h[:], hashStr) + + p := manifest.NewPermission(manifest.PermissionHash, h) + require.NoError(t, testCompile(t, di, false, *p)) + + p.Methods.Add("method1") + require.Error(t, testCompile(t, di, false, *p)) + require.NoError(t, testCompile(t, di, true, *p)) + + 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)) + }) + + t.Run("group", func(t *testing.T) { + priv, _ := keys.NewPrivateKey() + pw := manifest.NewPermission(manifest.PermissionGroup, priv.PublicKey()) + require.NoError(t, testCompile(t, di, false, *p, *pw)) + + pw.Methods.Add("invalid") + require.Error(t, testCompile(t, di, false, *p, *pw)) + + pw.Methods.Add("method2") + require.NoError(t, testCompile(t, di, false, *p, *pw)) + }) + }) +} diff --git a/pkg/compiler/debug.go b/pkg/compiler/debug.go index b74fb8cb0..2bcd6a52e 100644 --- a/pkg/compiler/debug.go +++ b/pkg/compiler/debug.go @@ -13,6 +13,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) @@ -24,6 +25,8 @@ type DebugInfo struct { Events []EventDebugInfo `json:"events"` // EmittedEvents contains events occurring in code. EmittedEvents map[string][][]string `json:"-"` + // InvokedContracts contains foreign contract invocations. + InvokedContracts map[util.Uint160][]string `json:"-"` // StaticVariables contains list of static variable names and types. StaticVariables []string `json:"static-variables"` } @@ -180,6 +183,7 @@ func (c *codegen) emitDebugInfo(contract []byte) *DebugInfo { d.Methods = append(d.Methods, *m) } d.EmittedEvents = c.emittedEvents + d.InvokedContracts = c.invokedContracts return d } diff --git a/pkg/compiler/inline.go b/pkg/compiler/inline.go index 07a908a04..9e2cc2079 100644 --- a/pkg/compiler/inline.go +++ b/pkg/compiler/inline.go @@ -7,6 +7,8 @@ import ( "go/types" "github.com/nspcc-dev/neo-go/pkg/core/interop/runtime" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" ) @@ -30,7 +32,7 @@ func (c *codegen) inlineCall(f *funcScope, n *ast.CallExpr) { pkg := c.buildInfo.program.Package(f.pkg.Path()) sig := c.typeOf(n.Fun).(*types.Signature) - c.processNotify(f, n.Args) + c.processStdlibCall(f, n.Args) // When inlined call is used during global initialization // there is no func scope, thus this if. @@ -109,42 +111,102 @@ func (c *codegen) inlineCall(f *funcScope, n *ast.CallExpr) { c.pkgInfoInline = c.pkgInfoInline[:len(c.pkgInfoInline)-1] } +func (c *codegen) processStdlibCall(f *funcScope, args []ast.Expr) { + if f == nil { + return + } + + if f.pkg.Path() == interopPrefix+"/runtime" && (f.name == "Notify" || f.name == "Log") { + c.processNotify(f, args) + } + + if f.pkg.Path() == interopPrefix+"/contract" && f.name == "Call" { + c.processContractCall(f, args) + } +} + func (c *codegen) processNotify(f *funcScope, args []ast.Expr) { - if f != nil && f.pkg.Path() == interopPrefix+"/runtime" { - if f.name != "Notify" && f.name != "Log" { + if c.scope != nil && c.isVerifyFunc(c.scope.decl) && + c.scope.pkg == c.mainPkg.Pkg && !c.buildInfo.options.NoEventsCheck { + c.prog.Err = fmt.Errorf("runtime.%s is not allowed in `Verify`", f.name) + return + } + + if f.name == "Log" { + return + } + + // Sometimes event name is stored in a var. + // Skip in this case. + tv := c.typeAndValueOf(args[0]) + if tv.Value == nil { + return + } + + params := make([]string, 0, len(args[1:])) + for _, p := range args[1:] { + st, _ := c.scAndVMTypeFromExpr(p) + params = append(params, st.String()) + } + + name := constant.StringVal(tv.Value) + if len(name) > runtime.MaxEventNameLen { + c.prog.Err = fmt.Errorf("event name '%s' should be less than %d", + name, runtime.MaxEventNameLen) + return + } + c.emittedEvents[name] = append(c.emittedEvents[name], params) +} + +func (c *codegen) processContractCall(f *funcScope, args []ast.Expr) { + // 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 + } + + // 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 + if value == nil { + return + } + + method := constant.StringVal(value) + currLst := c.invokedContracts[u] + for _, m := range currLst { + if m == method { return } + } - if c.scope != nil && c.isVerifyFunc(c.scope.decl) && - c.scope.pkg == c.mainPkg.Pkg && !c.buildInfo.options.NoEventsCheck { - c.prog.Err = fmt.Errorf("runtime.%s is not allowed in `Verify`", f.name) - return - } + value = c.typeAndValueOf(args[2]).Value + if value == nil { + return + } - if f.name == "Log" { - return - } - - // Sometimes event name is stored in a var. - // Skip in this case. - tv := c.typeAndValueOf(args[0]) - if tv.Value == nil { - return - } - - params := make([]string, 0, len(args[1:])) - for _, p := range args[1:] { - st, _ := c.scAndVMTypeFromExpr(p) - params = append(params, st.String()) - } - - name := constant.StringVal(tv.Value) - if len(name) > runtime.MaxEventNameLen { - c.prog.Err = fmt.Errorf("event name '%s' should be less than %d", - name, runtime.MaxEventNameLen) - return - } - c.emittedEvents[name] = append(c.emittedEvents[name], params) + flag, _ := constant.Uint64Val(value) + if flag&uint64(callflag.WriteStates|callflag.AllowNotify) != 0 { + c.invokedContracts[u] = append(currLst, method) } } diff --git a/pkg/compiler/testdata/runh/hash.go b/pkg/compiler/testdata/runh/hash.go new file mode 100644 index 000000000..b6b5d9204 --- /dev/null +++ b/pkg/compiler/testdata/runh/hash.go @@ -0,0 +1,13 @@ +package runh + +import "github.com/nspcc-dev/neo-go/pkg/interop" + +// RuntimeHash possibly returns some hash at runtime. +func RuntimeHash() interop.Hash160 { + return nil +} + +// RuntimeHashArgs possibly returns some hash at runtime. +func RuntimeHashArgs(s string) interop.Hash160 { + return nil +}