Merge pull request #1433 from nspcc-dev/vm/maxcomparablesize

vm: add MaxComparableSize and check reference counter after PACK-UNPACK

docs/compiler.md

@@ -24,6 +24,7 @@ a dialect of Go rather than a complete port of the language:
     overhead for all contracts. This can easily be mitigated by first storing values
     in variables and returning the result.
  * lambdas are supported, but closures are not.
+ * maps are supported, but valid map keys are booleans, integers and strings with length <= 64
 
 ## VM API (interop layer)
 Compiler translates interop function calls into NEO VM syscalls or (for custom

pkg/vm/stackitem/item.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"math/big"
 	"reflect"
 	"unicode/utf8"
@@ -16,14 +17,19 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/util"
 )
 
-// MaxBigIntegerSizeBits is the maximum size of BigInt item in bits.
+const (
-const MaxBigIntegerSizeBits = 32 * 8
+	// MaxBigIntegerSizeBits is the maximum size of BigInt item in bits.
-
+	MaxBigIntegerSizeBits = 32 * 8
-// MaxArraySize is the maximum array size allowed in the VM.
+	// MaxArraySize is the maximum array size allowed in the VM.
-const MaxArraySize = 1024
+	MaxArraySize = 1024
-
+	// MaxSize is the maximum item size allowed in the VM.
-// MaxSize is the maximum item size allowed in the VM.
+	MaxSize = 1024 * 1024
-const MaxSize = 1024 * 1024
+	// MaxByteArrayComparableSize is the maximum allowed length of ByteArray for Equals method.
+	// It is set to be the maximum uint16 value.
+	MaxByteArrayComparableSize = math.MaxUint16
+	// MaxKeySize is the maximum size of map key.
+	MaxKeySize = 64
+)
 
 // Item represents the "real" value that is pushed on the stack.
 type Item interface {
@@ -46,7 +52,10 @@ type Item interface {
 	Convert(Type) (Item, error)
 }
 
-var errInvalidConversion = errors.New("invalid conversion type")
+var (
+	errInvalidConversion          = errors.New("invalid conversion type")
+	errExceedingMaxComparableSize = errors.New("the operand exceeds the maximum comparable size")
+)
 
 // Make tries to make appropriate stack item from provided value.
 // It will panic if it's not possible.
@@ -536,13 +545,22 @@ func (i *ByteArray) TryInteger() (*big.Int, error) {
 
 // Equals implements Item interface.
 func (i *ByteArray) Equals(s Item) bool {
+	if len(i.value) > MaxByteArrayComparableSize {
+		panic(errExceedingMaxComparableSize)
+	}
 	if i == s {
 		return true
 	} else if s == nil {
 		return false
 	}
 	val, ok := s.(*ByteArray)
-	return ok && bytes.Equal(i.value, val.value)
+	if !ok {
+		return false
+	}
+	if len(val.value) > MaxByteArrayComparableSize {
+		panic(errExceedingMaxComparableSize)
+	}
+	return bytes.Equal(i.value, val.value)
 }
 
 // Dup implements Item interface.
@@ -757,8 +775,8 @@ func (i *Map) Convert(typ Type) (Item, error) {
 
 // Add adds key-value pair to the map.
 func (i *Map) Add(key, value Item) {
-	if !IsValidMapKey(key) {
+	if err := IsValidMapKey(key); err != nil {
-		panic("wrong key type")
+		panic(err)
 	}
 	index := i.Index(key)
 	if index >= 0 {
@@ -776,12 +794,18 @@ func (i *Map) Drop(index int) {
 
 // IsValidMapKey checks whether it's possible to use given Item as a Map
 // key.
-func IsValidMapKey(key Item) bool {
+func IsValidMapKey(key Item) error {
 	switch key.(type) {
-	case *Bool, *BigInteger, *ByteArray:
+	case *Bool, *BigInteger:
-		return true
+		return nil
+	case *ByteArray:
+		size := len(key.Value().([]byte))
+		if size > MaxKeySize {
+			return fmt.Errorf("invalid map key size: %d", size)
+		}
+		return nil
 	default:
-		return false
+		return fmt.Errorf("invalid map key of type %s", key.Type())
 	}
 }
 

pkg/vm/stackitem/item_test.go

@@ -143,6 +143,7 @@ var equalsTestCases = map[string][]struct {
 	item1  Item
 	item2  Item
 	result bool
+	panics bool
 }{
 	"struct": {
 		{
@@ -251,6 +252,21 @@ var equalsTestCases = map[string][]struct {
 			item2:  Make([]byte{1, 2, 3}),
 			result: true,
 		},
+		{
+			item1:  NewByteArray(make([]byte, MaxByteArrayComparableSize+1)),
+			item2:  NewByteArray([]byte{1, 2, 3}),
+			panics: true,
+		},
+		{
+			item1:  NewByteArray([]byte{1, 2, 3}),
+			item2:  NewByteArray(make([]byte, MaxByteArrayComparableSize+1)),
+			panics: true,
+		},
+		{
+			item1:  NewByteArray(make([]byte, MaxByteArrayComparableSize+1)),
+			item2:  NewByteArray(make([]byte, MaxByteArrayComparableSize+1)),
+			panics: true,
+		},
 	},
 	"array": {
 		{
@@ -350,9 +366,15 @@ func TestEquals(t *testing.T) {
 	for name, testBatch := range equalsTestCases {
 		for _, testCase := range testBatch {
 			t.Run(name, func(t *testing.T) {
-				assert.Equal(t, testCase.result, testCase.item1.Equals(testCase.item2))
+				if testCase.panics {
-				// Reference equals
+					assert.Panics(t, func() {
-				assert.Equal(t, true, testCase.item1.Equals(testCase.item1))
+						testCase.item1.Equals(testCase.item2)
+					})
+				} else {
+					assert.Equal(t, testCase.result, testCase.item1.Equals(testCase.item2))
+					// Reference equals
+					assert.Equal(t, true, testCase.item1.Equals(testCase.item1))
+				}
 			})
 		}
 	}

pkg/vm/stackitem/json.go

@@ -360,8 +360,8 @@ func FromJSONWithTypes(data []byte) (Item, error) {
 			key, err := FromJSONWithTypes(arr[i].Key)
 			if err != nil {
 				return nil, err
-			} else if !IsValidMapKey(key) {
+			} else if err = IsValidMapKey(key); err != nil {
-				return nil, fmt.Errorf("invalid map key of type %s", key.Type())
+				return nil, err
 			}
 			value, err := FromJSONWithTypes(arr[i].Value)
 			if err != nil {

pkg/vm/stackitem/json_test.go

@@ -88,7 +88,7 @@ func TestFromToJSON(t *testing.T) {
 		t.Run("BigNestedArray", getTestDecodeFunc(`[[[[[[[[[[[]]]]]]]]]]]`, nil))
 		t.Run("EncodeRecursive", func(t *testing.T) {
 			// add this item to speed up test a bit
-			item := NewByteArray(make([]byte, MaxSize/100))
+			item := NewByteArray(make([]byte, MaxKeySize))
 			t.Run("Array", func(t *testing.T) {
 				arr := NewArray([]Item{item})
 				arr.Append(arr)

pkg/vm/vm.go

@@ -531,8 +531,8 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 		v.estack.PushVal(stackitem.Null{})
 
 	case opcode.ISNULL:
-		res := v.estack.Pop().value.Equals(stackitem.Null{})
+		_, ok := v.estack.Pop().value.(stackitem.Null)
-		v.estack.PushVal(res)
+		v.estack.PushVal(ok)
 
 	case opcode.ISTYPE:
 		res := v.estack.Pop().Item()
@@ -1668,8 +1668,8 @@ func validateMapKey(key *Element) {
 	if key == nil {
 		panic("no key found")
 	}
-	if !stackitem.IsValidMapKey(key.Item()) {
+	if err := stackitem.IsValidMapKey(key.Item()); err != nil {
-		panic("key can't be a collection")
+		panic(err)
 	}
 }
 

pkg/vm/vm_test.go

@@ -2031,6 +2031,52 @@ func TestPACKGood(t *testing.T) {
 	assert.Equal(t, int64(1), vm.estack.Peek(1).BigInt().Int64())
 }
 
+func TestPACK_UNPACK_MaxSize(t *testing.T) {
+	prog := makeProgram(opcode.PACK, opcode.UNPACK)
+	elements := make([]int, stackitem.MaxArraySize)
+	vm := load(prog)
+	// canary
+	vm.estack.PushVal(1)
+	for i := len(elements) - 1; i >= 0; i-- {
+		vm.estack.PushVal(elements[i])
+	}
+	vm.estack.PushVal(len(elements))
+	runVM(t, vm)
+	// check reference counter = 1+1+1024
+	assert.Equal(t, 1+1+len(elements), vm.refs.size)
+	assert.Equal(t, 1+1+len(elements), vm.estack.Len()) // canary + length + elements
+	assert.Equal(t, int64(len(elements)), vm.estack.Peek(0).Value().(*big.Int).Int64())
+	for i := 0; i < len(elements); i++ {
+		e, ok := vm.estack.Peek(i + 1).Value().(*big.Int)
+		assert.True(t, ok)
+		assert.Equal(t, int64(elements[i]), e.Int64())
+	}
+	assert.Equal(t, int64(1), vm.estack.Peek(1+len(elements)).BigInt().Int64())
+}
+
+func TestPACK_UNPACK_PACK_MaxSize(t *testing.T) {
+	prog := makeProgram(opcode.PACK, opcode.UNPACK, opcode.PACK)
+	elements := make([]int, stackitem.MaxArraySize)
+	vm := load(prog)
+	// canary
+	vm.estack.PushVal(1)
+	for i := len(elements) - 1; i >= 0; i-- {
+		vm.estack.PushVal(elements[i])
+	}
+	vm.estack.PushVal(len(elements))
+	runVM(t, vm)
+	// check reference counter = 1+1+1024
+	assert.Equal(t, 1+1+len(elements), vm.refs.size)
+	assert.Equal(t, 2, vm.estack.Len())
+	a := vm.estack.Peek(0).Array()
+	assert.Equal(t, len(elements), len(a))
+	for i := 0; i < len(elements); i++ {
+		e := a[i].Value().(*big.Int)
+		assert.Equal(t, int64(elements[i]), e.Int64())
+	}
+	assert.Equal(t, int64(1), vm.estack.Peek(1).BigInt().Int64())
+}
+
 func TestUNPACKBadNotArray(t *testing.T) {
 	prog := makeProgram(opcode.UNPACK)
 	runWithArgs(t, prog, nil, 1)