From 4945145b09f5b77a6743bfd81bce465c3c39ead6 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Sat, 1 Jun 2024 13:03:07 +0300 Subject: [PATCH] interop: use executing contract state for permissions checks Do not use the updated contract state from native Management to perform permissions checks. We need to use the currently executing state instead got from the currently executing VM context until context is unloaded. Close #3471. Signed-off-by: Anna Shaleva --- cli/vm/cli.go | 2 +- pkg/core/blockchain.go | 2 +- pkg/core/interop/contract/call.go | 2 +- pkg/core/interop/runtime/engine.go | 10 +++++----- pkg/core/interop/runtime/ext_test.go | 2 +- pkg/vm/context.go | 8 ++++++++ pkg/vm/debug_test.go | 2 +- pkg/vm/vm.go | 14 ++++++++------ 8 files changed, 26 insertions(+), 16 deletions(-) diff --git a/cli/vm/cli.go b/cli/vm/cli.go index b504a51dd..8389d0c84 100644 --- a/cli/vm/cli.go +++ b/cli/vm/cli.go @@ -1105,7 +1105,7 @@ func handleRun(c *cli.Context) error { breaks := v.Context().BreakPoints() // We ensure that there's a context loaded. ic.ReuseVM(v) v.GasLimit = gasLimit - v.LoadNEFMethod(&cs.NEF, util.Uint160{}, cs.Hash, callflag.All, hasRet, offset, initOff, nil) + v.LoadNEFMethod(&cs.NEF, &cs.Manifest, util.Uint160{}, cs.Hash, callflag.All, hasRet, offset, initOff, nil) for _, bp := range breaks { v.AddBreakPoint(bp) } diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index a93946b2a..d5e090044 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -2928,7 +2928,7 @@ func (bc *Blockchain) InitVerificationContext(ic *interop.Context, hash util.Uin initOffset = md.Offset } ic.Invocations[cs.Hash]++ - ic.VM.LoadNEFMethod(&cs.NEF, util.Uint160{}, hash, callflag.ReadOnly, + ic.VM.LoadNEFMethod(&cs.NEF, &cs.Manifest, util.Uint160{}, hash, callflag.ReadOnly, true, verifyOffset, initOffset, nil) } if len(witness.InvocationScript) != 0 { diff --git a/pkg/core/interop/contract/call.go b/pkg/core/interop/contract/call.go index 712b9e1d9..61713c533 100644 --- a/pkg/core/interop/contract/call.go +++ b/pkg/core/interop/contract/call.go @@ -144,7 +144,7 @@ func callExFromNative(ic *interop.Context, caller util.Uint160, cs *state.Contra } return nil } - ic.VM.LoadNEFMethod(&cs.NEF, caller, cs.Hash, f, + ic.VM.LoadNEFMethod(&cs.NEF, &cs.Manifest, caller, cs.Hash, f, hasReturn, methodOff, initOff, onUnload) for e, i := ic.VM.Estack(), len(args)-1; i >= 0; i-- { diff --git a/pkg/core/interop/runtime/engine.go b/pkg/core/interop/runtime/engine.go index b7a985a33..3a16a4055 100644 --- a/pkg/core/interop/runtime/engine.go +++ b/pkg/core/interop/runtime/engine.go @@ -80,19 +80,19 @@ func Notify(ic *interop.Context) error { if len(name) > MaxEventNameLen { return fmt.Errorf("event name must be less than %d", MaxEventNameLen) } - curHash := ic.VM.GetCurrentScriptHash() - ctr, err := ic.GetContract(curHash) - if err != nil { + curr := ic.VM.Context().GetManifest() + if curr == nil { return errors.New("notifications are not allowed in dynamic scripts") } var ( - ev = ctr.Manifest.ABI.GetEvent(name) + ev = curr.ABI.GetEvent(name) checkErr error + curHash = ic.VM.GetCurrentScriptHash() ) if ev == nil { checkErr = fmt.Errorf("notification %s does not exist", name) } else { - err = ev.CheckCompliance(args) + err := ev.CheckCompliance(args) if err != nil { checkErr = fmt.Errorf("notification %s is invalid: %w", name, err) } diff --git a/pkg/core/interop/runtime/ext_test.go b/pkg/core/interop/runtime/ext_test.go index d33c1043f..f8d2bcf89 100644 --- a/pkg/core/interop/runtime/ext_test.go +++ b/pkg/core/interop/runtime/ext_test.go @@ -637,7 +637,7 @@ func TestNotify(t *testing.T) { _, _, bc, cs := getDeployedInternal(t) ic, err := bc.GetTestVM(trigger.Application, nil, nil) require.NoError(t, err) - ic.VM.LoadNEFMethod(&cs.NEF, caller, cs.Hash, callflag.NoneFlag, true, 0, -1, nil) + ic.VM.LoadNEFMethod(&cs.NEF, &cs.Manifest, caller, cs.Hash, callflag.NoneFlag, true, 0, -1, nil) ic.VM.Estack().PushVal(args) ic.VM.Estack().PushVal(name) return ic diff --git a/pkg/vm/context.go b/pkg/vm/context.go index e777fdbeb..9a5be2a1e 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -8,6 +8,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "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/nef" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/invocations" @@ -44,6 +45,8 @@ type scriptContext struct { // NEF represents a NEF file for the current contract. NEF *nef.File + // Manifest represents a manifest for the current contract. + Manifest *manifest.Manifest // invTree is an invocation tree (or a branch of it) for this context. invTree *invocations.Tree // onUnload is a callback that should be called after current context unloading @@ -249,6 +252,11 @@ func (c *Context) GetNEF() *nef.File { return c.sc.NEF } +// GetManifest returns Manifest used by this context if it's present. +func (c *Context) GetManifest() *manifest.Manifest { + return c.sc.Manifest +} + // NumOfReturnVals returns the number of return values expected from this context. func (c *Context) NumOfReturnVals() int { return c.retCount diff --git a/pkg/vm/debug_test.go b/pkg/vm/debug_test.go index ea985aa24..ca5df493a 100644 --- a/pkg/vm/debug_test.go +++ b/pkg/vm/debug_test.go @@ -56,6 +56,6 @@ func TestContext_BreakPoints(t *testing.T) { require.Equal(t, []int{3, 5}, v.Context().BreakPoints()) // New context -> clean breakpoints. - v.loadScriptWithCallingHash(prog, nil, util.Uint160{}, util.Uint160{}, callflag.All, 1, 3, nil) + v.loadScriptWithCallingHash(prog, nil, nil, util.Uint160{}, util.Uint160{}, callflag.All, 1, 3, nil) require.Equal(t, []int{}, v.Context().BreakPoints()) } diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index c48d993ec..d3959f1e2 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -18,6 +18,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/encoding/bigint" "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/nef" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" @@ -308,14 +309,14 @@ func (v *VM) LoadScript(b []byte) { // LoadScriptWithFlags loads script and sets call flag to f. func (v *VM) LoadScriptWithFlags(b []byte, f callflag.CallFlag) { - v.loadScriptWithCallingHash(b, nil, v.GetCurrentScriptHash(), util.Uint160{}, f, -1, 0, nil) + v.loadScriptWithCallingHash(b, nil, nil, v.GetCurrentScriptHash(), util.Uint160{}, f, -1, 0, nil) } // LoadDynamicScript loads the given script with the given flags. This script is // considered to be dynamic, it can either return no value at all or return // exactly one value. func (v *VM) LoadDynamicScript(b []byte, f callflag.CallFlag) { - v.loadScriptWithCallingHash(b, nil, v.GetCurrentScriptHash(), util.Uint160{}, f, -1, 0, DynamicOnUnload) + v.loadScriptWithCallingHash(b, nil, nil, v.GetCurrentScriptHash(), util.Uint160{}, f, -1, 0, DynamicOnUnload) } // LoadScriptWithHash is similar to the LoadScriptWithFlags method, but it also loads @@ -325,19 +326,19 @@ func (v *VM) LoadDynamicScript(b []byte, f callflag.CallFlag) { // accordingly). It's up to the user of this function to make sure the script and hash match // each other. func (v *VM) LoadScriptWithHash(b []byte, hash util.Uint160, f callflag.CallFlag) { - v.loadScriptWithCallingHash(b, nil, v.GetCurrentScriptHash(), hash, f, 1, 0, nil) + v.loadScriptWithCallingHash(b, nil, nil, v.GetCurrentScriptHash(), hash, f, 1, 0, nil) } // LoadNEFMethod allows to create a context to execute a method from the NEF // file with the specified caller and executing hash, call flags, return value, // method and _initialize offsets. -func (v *VM) LoadNEFMethod(exe *nef.File, caller util.Uint160, hash util.Uint160, f callflag.CallFlag, +func (v *VM) LoadNEFMethod(exe *nef.File, manifest *manifest.Manifest, caller util.Uint160, hash util.Uint160, f callflag.CallFlag, hasReturn bool, methodOff int, initOff int, onContextUnload ContextUnloadCallback) { var rvcount int if hasReturn { rvcount = 1 } - v.loadScriptWithCallingHash(exe.Script, exe, caller, hash, f, rvcount, methodOff, onContextUnload) + v.loadScriptWithCallingHash(exe.Script, exe, manifest, caller, hash, f, rvcount, methodOff, onContextUnload) if initOff >= 0 { v.Call(initOff) } @@ -345,7 +346,7 @@ func (v *VM) LoadNEFMethod(exe *nef.File, caller util.Uint160, hash util.Uint160 // loadScriptWithCallingHash is similar to LoadScriptWithHash but sets calling hash explicitly. // It should be used for calling from native contracts. -func (v *VM) loadScriptWithCallingHash(b []byte, exe *nef.File, caller util.Uint160, +func (v *VM) loadScriptWithCallingHash(b []byte, exe *nef.File, manifest *manifest.Manifest, caller util.Uint160, hash util.Uint160, f callflag.CallFlag, rvcount int, offset int, onContextUnload ContextUnloadCallback) { v.checkInvocationStackSize() ctx := NewContextWithParams(b, rvcount, offset) @@ -363,6 +364,7 @@ func (v *VM) loadScriptWithCallingHash(b []byte, exe *nef.File, caller util.Uint ctx.sc.scriptHash = hash ctx.sc.callingScriptHash = caller ctx.sc.NEF = exe + ctx.sc.Manifest = manifest if v.invTree != nil { curTree := v.invTree if parent != nil {