Merge pull request #2025 from nspcc-dev/compiler-check-calls

Check manifest permissions at compile-time

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"),
+		NoStandardCheck:    ctx.Bool("no-standards"),
-		NoEventsCheck:   ctx.Bool("no-events"),
+		NoEventsCheck:      ctx.Bool("no-events"),
+		NoPermissionsCheck: ctx.Bool("no-permissions"),
 	}
 
 	if len(confFile) != 0 {

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),
+		emittedEvents:    make(map[string][][]string),
-		sequencePoints: make(map[string][]DebugSeqPoint),
+		invokedContracts: make(map[util.Uint160][]string),
+		sequencePoints:   make(map[string][]DebugSeqPoint),
 	}
 }
 

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
 }

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())
-					targetPath := path.Join(examplePath, info.Name(), filename)
 					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))
+		})
+	})
+}

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
 }
 

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 c.scope != nil && c.isVerifyFunc(c.scope.decl) &&
-		if f.name != "Notify" && f.name != "Log" {
+		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) &&
+	value = c.typeAndValueOf(args[2]).Value
-			c.scope.pkg == c.mainPkg.Pkg && !c.buildInfo.options.NoEventsCheck {
+	if value == nil {
-			c.prog.Err = fmt.Errorf("runtime.%s is not allowed in `Verify`", f.name)
+		return
-			return
+	}
-		}
 
-		if f.name == "Log" {
+	flag, _ := constant.Uint64Val(value)
-			return
+	if flag&uint64(callflag.WriteStates|callflag.AllowNotify) != 0 {
-		}
+		c.invokedContracts[u] = append(currLst, method)
-
-		// 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)
 	}
 }
 

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
+}

pkg/rpc/server/testdata/test_contract.yml

@@ -10,3 +10,5 @@ events:
         type: Hash160
       - name: amount
         type: Integer
+permissions:
+  - methods: ["onNEP17Payment"]