diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go index 8256ad16a..a7e40b18b 100644 --- a/pkg/core/blockchain_test.go +++ b/pkg/core/blockchain_test.go @@ -1539,3 +1539,17 @@ func TestRemoveUntraceable(t *testing.T) { require.NoError(t, err) require.Len(t, b.Transactions, 0) } + +func TestInvalidNotification(t *testing.T) { + bc := newTestChain(t) + defer bc.Close() + + cs, _ := getTestContractState(bc) + require.NoError(t, bc.contracts.Management.PutContractState(bc.dao, cs)) + + aer, err := invokeContractMethod(bc, 1_00000000, cs.Hash, "invalidStack") + require.NoError(t, err) + require.Equal(t, 2, len(aer.Stack)) + require.Nil(t, aer.Stack[0]) + require.Equal(t, stackitem.InteropT, aer.Stack[1].Type()) +} diff --git a/pkg/core/interop_system_test.go b/pkg/core/interop_system_test.go index 19781c46e..1879ad916 100644 --- a/pkg/core/interop_system_test.go +++ b/pkg/core/interop_system_test.go @@ -506,6 +506,10 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) { emit.String(w.BinWriter, "destroy") emit.AppCall(w.BinWriter, mgmtHash) emit.Opcodes(w.BinWriter, opcode.RET) + invalidStackOff := w.Len() + emit.Opcodes(w.BinWriter, opcode.NEWARRAY0, opcode.DUP, opcode.DUP, opcode.APPEND, opcode.NEWMAP) + emit.Syscall(w.BinWriter, interopnames.SystemIteratorCreate) + emit.Opcodes(w.BinWriter, opcode.RET) script := w.Bytes() h := hash.Hash160(script) @@ -604,6 +608,11 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) { Offset: destroyOff, ReturnType: smartcontract.VoidType, }, + { + Name: "invalidStack", + Offset: invalidStackOff, + ReturnType: smartcontract.VoidType, + }, } cs := &state.Contract{ Script: script, diff --git a/pkg/core/state/notification_event.go b/pkg/core/state/notification_event.go index 4caf4bdc2..cd2a126f2 100644 --- a/pkg/core/state/notification_event.go +++ b/pkg/core/state/notification_event.go @@ -57,7 +57,11 @@ func (aer *AppExecResult) EncodeBinary(w *io.BinWriter) { w.WriteB(byte(aer.Trigger)) w.WriteB(byte(aer.VMState)) w.WriteU64LE(uint64(aer.GasConsumed)) - stackitem.EncodeBinaryStackItem(stackitem.NewArray(aer.Stack), w) + // Stack items are expected to be marshaled one by one. + w.WriteVarUint(uint64(len(aer.Stack))) + for _, it := range aer.Stack { + stackitem.EncodeBinaryStackItemAppExec(it, w) + } w.WriteArray(aer.Events) w.WriteVarBytes([]byte(aer.FaultException)) } @@ -68,15 +72,21 @@ func (aer *AppExecResult) DecodeBinary(r *io.BinReader) { aer.Trigger = trigger.Type(r.ReadB()) aer.VMState = vm.State(r.ReadB()) aer.GasConsumed = int64(r.ReadU64LE()) - item := stackitem.DecodeBinaryStackItem(r) - if r.Err == nil { - arr, ok := item.Value().([]stackitem.Item) - if !ok { - r.Err = errors.New("array expected") + sz := r.ReadVarUint() + if stackitem.MaxArraySize < sz && r.Err == nil { + r.Err = errors.New("invalid format") + } + if r.Err != nil { + return + } + arr := make([]stackitem.Item, sz) + for i := 0; i < int(sz); i++ { + arr[i] = stackitem.DecodeBinaryStackItemAppExec(r) + if r.Err != nil { return } - aer.Stack = arr } + aer.Stack = arr r.ReadArray(&aer.Events) aer.FaultException = r.ReadString() } @@ -182,23 +192,22 @@ type executionAux struct { // MarshalJSON implements implements json.Marshaler interface. func (e Execution) MarshalJSON() ([]byte, error) { - var st json.RawMessage + var errRecursive = []byte(`"error: recursive reference"`) arr := make([]json.RawMessage, len(e.Stack)) for i := range arr { + if e.Stack[i] == nil { + arr[i] = errRecursive + continue + } data, err := stackitem.ToJSONWithTypes(e.Stack[i]) if err != nil { - st = []byte(`"error: recursive reference"`) - break + data = errRecursive } arr[i] = data } - var err error - if st == nil { - st, err = json.Marshal(arr) - if err != nil { - return nil, err - } - + st, err := json.Marshal(arr) + if err != nil { + return nil, err } return json.Marshal(&executionAux{ Trigger: e.Trigger.String(), @@ -222,7 +231,11 @@ func (e *Execution) UnmarshalJSON(data []byte) error { for i := range arr { st[i], err = stackitem.FromJSONWithTypes(arr[i]) if err != nil { - break + var s string + if json.Unmarshal(arr[i], &s) != nil { + break + } + err = nil } } if err == nil { diff --git a/pkg/core/state/notification_event_test.go b/pkg/core/state/notification_event_test.go index da74d0a31..3e216a338 100644 --- a/pkg/core/state/notification_event_test.go +++ b/pkg/core/state/notification_event_test.go @@ -6,6 +6,7 @@ import ( "github.com/nspcc-dev/neo-go/internal/random" "github.com/nspcc-dev/neo-go/internal/testserdes" + "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" @@ -23,35 +24,63 @@ func TestEncodeDecodeNotificationEvent(t *testing.T) { } func TestEncodeDecodeAppExecResult(t *testing.T) { - t.Run("halt", func(t *testing.T) { - appExecResult := &AppExecResult{ + newAer := func() *AppExecResult { + return &AppExecResult{ Container: random.Uint256(), Execution: Execution{ Trigger: 1, VMState: vm.HaltState, GasConsumed: 10, - Stack: []stackitem.Item{}, + Stack: []stackitem.Item{stackitem.NewBool(true)}, Events: []NotificationEvent{}, }, } - + } + t.Run("halt", func(t *testing.T) { + appExecResult := newAer() + appExecResult.VMState = vm.HaltState testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult)) }) t.Run("fault", func(t *testing.T) { - appExecResult := &AppExecResult{ - Container: random.Uint256(), - Execution: Execution{ - Trigger: 1, - VMState: vm.FaultState, - GasConsumed: 10, - Stack: []stackitem.Item{}, - Events: []NotificationEvent{}, - FaultException: "unhandled error", - }, - } - + appExecResult := newAer() + appExecResult.VMState = vm.FaultState testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult)) }) + t.Run("with interop", func(t *testing.T) { + appExecResult := newAer() + appExecResult.Stack = []stackitem.Item{stackitem.NewInterop(nil)} + testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult)) + }) + t.Run("recursive reference", func(t *testing.T) { + var arr = stackitem.NewArray(nil) + arr.Append(arr) + appExecResult := newAer() + appExecResult.Stack = []stackitem.Item{arr, stackitem.NewBool(true), stackitem.NewInterop(123)} + + bs, err := testserdes.EncodeBinary(appExecResult) + require.NoError(t, err) + actual := new(AppExecResult) + require.NoError(t, testserdes.DecodeBinary(bs, actual)) + require.Equal(t, 3, len(actual.Stack)) + require.Nil(t, actual.Stack[0]) + require.Equal(t, true, actual.Stack[1].Value()) + require.Equal(t, stackitem.InteropT, actual.Stack[2].Type()) + + bs1, err := testserdes.EncodeBinary(actual) + require.NoError(t, err) + require.Equal(t, bs, bs1) + }) + t.Run("invalid item type", func(t *testing.T) { + aer := newAer() + w := io.NewBufBinWriter() + w.WriteBytes(aer.Container[:]) + w.WriteB(byte(aer.Trigger)) + w.WriteB(byte(aer.VMState)) + w.WriteU64LE(uint64(aer.GasConsumed)) + stackitem.EncodeBinaryStackItem(stackitem.NewBool(true), w.BinWriter) + require.NoError(t, w.Err) + require.Error(t, testserdes.DecodeBinary(w.Bytes(), new(AppExecResult))) + }) } func TestMarshalUnmarshalJSONNotificationEvent(t *testing.T) { @@ -127,7 +156,7 @@ func TestMarshalUnmarshalJSONAppExecResult(t *testing.T) { Trigger: trigger.Application, VMState: vm.FaultState, GasConsumed: 10, - Stack: []stackitem.Item{}, + Stack: []stackitem.Item{stackitem.NewBool(true)}, Events: []NotificationEvent{}, FaultException: "unhandled exception", }, @@ -149,20 +178,28 @@ func TestMarshalUnmarshalJSONAppExecResult(t *testing.T) { }) t.Run("MarshalJSON recursive reference", func(t *testing.T) { - i := make([]stackitem.Item, 1) - recursive := stackitem.NewArray(i) - i[0] = recursive - errorCases := []*AppExecResult{ - { - Execution: Execution{ - Stack: i, - }, + arr := stackitem.NewArray(nil) + arr.Append(arr) + errAer := &AppExecResult{ + Execution: Execution{ + Trigger: trigger.Application, + Stack: []stackitem.Item{arr, stackitem.NewBool(true), stackitem.NewInterop(123)}, }, } - for _, errCase := range errorCases { - _, err := json.Marshal(errCase) - require.NoError(t, err) - } + + bs, err := json.Marshal(errAer) + require.NoError(t, err) + + actual := new(AppExecResult) + require.NoError(t, json.Unmarshal(bs, actual)) + require.Equal(t, 3, len(actual.Stack)) + require.Nil(t, actual.Stack[0]) + require.Equal(t, true, actual.Stack[1].Value()) + require.Equal(t, stackitem.InteropT, actual.Stack[2].Type()) + + bs1, err := json.Marshal(actual) + require.NoError(t, err) + require.Equal(t, bs, bs1) }) t.Run("UnmarshalJSON error", func(t *testing.T) { diff --git a/pkg/vm/stackitem/serialization.go b/pkg/vm/stackitem/serialization.go index f6b603718..3a40f4590 100644 --- a/pkg/vm/stackitem/serialization.go +++ b/pkg/vm/stackitem/serialization.go @@ -23,14 +23,30 @@ func SerializeItem(item Item) ([]byte, error) { // similar to io.Serializable's EncodeBinary, but works with Item // interface. func EncodeBinaryStackItem(item Item, w *io.BinWriter) { - serializeItemTo(item, w, make(map[Item]bool)) + serializeItemTo(item, w, false, make(map[Item]bool)) } -func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) { +// EncodeBinaryStackItemAppExec encodes given Item into the given BinWriter. It's +// similar to EncodeBinaryStackItem but allows to encode interop (only type, value is lost). +func EncodeBinaryStackItemAppExec(item Item, w *io.BinWriter) { + bw := io.NewBufBinWriter() + serializeItemTo(item, bw.BinWriter, true, make(map[Item]bool)) + if bw.Err != nil { + w.WriteBytes([]byte{byte(InvalidT)}) + return + } + w.WriteBytes(bw.Bytes()) +} + +func serializeItemTo(item Item, w *io.BinWriter, allowInvalid bool, seen map[Item]bool) { if seen[item] { w.Err = errors.New("recursive structures can't be serialized") return } + if item == nil && allowInvalid { + w.WriteBytes([]byte{byte(InvalidT)}) + return + } switch t := item.(type) { case *ByteArray: @@ -46,6 +62,10 @@ func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) { w.WriteBytes([]byte{byte(IntegerT)}) w.WriteVarBytes(bigint.ToBytes(t.Value().(*big.Int))) case *Interop: + if allowInvalid { + w.WriteBytes([]byte{byte(InteropT)}) + return + } w.Err = errors.New("interop item can't be serialized") case *Array, *Struct: seen[item] = true @@ -60,7 +80,7 @@ func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) { arr := t.Value().([]Item) w.WriteVarUint(uint64(len(arr))) for i := range arr { - serializeItemTo(arr[i], w, seen) + serializeItemTo(arr[i], w, allowInvalid, seen) } case *Map: seen[item] = true @@ -68,8 +88,8 @@ func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) { w.WriteBytes([]byte{byte(MapT)}) w.WriteVarUint(uint64(len(t.Value().([]MapElement)))) for i := range t.Value().([]MapElement) { - serializeItemTo(t.Value().([]MapElement)[i].Key, w, seen) - serializeItemTo(t.Value().([]MapElement)[i].Value, w, seen) + serializeItemTo(t.Value().([]MapElement)[i].Key, w, allowInvalid, seen) + serializeItemTo(t.Value().([]MapElement)[i].Value, w, allowInvalid, seen) } case Null: w.WriteB(byte(AnyT)) @@ -91,6 +111,16 @@ func DeserializeItem(data []byte) (Item, error) { // as a function because Item itself is an interface. Caveat: always check // reader's error value before using the returned Item. func DecodeBinaryStackItem(r *io.BinReader) Item { + return decodeBinaryStackItem(r, false) +} + +// DecodeBinaryStackItemAppExec is similar to DecodeBinaryStackItem +// but allows Interop values to be present. +func DecodeBinaryStackItemAppExec(r *io.BinReader) Item { + return decodeBinaryStackItem(r, true) +} + +func decodeBinaryStackItem(r *io.BinReader, allowInvalid bool) Item { var t = Type(r.ReadB()) if r.Err != nil { return nil @@ -132,7 +162,15 @@ func DecodeBinaryStackItem(r *io.BinReader) Item { return m case AnyT: return Null{} + case InteropT: + if allowInvalid { + return NewInterop(nil) + } + fallthrough default: + if t == InvalidT && allowInvalid { + return nil + } r.Err = fmt.Errorf("unknown type: %v", t) return nil } diff --git a/pkg/vm/stackitem/type.go b/pkg/vm/stackitem/type.go index ea69fc48b..b28df9ebe 100644 --- a/pkg/vm/stackitem/type.go +++ b/pkg/vm/stackitem/type.go @@ -17,6 +17,7 @@ const ( StructT Type = 0x41 MapT Type = 0x48 InteropT Type = 0x60 + InvalidT Type = 0xFF ) // String implements fmt.Stringer interface.