From 554e48e7b766892f3b3412218fa25ad6445b5b50 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 29 Sep 2022 16:53:28 +0300 Subject: [PATCH] compiler: enforce runtime.Notify parameters cast If notification parameters type can be defined in a compile time then enforce parameter cast to the desired type got from manifest. --- docs/compiler.md | 8 +- pkg/compiler/inline.go | 67 +++++++++++-- pkg/compiler/interop_test.go | 144 +++++++++++++++++++++++++++ pkg/smartcontract/param_type.go | 35 +++++++ pkg/smartcontract/param_type_test.go | 25 +++++ 5 files changed, 266 insertions(+), 13 deletions(-) diff --git a/docs/compiler.md b/docs/compiler.md index 7d5810177..339efd057 100644 --- a/docs/compiler.md +++ b/docs/compiler.md @@ -289,8 +289,12 @@ As an example, consider `Transfer` event from `NEP-17` standard: By default, compiler performs some sanity checks. Most of the time it will report missing events and/or parameter type mismatch. It isn't prohibited to use a variable as an event name in code, but it will prevent -the compiler from analyzing the event. It is better to use either constant or string literal. -The check can be disabled with `--no-events` flag. +the compiler from analyzing the event. It is better to use either constant or string literal. +It isn't prohibited to use ellipsis expression as an event arguments, but it will also +prevent the compiler from analyzing the event. It is better to provide arguments directly +without `...`. The type conversion code will be emitted for checked events, it will cast +argument types to ones specified in the contract manifest. These checks and conversion can +be disabled with `--no-events` flag. ##### Permissions Each permission specifies contracts and methods allowed for this permission. diff --git a/pkg/compiler/inline.go b/pkg/compiler/inline.go index 56884830a..0f82d95ba 100644 --- a/pkg/compiler/inline.go +++ b/pkg/compiler/inline.go @@ -11,6 +11,7 @@ import ( "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" + "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) // inlineCall inlines call of n for function represented by f. @@ -37,7 +38,7 @@ func (c *codegen) inlineCall(f *funcScope, n *ast.CallExpr) { sig := c.typeOf(n.Fun).(*types.Signature) hasVarArgs := !n.Ellipsis.IsValid() - c.processStdlibCall(f, n.Args, !hasVarArgs) + eventParams := c.processStdlibCall(f, n.Args, !hasVarArgs) // When inlined call is used during global initialization // there is no func scope, thus this if. @@ -102,6 +103,11 @@ func (c *codegen) inlineCall(f *funcScope, n *ast.CallExpr) { c.scope.vars.locals = oldScope for i := sig.Params().Len() - 1; i < len(n.Args); i++ { ast.Walk(c, n.Args[i]) + // In case of runtime.Notify, its arguments need to be converted to proper type. + // i's initial value is 1 (variadic args start). + if eventParams != nil && eventParams[i-1] != nil { + c.emitConvert(*eventParams[i-1]) + } } c.scope.vars.locals = newScope c.packVarArgs(n, sig) @@ -129,52 +135,91 @@ 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, hasEllipsis bool) { +func (c *codegen) processStdlibCall(f *funcScope, args []ast.Expr, hasEllipsis bool) []*stackitem.Type { if f == nil { - return + return nil } + var eventParams []*stackitem.Type if f.pkg.Path() == interopPrefix+"/runtime" && (f.name == "Notify" || f.name == "Log") { - c.processNotify(f, args, hasEllipsis) + eventParams = c.processNotify(f, args, hasEllipsis) } if f.pkg.Path() == interopPrefix+"/contract" && f.name == "Call" { c.processContractCall(f, args) } + return eventParams } -func (c *codegen) processNotify(f *funcScope, args []ast.Expr, hasEllipsis bool) { +// processNotify checks whether notification emitting rules are met and returns expected +// notification signature if found. +func (c *codegen) processNotify(f *funcScope, args []ast.Expr, hasEllipsis bool) []*stackitem.Type { if c.scope != nil && c.isVerifyFunc(c.scope.decl) && c.scope.pkg == c.mainPkg.Types && (c.buildInfo.options == nil || !c.buildInfo.options.NoEventsCheck) { c.prog.Err = fmt.Errorf("runtime.%s is not allowed in `Verify`", f.name) - return + return nil } if f.name == "Log" { - return + return nil } // Sometimes event name is stored in a var. Or sometimes event args are provided // via ellipses (`slice...`). - // Skip in this case. + // Skip in this case. Also, don't enforce runtime.Notify parameters conversion. tv := c.typeAndValueOf(args[0]) if tv.Value == nil || hasEllipsis { - return + return nil } params := make([]string, 0, len(args[1:])) + vParams := make([]*stackitem.Type, 0, len(args[1:])) for _, p := range args[1:] { - st, _, _ := c.scAndVMTypeFromExpr(p) + st, vt, _ := c.scAndVMTypeFromExpr(p) params = append(params, st.String()) + vParams = append(vParams, &vt) } 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 + return nil + } + var eventFound bool + if c.buildInfo.options != nil && c.buildInfo.options.ContractEvents != nil && !c.buildInfo.options.NoEventsCheck { + for _, e := range c.buildInfo.options.ContractEvents { + if e.Name == name && len(e.Parameters) == len(vParams) { + eventFound = true + for i, scParam := range e.Parameters { + expectedType := scParam.Type.ConvertToStackitemType() + // No need to cast if the desired type is unknown. + if expectedType == stackitem.AnyT || + // Do not cast if desired type is Interop, the actual type is likely to be Any, leave the resolving to runtime.Notify. + expectedType == stackitem.InteropT || + // No need to cast if actual parameter type matches the desired one. + *vParams[i] == expectedType || + // expectedType doesn't contain Buffer anyway, but if actual variable type is Buffer, + // then runtime.Notify will convert it to ByteArray automatically, thus no need to emit conversion code. + (*vParams[i] == stackitem.BufferT && expectedType == stackitem.ByteArrayT) { + vParams[i] = nil + } else { + // For other cases the conversion code will be emitted using vParams... + vParams[i] = &expectedType + // ...thus, update emitted notification info in advance. + params[i] = scParam.Type.String() + } + } + } + } } c.emittedEvents[name] = append(c.emittedEvents[name], params) + // Do not enforce perfect expected/actual events match on this step, the final + // check wil be performed after compilation if --no-events option is off. + if eventFound { + return vParams + } + return nil } func (c *codegen) processContractCall(f *funcScope, args []ast.Expr) { diff --git a/pkg/compiler/interop_test.go b/pkg/compiler/interop_test.go index 07a617560..360c81f3a 100644 --- a/pkg/compiler/interop_test.go +++ b/pkg/compiler/interop_test.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "math/big" + "strconv" "strings" "testing" @@ -22,6 +23,7 @@ import ( cinterop "github.com/nspcc-dev/neo-go/pkg/interop" "github.com/nspcc-dev/neo-go/pkg/neotest" "github.com/nspcc-dev/neo-go/pkg/neotest/chain" + "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" @@ -548,3 +550,145 @@ func TestCallWithVersion(t *testing.T) { c.InvokeFail(t, "contract version mismatch", "callWithVersion", policyH.BytesBE(), 1, "getExecFeeFactor") }) } + +func TestForcedNotifyArgumentsConversion(t *testing.T) { + const methodWithEllipsis = "withEllipsis" + const methodWithoutEllipsis = "withoutEllipsis" + check := func(t *testing.T, method string, targetSCParamTypes []smartcontract.ParamType, expectedVMParamTypes []stackitem.Type, noEventsCheck bool) { + bc, acc := chain.NewSingle(t) + e := neotest.NewExecutor(t, bc, acc, acc) + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" + const arg4 = 4 // Const value. + func WithoutEllipsis() { + var arg0 int // Default value. + var arg1 int = 1 // Initialized value. + arg2 := 2 // Short decl. + var arg3 int + arg3 = 3 // Declare first, change value afterwards. + runtime.Notify("withoutEllipsis", arg0, arg1, arg2, arg3, arg4, 5, f(6)) // The fifth argument is basic literal. + } + func WithEllipsis() { + arg := []interface{}{0, 1, f(2), 3, 4, 5, 6} + runtime.Notify("withEllipsis", arg...) + } + func f(i int) int { + return i + }` + count := len(targetSCParamTypes) + if count != len(expectedVMParamTypes) { + t.Fatalf("parameters count mismatch: %d vs %d", count, len(expectedVMParamTypes)) + } + scParams := make([]manifest.Parameter, len(targetSCParamTypes)) + vmParams := make([]stackitem.Item, len(expectedVMParamTypes)) + for i := range scParams { + scParams[i] = manifest.Parameter{ + Name: strconv.Itoa(i), + Type: targetSCParamTypes[i], + } + defaultValue := stackitem.NewBigInteger(big.NewInt(int64(i))) + var ( + val stackitem.Item + err error + ) + if expectedVMParamTypes[i] == stackitem.IntegerT { + val = defaultValue + } else { + val, err = defaultValue.Convert(expectedVMParamTypes[i]) // exactly the same conversion should be emitted by compiler and performed by the contract code. + require.NoError(t, err) + } + vmParams[i] = val + } + ctr := neotest.CompileSource(t, e.CommitteeHash, strings.NewReader(src), &compiler.Options{ + Name: "Helper", + ContractEvents: []manifest.Event{ + { + Name: methodWithoutEllipsis, + Parameters: scParams, + }, + { + Name: methodWithEllipsis, + Parameters: scParams, + }, + }, + NoEventsCheck: noEventsCheck, + }) + e.DeployContract(t, ctr, nil) + c := e.CommitteeInvoker(ctr.Hash) + + t.Run(method, func(t *testing.T) { + h := c.Invoke(t, stackitem.Null{}, method) + aer := c.GetTxExecResult(t, h) + require.Equal(t, 1, len(aer.Events)) + require.Equal(t, stackitem.NewArray(vmParams), aer.Events[0].Item) + }) + } + checkSingleType := func(t *testing.T, method string, targetSCEventType smartcontract.ParamType, expectedVMType stackitem.Type, noEventsCheck ...bool) { + count := 7 + scParams := make([]smartcontract.ParamType, count) + vmParams := make([]stackitem.Type, count) + for i := range scParams { + scParams[i] = targetSCEventType + vmParams[i] = expectedVMType + } + var noEvents bool + if len(noEventsCheck) > 0 { + noEvents = noEventsCheck[0] + } + check(t, method, scParams, vmParams, noEvents) + } + + t.Run("good, single type, default values", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.IntegerType, stackitem.IntegerT) + }) + t.Run("good, single type, conversion to BooleanT", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.BoolType, stackitem.BooleanT) + }) + t.Run("good, single type, Hash160Type->ByteArray", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.Hash160Type, stackitem.ByteArrayT) + }) + t.Run("good, single type, Hash256Type->ByteArray", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.Hash256Type, stackitem.ByteArrayT) + }) + t.Run("good, single type, Signature->ByteArray", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.SignatureType, stackitem.ByteArrayT) + }) + t.Run("good, single type, String->ByteArray", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.StringType, stackitem.ByteArrayT) // Special case, runtime.Notify will convert any Buffer to ByteArray. + }) + t.Run("good, single type, PublicKeyType->ByteArray", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.PublicKeyType, stackitem.ByteArrayT) + }) + t.Run("good, single type, AnyType->do not change initial type", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.AnyType, stackitem.IntegerT) // Special case, compiler should leave the type "as is" and do not emit conversion code. + }) + // Test for InteropInterface->... is missing, because we don't enforce conversion to stackitem.InteropInterface, + // but compiler still checks these notifications against expected manifest. + t.Run("good, multiple types, check the conversion order", func(t *testing.T) { + check(t, methodWithoutEllipsis, []smartcontract.ParamType{ + smartcontract.IntegerType, + smartcontract.BoolType, + smartcontract.ByteArrayType, + smartcontract.PublicKeyType, + smartcontract.Hash160Type, + smartcontract.AnyType, // leave initial type + smartcontract.StringType, + }, []stackitem.Type{ + stackitem.IntegerT, + stackitem.BooleanT, + stackitem.ByteArrayT, + stackitem.ByteArrayT, + stackitem.ByteArrayT, + stackitem.IntegerT, // leave initial type + stackitem.ByteArrayT, + }, false) + }) + t.Run("with ellipsis, do not emit conversion code", func(t *testing.T) { + checkSingleType(t, methodWithEllipsis, smartcontract.IntegerType, stackitem.IntegerT) + checkSingleType(t, methodWithEllipsis, smartcontract.BoolType, stackitem.IntegerT) + checkSingleType(t, methodWithEllipsis, smartcontract.ByteArrayType, stackitem.IntegerT) + }) + t.Run("no events check => no conversion code", func(t *testing.T) { + checkSingleType(t, methodWithoutEllipsis, smartcontract.PublicKeyType, stackitem.IntegerT, true) + }) +} diff --git a/pkg/smartcontract/param_type.go b/pkg/smartcontract/param_type.go index 0160e357c..5b36de52f 100644 --- a/pkg/smartcontract/param_type.go +++ b/pkg/smartcontract/param_type.go @@ -331,3 +331,38 @@ func ConvertToParamType(val int) (ParamType, error) { } return UnknownType, errors.New("unknown parameter type") } + +// ConvertToStackitemType converts ParamType to corresponding Stackitem.Type. +func (pt ParamType) ConvertToStackitemType() stackitem.Type { + switch pt { + case SignatureType: + return stackitem.ByteArrayT + case BoolType: + return stackitem.BooleanT + case IntegerType: + return stackitem.IntegerT + case Hash160Type: + return stackitem.ByteArrayT + case Hash256Type: + return stackitem.ByteArrayT + case ByteArrayType: + return stackitem.ByteArrayT + case PublicKeyType: + return stackitem.ByteArrayT + case StringType: + // Do not use BufferT to match System.Runtime.Notify conversion rules. + return stackitem.ByteArrayT + case ArrayType: + return stackitem.ArrayT + case MapType: + return stackitem.MapT + case InteropInterfaceType: + return stackitem.InteropT + case VoidType: + return stackitem.AnyT + case AnyType: + return stackitem.AnyT + default: + panic(fmt.Sprintf("unknown param type %d", pt)) + } +} diff --git a/pkg/smartcontract/param_type_test.go b/pkg/smartcontract/param_type_test.go index 181c584f8..a9f52c11d 100644 --- a/pkg/smartcontract/param_type_test.go +++ b/pkg/smartcontract/param_type_test.go @@ -393,3 +393,28 @@ func TestConvertToParamType(t *testing.T) { _, err := ConvertToParamType(0x01) require.NotNil(t, err) } + +func TestConvertToStackitemType(t *testing.T) { + for p, expected := range map[ParamType]stackitem.Type{ + AnyType: stackitem.AnyT, + BoolType: stackitem.BooleanT, + IntegerType: stackitem.IntegerT, + ByteArrayType: stackitem.ByteArrayT, + StringType: stackitem.ByteArrayT, + Hash160Type: stackitem.ByteArrayT, + Hash256Type: stackitem.ByteArrayT, + PublicKeyType: stackitem.ByteArrayT, + SignatureType: stackitem.ByteArrayT, + ArrayType: stackitem.ArrayT, + MapType: stackitem.MapT, + InteropInterfaceType: stackitem.InteropT, + VoidType: stackitem.AnyT, + } { + actual := p.ConvertToStackitemType() + require.Equal(t, expected, actual) + } + + require.Panics(t, func() { + UnknownType.ConvertToStackitemType() + }) +}