From aa5b9de9d0b535d58958cad7d308a968afdee18c Mon Sep 17 00:00:00 2001 From: ascandone Date: Wed, 21 Oct 2026 16:57:35 +0200 Subject: [PATCH] fix(vm): reject a pull whose destination aliases its cap or overdraft funds.Pull writes the destination before it reads the bounds, so an aliased cap no longer bounds the pull and an aliased overdraft doubles the balance. The verifier now rejects that shape. --- bytecode-verifier.md | 1 + instruction-encoding.md | 2 +- internal/vm/fuzz_test.go | 1 + internal/vm/instruction.go | 1 + internal/vm/ir_test.go | 32 ++++++++++++++++++++++++++++++++ internal/vm/verify.go | 4 ++++ internal/vm/verify_test.go | 22 ++++++++++++++++++++++ 7 files changed, 62 insertions(+), 1 deletion(-) diff --git a/bytecode-verifier.md b/bytecode-verifier.md index facc9760..8bfbdd2f 100644 --- a/bytecode-verifier.md +++ b/bytecode-verifier.md @@ -37,6 +37,7 @@ that passes vars should use the second form. | Flag operands are 0 or 1 | `Op_MarkEnd` tests `A == 1`, so a 2 silently commits a region that meant to rewind | | Definite assignment | Reading a register not written on every path reaching the instruction | | The current asset is set before it is used | A `send` or `pull` before any `set_current_asset` | +| A pull's destination does not alias its cap or overdraft | `funds.Pull` writes the destination before it reads the bounds on the bounded path: aliased, the cap no longer bounds the pull and the overdraft doubles the balance | | Mark discipline (see below) | An `Op_MarkEnd` with no open mark, a send/save/asset change inside a region, a run ending with a region open | | (`VerifyWithVars`) Var-pool indices are in range | `Op_LoadVar*` indexing past the pool, or dereferencing a nil `*Vars` | diff --git a/instruction-encoding.md b/instruction-encoding.md index 3763a212..bd026985 100644 --- a/instruction-encoding.md +++ b/instruction-encoding.md @@ -355,7 +355,7 @@ Instructions are **4 bytes** wide: `[Opcode: 8] [A: 8] [B: 8] [C: 8]`. ​​↳ cont. overdraftcolorscope - Overdraft bound int_regs[A] (0xFF = unbounded), color str_regs[B] and scope str_regs[C] (0xFF = none). Cap and overdraft both 0xFF traps (unbounded source) + Overdraft bound int_regs[A] (0xFF = unbounded), color str_regs[B] and scope str_regs[C] (0xFF = none). Cap and overdraft both 0xFF traps (unbounded source). The destination register may not be the cap or the overdraft register (verifier-rejected) 810x51SEND_TO_ACCOUNT diff --git a/internal/vm/fuzz_test.go b/internal/vm/fuzz_test.go index 647b894c..2fd6c002 100644 --- a/internal/vm/fuzz_test.go +++ b/internal/vm/fuzz_test.go @@ -23,6 +23,7 @@ func FuzzExec(f *testing.F) { f.Add([]byte{byte(Op_LoadStr), 0, 0, 0, byte(Op_Jmp), 0, 1, 0}) // jump past the end f.Add([]byte{byte(Op_ConstTrue), 0, 0, 0, byte(Op_JmpIfTrue), 0, 0, 0}) f.Add([]byte{byte(Op_MarkPush), 0, 0, 0, byte(Op_MarkEnd), 1, 0, 0}) + f.Add([]byte{byte(Op_PullAccount), 1, 0, 1, 0xFF, nilReg, nilReg, nilReg}) // dest aliases cap stringsPool := []string{"world", "dest", "USD/2"} intsPool := []big.Int{*big.NewInt(0), *big.NewInt(7)} diff --git a/internal/vm/instruction.go b/internal/vm/instruction.go index c46f280c..5c4c0a52 100644 --- a/internal/vm/instruction.go +++ b/internal/vm/instruction.go @@ -168,6 +168,7 @@ const ( // The most general form: account,cap,overdraft,color,scope // The 0xFF special register means NULL for cap,overdraft,color and scope // ext.A = overdraft (int reg), ext.B = color (str reg), ext.C = scope (str reg) + // A (dest) may not be the same register as C or ext.A: verifier-rejected Op_PullAccount Opcode = 0x50 // account?, cap?, scope? diff --git a/internal/vm/ir_test.go b/internal/vm/ir_test.go index 80c36a5f..49214979 100644 --- a/internal/vm/ir_test.go +++ b/internal/vm/ir_test.go @@ -1369,3 +1369,35 @@ func TestIRErrorsDoNotAliasRegisters(t *testing.T) { require.Equal(t, vm.NegativeAmountError{Amount: *big.NewInt(-20)}, second) }) } + +// A pull whose destination is also its cap or overdraft reads the bound after +// having overwritten it. +func TestIRPullDestAliasingItsBoundIsRejected(t *testing.T) { + t.Run("cap", func(t *testing.T) { + program := assembleUnverifiedIR(t, ` + $asset = "USD/2" + set_current_asset($asset) + $src = "src" + $dest = "dest" + $overdraft = 0 + $cap = 100 + $cap = pull_account(account: $src, cap: $cap, overdraft: $overdraft) + send_to_account(account: $dest) +`) + require.ErrorContains(t, vm.Verify(program), "aliases its cap or overdraft") + }) + + t.Run("overdraft", func(t *testing.T) { + program := assembleUnverifiedIR(t, ` + $asset = "USD/2" + set_current_asset($asset) + $src = "src" + $dest = "dest" + $overdraft = 0 + $cap = 100 + $overdraft = pull_account(account: $src, cap: $cap, overdraft: $overdraft) + send_to_account(account: $dest) +`) + require.ErrorContains(t, vm.Verify(program), "aliases its cap or overdraft") + }) +} diff --git a/internal/vm/verify.go b/internal/vm/verify.go index 11b4bdcb..94930cbb 100644 --- a/internal/vm/verify.go +++ b/internal/vm/verify.go @@ -580,6 +580,10 @@ func decodeInstr(instr, ext Instruction) (decoded, error) { // --- funds & postings case Op_PullAccount: + // funds.Pull writes the destination before it reads the bounds + if instr.A == instr.C || instr.A == ext.A { + err = fmt.Errorf("pull destination int register %d aliases its cap or overdraft", instr.A) + } read(bankStr, instr.B) // account readOpt(bankInt, instr.C) // cap readOpt(bankInt, ext.A) // overdraft diff --git a/internal/vm/verify_test.go b/internal/vm/verify_test.go index b5f77d37..dd2cd42d 100644 --- a/internal/vm/verify_test.go +++ b/internal/vm/verify_test.go @@ -258,3 +258,25 @@ func TestVerifyWithVarsAllowsNilWhenNoneAreRead(t *testing.T) { func TestVerifyEmptyProgram(t *testing.T) { mustAccept(t, Program{}) } + +func TestVerifyPullDestAliasesCapOrOverdraft(t *testing.T) { + pull := func(dest, cap, overdraft byte) Program { + return Program{ + Instructions: []Instruction{ + bc(Op_LoadStr, 0, 0), + bc(Op_LoadInt, 0, 0), + bc(Op_LoadInt, 1, 0), + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, dest, 0, cap), + abc(0, overdraft, nilReg, nilReg), // ext + }, + StringsPool: []string{"acc"}, + IntsPool: []big.Int{*big.NewInt(1)}, + } + } + requireRejectedWith(t, pull(0, 0, 1), "aliases its cap or overdraft") + requireRejectedWith(t, pull(1, 0, 1), "aliases its cap or overdraft") + mustAccept(t, pull(2, 0, 1)) + // the two bounds are read-only, so they may share a register + mustAccept(t, pull(2, 0, 0)) +}