From 7cd69ea8e2a929873222f1c07bd7a535f8eb5cf7 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 18 Sep 2019 14:19:58 +0300 Subject: [PATCH 1/3] vm: truncate length in SUBSTR Also fail on first error, without changing the stack. --- pkg/vm/vm.go | 14 +++++++++----- pkg/vm/vm_test.go | 12 ++++++++---- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 3592291a9..a523642d5 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -318,18 +318,22 @@ func (v *VM) execute(ctx *Context, op Instruction) { v.estack.PushVal(ab) case SUBSTR: l := int(v.estack.Pop().BigInt().Int64()) - o := int(v.estack.Pop().BigInt().Int64()) - s := v.estack.Pop().Bytes() if l < 0 { panic("negative length") } + o := int(v.estack.Pop().BigInt().Int64()) if o < 0 { panic("negative index") } - if l+o > len(s) { - panic("out of bounds access") + s := v.estack.Pop().Bytes() + if o > len(s) { + panic("invalid offset") } - v.estack.PushVal(s[o : o+l]) + last := l + o + if last > len(s) { + last = len(s) + } + v.estack.PushVal(s[o:last]) case LEFT: l := int(v.estack.Pop().BigInt().Int64()) if l < 0 { diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index 9d8e1ca0f..3856d9d2a 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -1138,20 +1138,22 @@ func TestSUBSTRBadOffset(t *testing.T) { prog := makeProgram(SUBSTR) vm := load(prog) vm.estack.PushVal([]byte("abcdef")) - vm.estack.PushVal(6) + vm.estack.PushVal(7) vm.estack.PushVal(1) vm.Run() assert.Equal(t, true, vm.state.HasFlag(faultState)) } -func TestSUBSTRBadLen(t *testing.T) { +func TestSUBSTRBigLen(t *testing.T) { prog := makeProgram(SUBSTR) vm := load(prog) vm.estack.PushVal([]byte("abcdef")) vm.estack.PushVal(1) vm.estack.PushVal(6) vm.Run() - assert.Equal(t, true, vm.state.HasFlag(faultState)) + assert.Equal(t, false, vm.state.HasFlag(faultState)) + assert.Equal(t, 1, vm.estack.Len()) + assert.Equal(t, []byte("bcdef"), vm.estack.Pop().Bytes()) } func TestSUBSTRBad387(t *testing.T) { @@ -1163,7 +1165,9 @@ func TestSUBSTRBad387(t *testing.T) { vm.estack.PushVal(1) vm.estack.PushVal(6) vm.Run() - assert.Equal(t, true, vm.state.HasFlag(faultState)) + assert.Equal(t, false, vm.state.HasFlag(faultState)) + assert.Equal(t, 1, vm.estack.Len()) + assert.Equal(t, []byte("bcdef"), vm.estack.Pop().Bytes()) } func TestSUBSTRBadNegativeOffset(t *testing.T) { From 4a8be486f01f1990d0357ddaadb2cbe09a85c2b3 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 18 Sep 2019 14:24:42 +0300 Subject: [PATCH 2/3] vm: do not pop items in OVER --- pkg/vm/vm.go | 7 +------ pkg/vm/vm_test.go | 2 ++ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index a523642d5..2bed4fb6c 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -408,15 +408,10 @@ func (v *VM) execute(ctx *Context, op Instruction) { } case OVER: - b := v.estack.Pop() - if b == nil { - panic("no top-level element found") - } - a := v.estack.Peek(0) + a := v.estack.Peek(1) if a == nil { panic("no second element found") } - v.estack.Push(b) v.estack.Push(a) case PICK: diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index 3856d9d2a..ac9dbe89e 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -920,6 +920,8 @@ func TestOVERbadNoitem(t *testing.T) { vm.estack.PushVal(1) vm.Run() assert.Equal(t, true, vm.state.HasFlag(faultState)) + assert.Equal(t, 1, vm.estack.Len()) + assert.Equal(t, makeStackItem(1), vm.estack.Pop().value) } func TestOVERbadNoitems(t *testing.T) { From 3c53beca82cf9675148613627e9c69cf5b787158 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 18 Sep 2019 14:35:29 +0300 Subject: [PATCH 3/3] vm: restrict SHL/SHR arguments to -256..256 Also unify SHL/SHR implementation. --- pkg/vm/vm.go | 25 ++++++++++++++----------- pkg/vm/vm_test.go | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 2bed4fb6c..0108b6a4e 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -22,6 +22,11 @@ var ( ModeMute Mode = 1 << 0 ) +const ( + maxSHLArg = 256 + minSHLArg = -256 +) + // VM represents the virtual machine. type VM struct { state State @@ -508,21 +513,19 @@ func (v *VM) execute(ctx *Context, op Instruction) { a := v.estack.Pop().BigInt() v.estack.PushVal(new(big.Int).Mod(a, b)) - case SHL: - b := v.estack.Pop().BigInt() - if b.Int64() == 0 { + case SHL, SHR: + b := v.estack.Pop().BigInt().Int64() + if b == 0 { return + } else if b < minSHLArg || b > maxSHLArg { + panic(fmt.Sprintf("operand must be between %d and %d", minSHLArg, maxSHLArg)) } a := v.estack.Pop().BigInt() - v.estack.PushVal(new(big.Int).Lsh(a, uint(b.Int64()))) - - case SHR: - b := v.estack.Pop().BigInt() - if b.Int64() == 0 { - return + if op == SHL { + v.estack.PushVal(new(big.Int).Lsh(a, uint(b))) + } else { + v.estack.PushVal(new(big.Int).Rsh(a, uint(b))) } - a := v.estack.Pop().BigInt() - v.estack.PushVal(new(big.Int).Rsh(a, uint(b.Int64()))) case BOOLAND: b := v.estack.Pop().Bool() diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index ac9dbe89e..80cceba9b 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -305,6 +305,15 @@ func TestSHRZero(t *testing.T) { assert.Equal(t, makeStackItem([]byte{0, 1}), vm.estack.Pop().value) } +func TestSHRSmallValue(t *testing.T) { + prog := makeProgram(SHR) + vm := load(prog) + vm.estack.PushVal(5) + vm.estack.PushVal(-257) + vm.Run() + assert.Equal(t, true, vm.state.HasFlag(faultState)) +} + func TestSHLGood(t *testing.T) { prog := makeProgram(SHL) vm := load(prog) @@ -327,6 +336,15 @@ func TestSHLZero(t *testing.T) { assert.Equal(t, makeStackItem([]byte{0, 1}), vm.estack.Pop().value) } +func TestSHLBigValue(t *testing.T) { + prog := makeProgram(SHL) + vm := load(prog) + vm.estack.PushVal(5) + vm.estack.PushVal(257) + vm.Run() + assert.Equal(t, true, vm.state.HasFlag(faultState)) +} + func TestLT(t *testing.T) { prog := makeProgram(LT) vm := load(prog)