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..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" ) @@ -43,6 +44,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 +297,37 @@ 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 { + knownHash := !h.Equals(util.Uint160{}) + + methodLoop: + for _, m := range methods { + for _, p := range o.Permissions { + // Group or wildcard permission is ok to try. + if knownHash && p.Contract.Type == manifest.PermissionHash && !p.Contract.Hash().Equals(h) { + continue + } + + if p.Methods.Contains(m) { + continue methodLoop + } + } + + 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) + } + } + } return m, nil } diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index eb18a1b1e..62c5f2b87 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -8,10 +8,14 @@ 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" + "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" ) @@ -53,12 +57,8 @@ func TestCompiler(t *testing.T) { // there are also a couple of files inside the `examplePath` which doesn't need to be compiled continue } - infos, err := ioutil.ReadDir(path.Join(examplePath, info.Name())) - require.NoError(t, err) - require.False(t, len(infos) == 0, "detected smart contract folder with no contract in it") - filename := filterFilename(infos) - targetPath := path.Join(examplePath, info.Name(), filename) + targetPath := path.Join(examplePath, info.Name()) require.NoError(t, compileFile(targetPath)) } }, @@ -86,15 +86,6 @@ func TestCompiler(t *testing.T) { } } -func filterFilename(infos []os.FileInfo) string { - for _, info := range infos { - if !info.IsDir() { - return info.Name() - } - } - return "" -} - func compileFile(src string) error { _, err := compiler.Compile(src, nil) return err @@ -197,3 +188,109 @@ 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(), "method4", 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)) + + 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.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) { + 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, *pr)) + + pw.Methods.Add("method2") + require.Error(t, testCompile(t, di, false, *p, *pw)) + require.NoError(t, testCompile(t, di, false, *p, *pw, *pr)) + }) + }) +} 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..31329ddae 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,95 @@ 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) { + 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 { + // 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 + } + } + } + } + + 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 +} 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