diff --git a/pkg/core/interop/contract/call.go b/pkg/core/interop/contract/call.go index 61713c533..60f8ac9c5 100644 --- a/pkg/core/interop/contract/call.go +++ b/pkg/core/interop/contract/call.go @@ -6,6 +6,7 @@ import ( "math/big" "strings" + "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" @@ -77,12 +78,18 @@ func callInternal(ic *interop.Context, cs *state.Contract, name string, f callfl if md.Safe { f &^= (callflag.WriteStates | callflag.AllowNotify) } else if ctx := ic.VM.Context(); ctx != nil && ctx.IsDeployed() { - curr, err := ic.GetContract(ic.VM.GetCurrentScriptHash()) - if err == nil { - if !curr.Manifest.CanCall(cs.Hash, &cs.Manifest, name) { - return errors.New("disallowed method call") + var mfst *manifest.Manifest + if ic.IsHardforkEnabled(config.HFDomovoi) { + mfst = ctx.GetManifest() + } else { + curr, err := ic.GetContract(ic.VM.GetCurrentScriptHash()) + if err == nil { + mfst = &curr.Manifest } } + if mfst != nil && !mfst.CanCall(cs.Hash, &cs.Manifest, name) { + return errors.New("disallowed method call") + } } return callExFromNative(ic, ic.VM.GetCurrentScriptHash(), cs, name, args, f, hasReturn, isDynamic, false) } diff --git a/pkg/core/interop/contract/call_test.go b/pkg/core/interop/contract/call_test.go index 451d5b41d..c1f6034f3 100644 --- a/pkg/core/interop/contract/call_test.go +++ b/pkg/core/interop/contract/call_test.go @@ -11,6 +11,7 @@ import ( "github.com/nspcc-dev/neo-go/internal/contracts" "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/core/block" "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/core/interop/contract" @@ -173,6 +174,117 @@ func TestCall(t *testing.T) { }) } +func TestSystemContractCall_Permissions(t *testing.T) { + check := func(t *testing.T, cfg func(*config.Blockchain), shouldUpdateFail bool) { + bc, acc := chain.NewSingleWithCustomConfig(t, cfg) + e := neotest.NewExecutor(t, bc, acc, acc) + + // Contract A has an unsafe method. + srcA := `package contractA + func RetOne() int { + return 1 + }` + ctrA := neotest.CompileSource(t, acc.ScriptHash(), strings.NewReader(srcA), &compiler.Options{ + NoEventsCheck: true, + NoPermissionsCheck: true, + Name: "contractA", + }) + e.DeployContract(t, ctrA, nil) + + var hashAStr string + for i := 0; i < util.Uint160Size; i++ { + hashAStr += fmt.Sprintf("%#x", ctrA.Hash[i]) + if i != util.Uint160Size-1 { + hashAStr += ", " + } + } + + // Contract B has a method that calls contract's A and another update method that + // calls contract's A after B's update. + srcB := `package contractB + import ( + "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/interop/contract" + "github.com/nspcc-dev/neo-go/pkg/interop/native/management" + ) + func CallRetOne() int { + res := contract.Call(interop.Hash160{` + hashAStr + `}, "retOne", contract.All).(int) + return res + } + func Update(nef []byte, manifest []byte) int { + management.Update(nef, manifest) + res := contract.Call(interop.Hash160{` + hashAStr + `}, "retOne", contract.All).(int) + return res + }` + ctrB := neotest.CompileSource(t, acc.ScriptHash(), strings.NewReader(srcB), &compiler.Options{ + Name: "contractB", + NoEventsCheck: true, + NoPermissionsCheck: true, + Permissions: []manifest.Permission{ + { + Contract: manifest.PermissionDesc{Type: manifest.PermissionHash, Value: ctrA.Hash}, + Methods: manifest.WildStrings{Value: []string{"retOne"}}, + }, + { + Methods: manifest.WildStrings{Value: []string{"update"}}, + }, + }, + }) + e.DeployContract(t, ctrB, nil) + ctrBInvoker := e.ValidatorInvoker(ctrB.Hash) + + // ctrBUpdated differs from ctrB in that it has no permission to call retOne method of ctrA + ctrBUpdated := neotest.CompileSource(t, acc.ScriptHash(), strings.NewReader(srcB), &compiler.Options{ + Name: "contractB", + NoEventsCheck: true, + NoPermissionsCheck: true, + Permissions: []manifest.Permission{ + { + Contract: manifest.PermissionDesc{Type: manifest.PermissionHash, Value: ctrA.Hash}, + Methods: manifest.WildStrings{Value: []string{}}, + }, + { + Methods: manifest.WildStrings{Value: []string{"update"}}, + }, + }, + }) + + // Call to A before B update should HALT. + ctrBInvoker.Invoke(t, stackitem.Make(1), "callRetOne") + + // Call to A in the same context as B update should HALT. + n, err := ctrBUpdated.NEF.Bytes() + require.NoError(t, err) + m, err := json.Marshal(ctrBUpdated.Manifest) + require.NoError(t, err) + if shouldUpdateFail { + ctrBInvoker.InvokeFail(t, "System.Contract.Call failed: disallowed method call", "update", n, m) + } else { + ctrBInvoker.Invoke(t, stackitem.Make(1), "update", n, m) + } + + // If contract is updated, then all to A after B update should FAULT (manifest + // is updated, no permission to call retOne method). + if !shouldUpdateFail { + ctrBInvoker.InvokeFail(t, "System.Contract.Call failed: disallowed method call", "callRetOne") + } + } + + // Pre-Domovoi behaviour: an updated contract state is used for permissions check. + check(t, func(cfg *config.Blockchain) { + cfg.Hardforks = map[string]uint32{ + config.HFDomovoi.String(): 100500, + } + }, true) + + // Post-Domovoi behaviour: an executing contract state is used for permissions check. + check(t, func(cfg *config.Blockchain) { + cfg.Hardforks = map[string]uint32{ + config.HFDomovoi.String(): 0, + } + }, false) +} + func TestLoadToken(t *testing.T) { bc, acc := chain.NewSingle(t) e := neotest.NewExecutor(t, bc, acc, acc)