stackitem: rework struct cloning protection

Count everything, fail early, make it more compatible with
neo-project/neo-vm#423.

pkg/vm/stackitem/item.go

@@ -23,7 +23,9 @@ const (
 	// MaxSize is the maximum item size allowed in the VM.
 	MaxSize = 1024 * 1024
 	// MaxComparableNumOfItems is the maximum number of items that can be compared for structs.
-	MaxComparableNumOfItems = 2048
+	MaxComparableNumOfItems = MaxDeserialized
+	// MaxClonableNumOfItems is the maximum number of items that can be cloned in structs.
+	MaxClonableNumOfItems = MaxDeserialized
 	// MaxByteArrayComparableSize is the maximum allowed length of ByteArray for Equals method.
 	// It is set to be the maximum uint16 value.
 	MaxByteArrayComparableSize = math.MaxUint16
@@ -323,13 +325,18 @@ func (i *Struct) Convert(typ Type) (Item, error) {
 
 // Clone returns a Struct with all Struct fields copied by value.
 // Array fields are still copied by reference.
-func (i *Struct) Clone(limit int) (*Struct, error) {
+func (i *Struct) Clone() (*Struct, error) {
+	var limit = MaxClonableNumOfItems - 1 // For this struct itself.
 	return i.clone(&limit)
 }
 
 func (i *Struct) clone(limit *int) (*Struct, error) {
 	ret := &Struct{make([]Item, len(i.value))}
 	for j := range i.value {
+		*limit--
+		if *limit < 0 {
+			return nil, ErrTooBig
+		}
 		switch t := i.value[j].(type) {
 		case *Struct:
 			var err error
@@ -338,13 +345,9 @@ func (i *Struct) clone(limit *int) (*Struct, error) {
 			if err != nil {
 				return nil, err
 			}
-			*limit--
 		default:
 			ret.value[j] = t
 		}
-		if *limit < 0 {
-			return nil, ErrTooBig
-		}
 	}
 	return ret, nil
 }

pkg/vm/stackitem/item_test.go

@@ -397,7 +397,7 @@ func TestEqualsDeepStructure(t *testing.T) {
 	var layerUp = func(sa *Struct, num int) (*Struct, int) {
 		items := []Item{}
 		for i := 0; i < perStruct; i++ {
-			clon, err := sa.Clone(100500)
+			clon, err := sa.Clone()
 			require.NoError(t, err)
 			items = append(items, clon)
 		}
@@ -410,14 +410,13 @@ func TestEqualsDeepStructure(t *testing.T) {
 		sa, num = layerUp(sa, num)
 	}
 	require.Less(t, num, MaxComparableNumOfItems)
-	sb, err := sa.Clone(num)
+	sb, err := sa.Clone()
 	require.NoError(t, err)
 	require.True(t, sa.Equals(sb))
 	sa, num = layerUp(sa, num)
+	sb, num = layerUp(sb, num)
 
 	require.Less(t, MaxComparableNumOfItems, num)
-	sb, err = sa.Clone(num)
-	require.NoError(t, err)
 	require.Panics(t, func() { sa.Equals(sb) })
 }
 
@@ -508,9 +507,12 @@ func TestNewVeryBigInteger(t *testing.T) {
 func TestStructClone(t *testing.T) {
 	st0 := Struct{}
 	st := Struct{value: []Item{&st0}}
-	_, err := st.Clone(1)
-	require.NoError(t, err)
-	_, err = st.Clone(0)
+	for i := 0; i < MaxClonableNumOfItems-1; i++ {
+		nst, err := st.Clone()
+		require.NoError(t, err)
+		st = Struct{value: []Item{nst}}
+	}
+	_, err := st.Clone()
 	require.Error(t, err)
 }
 

pkg/vm/vm.go

@@ -1052,7 +1052,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 		itemElem := v.estack.Pop()
 		arrElem := v.estack.Pop()
 
-		val := cloneIfStruct(itemElem.value, MaxStackSize-v.refs.size)
+		val := cloneIfStruct(itemElem.value)
 
 		switch t := arrElem.value.(type) {
 		case *stackitem.Array:
@@ -1358,12 +1358,12 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 			src := t.Value().([]stackitem.Item)
 			arr = make([]stackitem.Item, len(src))
 			for i := range src {
-				arr[i] = cloneIfStruct(src[i], MaxStackSize-v.refs.size)
+				arr[i] = cloneIfStruct(src[i])
 			}
 		case *stackitem.Map:
 			arr = make([]stackitem.Item, 0, t.Len())
 			for k := range t.Value().([]stackitem.MapElement) {
-				arr = append(arr, cloneIfStruct(t.Value().([]stackitem.MapElement)[k].Value, MaxStackSize-v.refs.size))
+				arr = append(arr, cloneIfStruct(t.Value().([]stackitem.MapElement)[k].Value))
 			}
 		default:
 			panic("not a Map, Array or Struct")
@@ -1729,10 +1729,10 @@ func checkMultisig1(v *VM, curve elliptic.Curve, h []byte, pkeys [][]byte, sig [
 	return false
 }
 
-func cloneIfStruct(item stackitem.Item, limit int) stackitem.Item {
+func cloneIfStruct(item stackitem.Item) stackitem.Item {
 	switch it := item.(type) {
 	case *stackitem.Struct:
-		ret, err := it.Clone(limit)
+		ret, err := it.Clone()
 		if err != nil {
 			panic(err)
 		}