From 23b78b0be2c29a8b303391b927a0d0a8f817bc36 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 6 May 2022 10:27:48 +0300 Subject: [PATCH 1/2] vm: limit ByteArray comparable length Close #2460. --- pkg/vm/stackitem/item.go | 54 +++++++++++++++++++++++++++++-------- pkg/vm/vm_test.go | 57 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 11 deletions(-) diff --git a/pkg/vm/stackitem/item.go b/pkg/vm/stackitem/item.go index 158b0cc5d..d7c92a5fe 100644 --- a/pkg/vm/stackitem/item.go +++ b/pkg/vm/stackitem/item.go @@ -263,19 +263,31 @@ func (i *Struct) equalStruct(s *Struct, limit *int) bool { } else if len(i.value) != len(s.value) { return false } + var maxComparableSize = MaxByteArrayComparableSize for j := range i.value { *limit-- if *limit == 0 { panic(errTooBigElements) } - sa, oka := i.value[j].(*Struct) - sb, okb := s.value[j].(*Struct) - if oka && okb { - if !sa.equalStruct(sb, limit) { + arr, ok := i.value[j].(*ByteArray) + if ok { + if !arr.equalsLimited(s.value[j], &maxComparableSize) { + return false + } + } else { + if maxComparableSize == 0 { + panic(errTooBigComparable) + } + maxComparableSize-- + sa, oka := i.value[j].(*Struct) + sb, okb := s.value[j].(*Struct) + if oka && okb { + if !sa.equalStruct(sb, limit) { + return false + } + } else if !i.value[j].Equals(s.value[j]) { return false } - } else if !i.value[j].Equals(s.value[j]) { - return false } } return true @@ -594,19 +606,39 @@ func (i ByteArray) TryInteger() (*big.Int, error) { // Equals implements the Item interface. func (i *ByteArray) Equals(s Item) bool { - if len(*i) > MaxByteArrayComparableSize { + var limit = MaxByteArrayComparableSize + return i.equalsLimited(s, &limit) +} + +// equalsLimited compares ByteArray with provided stackitem using the limit. +func (i *ByteArray) equalsLimited(s Item, limit *int) bool { + if i == nil { + return s == nil + } + lCurr := len(*i) + if lCurr > *limit || *limit == 0 { panic(errTooBigComparable) } - if i == s { - return true - } else if s == nil { + + var comparedSize = 1 + defer func() { *limit -= comparedSize }() + + if s == nil { return false } val, ok := s.(*ByteArray) if !ok { return false } - if len(*val) > MaxByteArrayComparableSize { + comparedSize = lCurr + lOther := len(*val) + if lOther > comparedSize { + comparedSize = lOther + } + if i == val { + return true + } + if lOther > *limit { panic(errTooBigComparable) } return bytes.Equal(*i, *val) diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index dd6ffdd5b..e342434f4 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -809,6 +809,63 @@ func TestEQUAL(t *testing.T) { t.Run("Buffer", getTestFuncForVM(prog, false, stackitem.NewBuffer([]byte{42}), stackitem.NewBuffer([]byte{42}))) } +func TestEQUALByteArrayWithLimit(t *testing.T) { + prog := makeProgram(opcode.EQUAL) + t.Run("fits limit, equal", func(t *testing.T) { + args := make([]stackitem.Item, 2) + for i := range args { + args[i] = stackitem.NewStruct([]stackitem.Item{ + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2)), + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2+1)), // MaxByteArrayComparableSize is even, thus use +1 + }) + } + getTestFuncForVM(prog, true, args[0], args[1])(t) + }) + t.Run("exceeds limit", func(t *testing.T) { + args := make([]stackitem.Item, 2) + for i := range args { + args[i] = stackitem.NewStruct([]stackitem.Item{ + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2+2)), + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2)), + }) + } + getTestFuncForVM(prog, nil, args[0], args[1])(t) // should FAULT due to comparable limit exceeding + }) + t.Run("fits limit, second elements are not equal", func(t *testing.T) { + args := make([]stackitem.Item, 2) + for i := range args { + args[i] = stackitem.NewStruct([]stackitem.Item{ + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize-1)), + stackitem.NewBuffer(make([]byte, 1)), + }) + } + getTestFuncForVM(prog, false, args[0], args[1])(t) // no limit is exceeded, but the second struct item is a Buffer. + }) + t.Run("fits limit, equal", func(t *testing.T) { + args := make([]stackitem.Item, 2) + buf := stackitem.NewBuffer(make([]byte, 100500)) // takes only 1 comparable unit despite its length + for i := range args { + args[i] = stackitem.NewStruct([]stackitem.Item{ + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize-1)), + buf, + }) + } + getTestFuncForVM(prog, true, args[0], args[1])(t) // should HALT, because no limit exceeded + }) + t.Run("exceeds limit, equal", func(t *testing.T) { + args := make([]stackitem.Item, 2) + buf := stackitem.NewBuffer(make([]byte, 100500)) // takes only 1 comparable unit despite its length + for i := range args { + args[i] = stackitem.NewStruct([]stackitem.Item{ + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize-1)), // MaxByteArrayComparableSize-1 comparable units + buf, // 1 comparable unit + buf, // 1 comparable unit + }) + } + getTestFuncForVM(prog, nil, args[0], args[1])(t) // should FAULT, because limit is exceeded: + }) +} + func runWithArgs(t *testing.T, prog []byte, result interface{}, args ...interface{}) { getTestFuncForVM(prog, result, args...)(t) } From 8af043520dfff2f4b7df35a77fc6b53210298d4a Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 6 May 2022 13:29:00 +0300 Subject: [PATCH 2/2] vm: adjust MaxByteArrayComparableSize value https://github.com/neo-project/neo-vm/pull/454#discussion_r857533800. --- pkg/vm/stackitem/item.go | 4 ++-- pkg/vm/vm_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/vm/stackitem/item.go b/pkg/vm/stackitem/item.go index d7c92a5fe..18afb7349 100644 --- a/pkg/vm/stackitem/item.go +++ b/pkg/vm/stackitem/item.go @@ -27,8 +27,8 @@ const ( // MaxClonableNumOfItems is the maximum number of items that can be cloned in structs. MaxClonableNumOfItems = MaxDeserialized // MaxByteArrayComparableSize is the maximum allowed length of a ByteArray for Equals method. - // It is set to be the maximum uint16 value. - MaxByteArrayComparableSize = math.MaxUint16 + // It is set to be the maximum uint16 value + 1. + MaxByteArrayComparableSize = math.MaxUint16 + 1 // MaxKeySize is the maximum size of a map key. MaxKeySize = 64 ) diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index e342434f4..ce2d9375e 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -816,7 +816,7 @@ func TestEQUALByteArrayWithLimit(t *testing.T) { for i := range args { args[i] = stackitem.NewStruct([]stackitem.Item{ stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2)), - stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2+1)), // MaxByteArrayComparableSize is even, thus use +1 + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2)), }) } getTestFuncForVM(prog, true, args[0], args[1])(t) @@ -825,7 +825,7 @@ func TestEQUALByteArrayWithLimit(t *testing.T) { args := make([]stackitem.Item, 2) for i := range args { args[i] = stackitem.NewStruct([]stackitem.Item{ - stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2+2)), + stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2+1)), stackitem.NewByteArray(make([]byte, stackitem.MaxByteArrayComparableSize/2)), }) }