Merge pull request #2538 from nspcc-dev/restrict-out-of-bounds

core: check methods offsets while contract deploying

pkg/core/native/management.go

@@ -614,7 +614,7 @@ func checkScriptAndMethods(script []byte, methods []manifest.Method) error {
 	offsets := bitfield.New(l)
 	for i := range methods {
 		if methods[i].Offset >= l {
-			continue
+			return fmt.Errorf("method %s/%d: offset is out of the script range", methods[i].Name, len(methods[i].Parameters))
 		}
 		offsets.Set(methods[i].Offset)
 	}

pkg/core/native/native_test/management_test.go

@@ -135,19 +135,11 @@ func TestManagement_ContractDeploy(t *testing.T) {
 		badManifest := cs1.Manifest
 		badManifest.ABI.Methods = make([]manifest.Method, len(cs1.Manifest.ABI.Methods))
 		copy(badManifest.ABI.Methods, cs1.Manifest.ABI.Methods)
-		badManifest.ABI.Methods[0].Offset = 100500 // out of bounds, but it's OK, this method will not be checked then.
+		badManifest.ABI.Methods[0].Offset = 100500 // out of bounds
 		manifB, err := json.Marshal(&badManifest)
 		require.NoError(t, err)
 
-		tx := c.PrepareInvokeNoSign(t, "deploy", nefBytes, manifB)
+		managementInvoker.InvokeFail(t, "method add/2: offset is out of the script range", "deploy", nefBytes, manifB)
-		tx.Signers = []transaction.Signer{{}} // Need dummy signer to deploy.
-		b := c.NewUnsignedBlock(t, tx)
-		ic := c.Chain.GetTestVM(trigger.Application, tx, b)
-		t.Cleanup(ic.Finalize)
-
-		ic.VM.LoadWithFlags(tx.Script, callflag.All)
-		err = ic.VM.Run()
-		require.NoError(t, err)
 	})
 	t.Run("bad methods in manifest 2", func(t *testing.T) {
 		var badManifest = cs1.Manifest

pkg/vm/context.go

@@ -91,7 +91,7 @@ func (c *Context) NextIP() int {
 
 // Jump unconditionally moves the next instruction pointer to the specified location.
 func (c *Context) Jump(pos int) {
-	if pos < 0 || pos > len(c.prog) {
+	if pos < 0 || pos >= len(c.prog) {
 		panic("instruction offset is out of range")
 	}
 	c.nextip = pos