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)) +}