Skip to content

fix(vm): guard VM stack against underflow #860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(vm): guard VM stack against underflow #860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions vm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,10 +589,16 @@ func (vm *VM) push(value any) {
}

func (vm *VM) current() any {
if len(vm.Stack) == 0 {
panic("stack underflow")
}
return vm.Stack[len(vm.Stack)-1]
Copy link
Member

@antonmedv antonmedv Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this will also panic (with differnt message obviosly)

Copy link
Contributor Author

@thevilledev thevilledev Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's then easier to assert to that consistent error msg (if wanted). Maybe "robust" would have been a better choice than "safer".

Also just realised that this might become handy if the project fuzzer would do bytecode fuzzing as well. Distinguishing stack underflow from other errors.

Copy link
Contributor Author

@thevilledev thevilledev Nov 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote a small bytecode fuzzer and found other similar control flow edge cases. While the compiler never emits negative offsets for jump opcodes like OpJump and OpJumpIfTrue, the VM will happily take those in if defined as program bytecode. I was able to generate an infinite loop quite easily.

Example:

OpInt        // arg:  ...
OpInt        // arg:  ...
OpJump       // arg: -N (negative offset)
OpInt
OpJump

Fix is simple:

func (vm *VM) Run(program *Program, env any) (_ any, err error) {
    ...
	for vm.ip < len(program.Bytecode) {
		case OpJump:
			if arg < 0 {
				panic("negative jump offset is invalid")
			}
			vm.ip += arg
            ...

I can certainly open PRs to fix these, if we find this type of VM hardening feasible.

Copy link
Member

@antonmedv antonmedv Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m thinking of what benefits other than more readable error messages those checks can bring

}

func (vm *VM) pop() any {
if len(vm.Stack) == 0 {
panic("stack underflow")
}
value := vm.Stack[len(vm.Stack)-1]
vm.Stack = vm.Stack[:len(vm.Stack)-1]
return value
Expand Down
50 changes: 50 additions & 0 deletions vm/vm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1331,3 +1331,53 @@ func TestVM_Limits(t *testing.T) {
})
}
}

func TestVM_StackUnderflow(t *testing.T) {
tests := []struct {
name string
bytecode []vm.Opcode
args []int
expectError string
}{
{
name: "pop after push",
bytecode: []vm.Opcode{vm.OpInt, vm.OpPop},
args: []int{42, 0},
},
{
name: "underflow after valid operations",
bytecode: []vm.Opcode{vm.OpInt, vm.OpInt, vm.OpPop, vm.OpPop, vm.OpPop},
args: []int{1, 2, 0, 0, 0},
expectError: "stack underflow",
},
{
name: "pop on empty stack",
bytecode: []vm.Opcode{vm.OpPop},
args: []int{0},
expectError: "stack underflow",
},
{
name: "pop after push",
bytecode: []vm.Opcode{vm.OpInt, vm.OpPop},
args: []int{123, 0},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
program := &vm.Program{
Bytecode: tt.bytecode,
Arguments: tt.args,
Constants: []any{},
}

_, err := vm.Run(program, nil)
if tt.expectError != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tt.expectError)
} else {
require.NoError(t, err)
}
})
}
}
Loading