From 233307aca536c9d86e11159e1d61de5544280676 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 19 Jul 2021 13:35:48 +0300 Subject: [PATCH] stackitem: completely drop MaxArraySize Turns out C# VM doesn't have it since preview2, so our limiting of MaxArraySize in incompatible with it. Removing this limit shouldn't be a problem with the reference counter we have, both APPEND and SETITEM add things to reference counter and we can't exceed MaxStackSize. PACK on the other hand can't get more than MaxStackSize-1 of input elements. Unify NEWSTRUCT with NEWARRAY* and use better integer checks at the same time. Multisig limit is still 1024. --- pkg/compiler/assign_test.go | 6 ++-- pkg/core/state/notification_event.go | 2 +- pkg/vm/contract_checks.go | 9 ++++-- pkg/vm/stackitem/item.go | 3 -- pkg/vm/stackitem/serialization.go | 8 +++--- pkg/vm/stackitem/serialization_test.go | 12 ++++---- pkg/vm/vm.go | 38 +++++++++---------------- pkg/vm/vm_test.go | 39 +++++++++++--------------- 8 files changed, 50 insertions(+), 67 deletions(-) diff --git a/pkg/compiler/assign_test.go b/pkg/compiler/assign_test.go index 4c7007a0f..182224ea4 100644 --- a/pkg/compiler/assign_test.go +++ b/pkg/compiler/assign_test.go @@ -3,8 +3,6 @@ package compiler_test import ( "math/big" "testing" - - "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) var assignTestCases = []testCase{ @@ -154,9 +152,9 @@ func TestManyAssignments(t *testing.T) { src2 := `return a }` - for i := 0; i < stackitem.MaxArraySize; i++ { + for i := 0; i < 1024; i++ { src1 += "a += 1\n" } - eval(t, src1+src2, big.NewInt(stackitem.MaxArraySize)) + eval(t, src1+src2, big.NewInt(1024)) } diff --git a/pkg/core/state/notification_event.go b/pkg/core/state/notification_event.go index 87ea9dbb1..5c7962ca0 100644 --- a/pkg/core/state/notification_event.go +++ b/pkg/core/state/notification_event.go @@ -72,7 +72,7 @@ func (aer *AppExecResult) DecodeBinary(r *io.BinReader) { aer.VMState = vm.State(r.ReadB()) aer.GasConsumed = int64(r.ReadU64LE()) sz := r.ReadVarUint() - if stackitem.MaxArraySize < sz && r.Err == nil { + if stackitem.MaxDeserialized < sz && r.Err == nil { r.Err = errors.New("invalid format") } if r.Err != nil { diff --git a/pkg/vm/contract_checks.go b/pkg/vm/contract_checks.go index 9841ffb5b..40b7d6289 100644 --- a/pkg/vm/contract_checks.go +++ b/pkg/vm/contract_checks.go @@ -12,6 +12,9 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) +// MaxMultisigKeys is the maximum number of used keys for correct multisig contract. +const MaxMultisigKeys = 1024 + var ( verifyInteropID = interopnames.ToID([]byte(interopnames.SystemCryptoCheckSig)) multisigInteropID = interopnames.ToID([]byte(interopnames.SystemCryptoCheckMultisig)) @@ -25,14 +28,14 @@ func getNumOfThingsFromInstr(instr opcode.Opcode, param []byte) (int, bool) { nthings = int(instr-opcode.PUSH1) + 1 case instr <= opcode.PUSHINT256: n := bigint.FromBytes(param) - if !n.IsInt64() || n.Int64() > stackitem.MaxArraySize { + if !n.IsInt64() || n.Sign() < 0 || n.Int64() > MaxMultisigKeys { return 0, false } nthings = int(n.Int64()) default: return 0, false } - if nthings < 1 || nthings > stackitem.MaxArraySize { + if nthings < 1 || nthings > MaxMultisigKeys { return 0, false } return nthings, true @@ -76,7 +79,7 @@ func ParseMultiSigContract(script []byte) (int, [][]byte, bool) { } pubs = append(pubs, param) nkeys++ - if nkeys > stackitem.MaxArraySize { + if nkeys > MaxMultisigKeys { return nsigs, nil, false } } diff --git a/pkg/vm/stackitem/item.go b/pkg/vm/stackitem/item.go index 3f554f0b1..b4efa174b 100644 --- a/pkg/vm/stackitem/item.go +++ b/pkg/vm/stackitem/item.go @@ -20,8 +20,6 @@ import ( const ( // MaxBigIntegerSizeBits is the maximum size of BigInt item in bits. MaxBigIntegerSizeBits = 32 * 8 - // MaxArraySize is the maximum array size allowed in the VM. - MaxArraySize = 1024 // 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. @@ -71,7 +69,6 @@ var ( // value exceeds MaxSize. ErrTooBig = errors.New("too big") - errTooBigArray = fmt.Errorf("%w: array", ErrTooBig) errTooBigComparable = fmt.Errorf("%w: uncomparable", ErrTooBig) errTooBigInteger = fmt.Errorf("%w: integer", ErrTooBig) errTooBigKey = fmt.Errorf("%w: map key", ErrTooBig) diff --git a/pkg/vm/stackitem/serialization.go b/pkg/vm/stackitem/serialization.go index c4a8afac8..b9a814e46 100644 --- a/pkg/vm/stackitem/serialization.go +++ b/pkg/vm/stackitem/serialization.go @@ -236,8 +236,8 @@ func (r *deserContext) decodeBinary() Item { return NewBigInteger(num) case ArrayT, StructT: size := int(r.ReadVarUint()) - if size > MaxArraySize { - r.Err = errTooBigArray + if size > MaxDeserialized { + r.Err = errTooBigElements return nil } arr := make([]Item, size) @@ -251,8 +251,8 @@ func (r *deserContext) decodeBinary() Item { return NewStruct(arr) case MapT: size := int(r.ReadVarUint()) - if size > MaxArraySize { - r.Err = errTooBigArray + if size > MaxDeserialized { + r.Err = errTooBigElements return nil } m := NewMap() diff --git a/pkg/vm/stackitem/serialization_test.go b/pkg/vm/stackitem/serialization_test.go index fc0a88338..139fbcbcf 100644 --- a/pkg/vm/stackitem/serialization_test.go +++ b/pkg/vm/stackitem/serialization_test.go @@ -52,8 +52,8 @@ func TestSerialize(t *testing.T) { arr.Value().([]Item)[0] = arr testSerialize(t, ErrRecursive, arr) - items := make([]Item, 0, MaxArraySize) - for i := 0; i < MaxArraySize; i++ { + items := make([]Item, 0, MaxDeserialized-1) + for i := 0; i < MaxDeserialized-1; i++ { items = append(items, zeroByteArray) } testSerialize(t, nil, newItem(items)) @@ -141,12 +141,14 @@ func TestSerialize(t *testing.T) { testSerialize(t, ErrTooBig, m) m = NewMap() - for i := 0; i < MaxArraySize; i++ { + for i := 0; i < MaxDeserialized/2-1; i++ { m.Add(Make(i), zeroByteArray) } - // testSerialize(t, nil, m) // It contains too many elements already, so ErrTooBig. + testSerialize(t, nil, m) - m.Add(Make(100500), zeroByteArray) + for i := 0; i <= MaxDeserialized; i++ { + m.Add(Make(i), zeroByteArray) + } data, err := Serialize(m) require.NoError(t, err) _, err = Deserialize(data) diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 09c00699d..0d65f3bf2 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -1027,31 +1027,27 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro case opcode.NEWARRAY0: v.estack.PushVal(stackitem.NewArray([]stackitem.Item{})) - case opcode.NEWARRAY, opcode.NEWARRAYT: - item := v.estack.Pop() - n := item.BigInt().Int64() - if n > stackitem.MaxArraySize { - panic("too long array") + case opcode.NEWARRAY, opcode.NEWARRAYT, opcode.NEWSTRUCT: + n := toInt(v.estack.Pop().BigInt()) + if n < 0 || n > MaxStackSize { + panic("wrong number of elements") } typ := stackitem.AnyT if op == opcode.NEWARRAYT { typ = stackitem.Type(parameter[0]) } items := makeArrayOfType(int(n), typ) - v.estack.PushVal(stackitem.NewArray(items)) + var res stackitem.Item + if op == opcode.NEWSTRUCT { + res = stackitem.NewStruct(items) + } else { + res = stackitem.NewArray(items) + } + v.estack.PushVal(res) case opcode.NEWSTRUCT0: v.estack.PushVal(stackitem.NewStruct([]stackitem.Item{})) - case opcode.NEWSTRUCT: - item := v.estack.Pop() - n := item.BigInt().Int64() - if n > stackitem.MaxArraySize { - panic("too long struct") - } - items := makeArrayOfType(int(n), stackitem.AnyT) - v.estack.PushVal(stackitem.NewStruct(items)) - case opcode.APPEND: itemElem := v.estack.Pop() arrElem := v.estack.Pop() @@ -1060,14 +1056,8 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro switch t := arrElem.value.(type) { case *stackitem.Array: - if t.Len() >= stackitem.MaxArraySize { - panic("too long array") - } t.Append(val) case *stackitem.Struct: - if t.Len() >= stackitem.MaxArraySize { - panic("too long struct") - } t.Append(val) default: panic("APPEND: not of underlying type Array") @@ -1076,8 +1066,8 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro v.refs.Add(val) case opcode.PACK: - n := int(v.estack.Pop().BigInt().Int64()) - if n < 0 || n > v.estack.Len() || n > stackitem.MaxArraySize { + n := toInt(v.estack.Pop().BigInt()) + if n < 0 || n > v.estack.Len() { panic("OPACK: invalid length") } @@ -1148,8 +1138,6 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro case *stackitem.Map: if i := t.Index(key.value); i >= 0 { v.refs.Remove(t.Value().([]stackitem.MapElement)[i].Value) - } else if t.Len() >= stackitem.MaxArraySize { - panic("too big map") } t.Add(key.value, item) v.refs.Add(item) diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index dffe8e4d1..8fb07646f 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -1057,7 +1057,7 @@ func TestNEWSTRUCT0(t *testing.T) { func TestNEWARRAYArray(t *testing.T) { prog := makeProgram(opcode.NEWARRAY) t.Run("ByteArray", getTestFuncForVM(prog, stackitem.NewArray([]stackitem.Item{}), []byte{})) - t.Run("BadSize", getTestFuncForVM(prog, nil, stackitem.MaxArraySize+1)) + t.Run("BadSize", getTestFuncForVM(prog, nil, MaxStackSize+1)) t.Run("Integer", getTestFuncForVM(prog, []stackitem.Item{stackitem.Null{}}, 1)) } @@ -1108,7 +1108,7 @@ func TestNEWARRAYT(t *testing.T) { func TestNEWSTRUCT(t *testing.T) { prog := makeProgram(opcode.NEWSTRUCT) t.Run("ByteArray", getTestFuncForVM(prog, stackitem.NewStruct([]stackitem.Item{}), []byte{})) - t.Run("BadSize", getTestFuncForVM(prog, nil, stackitem.MaxArraySize+1)) + t.Run("BadSize", getTestFuncForVM(prog, nil, MaxStackSize+1)) t.Run("Integer", getTestFuncForVM(prog, stackitem.NewStruct([]stackitem.Item{stackitem.Null{}}), 1)) } @@ -1136,15 +1136,20 @@ func TestAPPENDBad(t *testing.T) { func TestAPPENDGoodSizeLimit(t *testing.T) { prog := makeProgram(opcode.NEWARRAY, opcode.DUP, opcode.PUSH0, opcode.APPEND) vm := load(prog) - vm.estack.PushVal(stackitem.MaxArraySize - 1) + vm.estack.PushVal(MaxStackSize - 3) // 1 for array, 1 for copy, 1 for pushed 0. runVM(t, vm) assert.Equal(t, 1, vm.estack.Len()) - assert.Equal(t, stackitem.MaxArraySize, len(vm.estack.Pop().Array())) + assert.Equal(t, MaxStackSize-2, len(vm.estack.Pop().Array())) } func TestAPPENDBadSizeLimit(t *testing.T) { prog := makeProgram(opcode.NEWARRAY, opcode.DUP, opcode.PUSH0, opcode.APPEND) - runWithArgs(t, prog, nil, stackitem.MaxArraySize) + runWithArgs(t, prog, nil, MaxStackSize) +} + +func TestAPPENDRefSizeLimit(t *testing.T) { + prog := makeProgram(opcode.NEWARRAY0, opcode.DUP, opcode.DUP, opcode.APPEND, opcode.JMP, 0xfd) + runWithArgs(t, prog, nil) } func TestPICKITEM(t *testing.T) { @@ -1205,19 +1210,19 @@ func TestSETITEMMap(t *testing.T) { func TestSETITEMBigMapBad(t *testing.T) { prog := makeProgram(opcode.SETITEM) m := stackitem.NewMap() - for i := 0; i < stackitem.MaxArraySize; i++ { + for i := 0; i < MaxStackSize; i++ { m.Add(stackitem.Make(i), stackitem.Make(i)) } - runWithArgs(t, prog, nil, m, stackitem.MaxArraySize, 0) + runWithArgs(t, prog, nil, m, m, MaxStackSize, 0) } // This test checks is SETITEM properly updates reference counter. -// 1. Create 2 arrays of size MaxArraySize - 3. (MaxStackSize = 2 * MaxArraySize) +// 1. Create 2 arrays of size MaxStackSize/2 - 3. // 2. SETITEM each of them to a map. // 3. Replace each of them with a scalar value. func TestSETITEMMapStackLimit(t *testing.T) { - size := stackitem.MaxArraySize - 3 + size := MaxStackSize/2 - 3 m := stackitem.NewMap() m.Add(stackitem.NewBigInteger(big.NewInt(1)), stackitem.NewArray(makeArrayOfType(size, stackitem.BooleanT))) m.Add(stackitem.NewBigInteger(big.NewInt(2)), stackitem.NewArray(makeArrayOfType(size, stackitem.BooleanT))) @@ -1237,7 +1242,7 @@ func TestSETITEMBigMapGood(t *testing.T) { vm := load(prog) m := stackitem.NewMap() - for i := 0; i < stackitem.MaxArraySize; i++ { + for i := 0; i < MaxStackSize-3; i++ { m.Add(stackitem.Make(i), stackitem.Make(i)) } vm.estack.Push(&Element{value: m}) @@ -1723,16 +1728,6 @@ func TestPACK(t *testing.T) { t.Run("Good0Len", getTestFuncForVM(prog, []stackitem.Item{}, 0)) } -func TestPACKBigLen(t *testing.T) { - prog := makeProgram(opcode.PACK) - vm := load(prog) - for i := 0; i <= stackitem.MaxArraySize; i++ { - vm.estack.PushVal(0) - } - vm.estack.PushVal(stackitem.MaxArraySize + 1) - checkVMFailed(t, vm) -} - func TestPACKGood(t *testing.T) { prog := makeProgram(opcode.PACK) elements := []int{55, 34, 42} @@ -1756,7 +1751,7 @@ func TestPACKGood(t *testing.T) { func TestPACK_UNPACK_MaxSize(t *testing.T) { prog := makeProgram(opcode.PACK, opcode.UNPACK) - elements := make([]int, stackitem.MaxArraySize) + elements := make([]int, MaxStackSize-2) vm := load(prog) // canary vm.estack.PushVal(1) @@ -1779,7 +1774,7 @@ func TestPACK_UNPACK_MaxSize(t *testing.T) { func TestPACK_UNPACK_PACK_MaxSize(t *testing.T) { prog := makeProgram(opcode.PACK, opcode.UNPACK, opcode.PACK) - elements := make([]int, stackitem.MaxArraySize) + elements := make([]int, MaxStackSize-2) vm := load(prog) // canary vm.estack.PushVal(1)