Skip to content

Commit

Permalink
cannon: Handle preimage bounds checks consistently (#11911)
Browse files Browse the repository at this point in the history 
* cannon: Handle preimage bounds checks consistently

* cannon: Cleanup stray comment
  • Loading branch information
@mbaxter
mbaxter authored Sep 13, 2024
1 parent 3056348 commit c8d6dbb
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 35 deletions.
4 changes: 2 additions & 2 deletions cannon/mipsevm/exec/preimage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ func (p *TrackingPreimageOracleReader) ReadPreimage(key [32]byte, offset uint32)
p.lastPreimage = preimage
}
p.lastPreimageOffset = offset
if offset > uint32(len(preimage)) {
return
if offset >= uint32(len(preimage)) {
panic("Preimage offset out-of-bounds")
}
datLen = uint32(copy(dat[:], preimage[offset:]))
return
Expand Down
14 changes: 7 additions & 7 deletions cannon/mipsevm/multithreaded/testutil/expectations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type ExpectedMTState struct {
Step uint64
LastHint hexutil.Bytes
MemoryRoot common.Hash
expectedMemory *memory.Memory
// Threading-related expectations
StepsSinceLastContextSwitch uint64
Wakeup uint32
Expand All @@ -34,7 +35,6 @@ type ExpectedMTState struct {
prestateActiveThreadOrig ExpectedThreadState // Cached for internal use
ActiveThreadId uint32
threadExpectations map[uint32]*ExpectedThreadState
memoryExpectations *memory.Memory
}

type ExpectedThreadState struct {
Expand Down Expand Up @@ -83,7 +83,7 @@ func NewExpectedMTState(fromState *multithreaded.State) *ExpectedMTState {
prestateActiveThreadOrig: *newExpectedThreadState(currentThread), // Cache prestate thread for internal use
ActiveThreadId: currentThread.ThreadId,
threadExpectations: expectedThreads,
memoryExpectations: fromState.Memory.Copy(),
expectedMemory: fromState.Memory.Copy(),
}
}

Expand Down Expand Up @@ -113,14 +113,14 @@ func (e *ExpectedMTState) ExpectStep() {
}

func (e *ExpectedMTState) ExpectMemoryWrite(addr uint32, val uint32) {
e.memoryExpectations.SetMemory(addr, val)
e.MemoryRoot = e.memoryExpectations.MerkleRoot()
e.expectedMemory.SetMemory(addr, val)
e.MemoryRoot = e.expectedMemory.MerkleRoot()
}

func (e *ExpectedMTState) ExpectMemoryWriteMultiple(addr uint32, val uint32, addr2 uint32, val2 uint32) {
e.memoryExpectations.SetMemory(addr, val)
e.memoryExpectations.SetMemory(addr2, val)
e.MemoryRoot = e.memoryExpectations.MerkleRoot()
e.expectedMemory.SetMemory(addr, val)
e.expectedMemory.SetMemory(addr2, val)
e.MemoryRoot = e.expectedMemory.MerkleRoot()
}

func (e *ExpectedMTState) ExpectPreemption(preState *multithreaded.State) {
Expand Down
82 changes: 82 additions & 0 deletions cannon/mipsevm/tests/evm_common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tests

import (
"bytes"
"encoding/binary"
"fmt"
"io"
"os"
Expand All @@ -12,12 +13,14 @@ import (

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/core/tracing"
"github.com/ethereum/go-ethereum/crypto"
"github.com/stretchr/testify/require"

"github.com/ethereum-optimism/optimism/cannon/mipsevm/exec"
"github.com/ethereum-optimism/optimism/cannon/mipsevm/memory"
"github.com/ethereum-optimism/optimism/cannon/mipsevm/program"
"github.com/ethereum-optimism/optimism/cannon/mipsevm/testutil"
preimage "github.com/ethereum-optimism/optimism/op-preimage"
)

func TestEVM(t *testing.T) {
Expand Down Expand Up @@ -227,6 +230,85 @@ func TestEVM_MMap(t *testing.T) {
}
}

func TestEVM_SysRead_Preimage(t *testing.T) {
var tracer *tracing.Hooks

preimageValue := make([]byte, 0, 8)
preimageValue = binary.BigEndian.AppendUint32(preimageValue, 0x12_34_56_78)
preimageValue = binary.BigEndian.AppendUint32(preimageValue, 0x98_76_54_32)

versions := GetMipsVersionTestCases(t)

cases := []struct {
name string
addr uint32
count uint32
writeLen uint32
preimageOffset uint32
prestateMem uint32
postateMem uint32
shouldPanic bool
}{
{name: "Aligned addr, write 1 byte", addr: 0x00_00_FF_00, count: 1, writeLen: 1, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0x12_FF_FF_FF},
{name: "Aligned addr, write 2 byte", addr: 0x00_00_FF_00, count: 2, writeLen: 2, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0x12_34_FF_FF},
{name: "Aligned addr, write 3 byte", addr: 0x00_00_FF_00, count: 3, writeLen: 3, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0x12_34_56_FF},
{name: "Aligned addr, write 4 byte", addr: 0x00_00_FF_00, count: 4, writeLen: 4, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0x12_34_56_78},
{name: "1-byte misaligned addr, write 1 byte", addr: 0x00_00_FF_01, count: 1, writeLen: 1, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_12_FF_FF},
{name: "1-byte misaligned addr, write 2 byte", addr: 0x00_00_FF_01, count: 2, writeLen: 2, preimageOffset: 9, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_34_56_FF},
{name: "1-byte misaligned addr, write 3 byte", addr: 0x00_00_FF_01, count: 3, writeLen: 3, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_12_34_56},
{name: "2-byte misaligned addr, write 1 byte", addr: 0x00_00_FF_02, count: 1, writeLen: 1, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_FF_12_FF},
{name: "2-byte misaligned addr, write 2 byte", addr: 0x00_00_FF_02, count: 2, writeLen: 2, preimageOffset: 12, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_FF_98_76},
{name: "3-byte misaligned addr, write 1 byte", addr: 0x00_00_FF_03, count: 1, writeLen: 1, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_FF_FF_12},
{name: "Count of 0", addr: 0x00_00_FF_03, count: 0, writeLen: 0, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_FF_FF_FF},
{name: "Count greater than 4", addr: 0x00_00_FF_00, count: 15, writeLen: 4, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0x12_34_56_78},
{name: "Count greater than 4, unaligned", addr: 0x00_00_FF_01, count: 15, writeLen: 3, preimageOffset: 8, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_12_34_56},
{name: "Offset at last byte", addr: 0x00_00_FF_00, count: 4, writeLen: 1, preimageOffset: 15, prestateMem: 0xFF_FF_FF_FF, postateMem: 0x32_FF_FF_FF},
{name: "Offset just out of bounds", addr: 0x00_00_FF_00, count: 4, writeLen: 0, preimageOffset: 16, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_FF_FF_FF, shouldPanic: true},
{name: "Offset out of bounds", addr: 0x00_00_FF_00, count: 4, writeLen: 0, preimageOffset: 17, prestateMem: 0xFF_FF_FF_FF, postateMem: 0xFF_FF_FF_FF, shouldPanic: true},
}
for i, c := range cases {
for _, v := range versions {
tName := fmt.Sprintf("%v (%v)", c.name, v.Name)
t.Run(tName, func(t *testing.T) {
effAddr := 0xFFffFFfc & c.addr
preimageKey := preimage.Keccak256Key(crypto.Keccak256Hash(preimageValue)).PreimageKey()
oracle := testutil.StaticOracle(t, preimageValue)
goVm := v.VMFactory(oracle, os.Stdout, os.Stderr, testutil.CreateLogger(), testutil.WithRandomization(int64(i)), testutil.WithPreimageKey(preimageKey), testutil.WithPreimageOffset(c.preimageOffset))
state := goVm.GetState()
step := state.GetStep()

// Set up state
state.GetRegistersRef()[2] = exec.SysRead
state.GetRegistersRef()[4] = exec.FdPreimageRead
state.GetRegistersRef()[5] = c.addr
state.GetRegistersRef()[6] = c.count
state.GetMemory().SetMemory(state.GetPC(), syscallInsn)
state.GetMemory().SetMemory(effAddr, c.prestateMem)

// Setup expectations
expected := testutil.NewExpectedState(state)
expected.ExpectStep()
expected.Registers[2] = c.writeLen
expected.Registers[7] = 0 // no error
expected.PreimageOffset += c.writeLen
expected.ExpectMemoryWrite(effAddr, c.postateMem)

if c.shouldPanic {
require.Panics(t, func() { _, _ = goVm.Step(true) })
testutil.AssertPreimageOracleReverts(t, preimageKey, preimageValue, c.preimageOffset, v.Contracts, tracer)
} else {
stepWitness, err := goVm.Step(true)
require.NoError(t, err)

// Check expectations
expected.Validate(t, state)
testutil.ValidateEVM(t, stepWitness, step, goVm, v.StateHashFn, v.Contracts, tracer)
}
})
}
}
}

func TestEVMSysWriteHint(t *testing.T) {
var tracer *tracing.Hooks

Expand Down
71 changes: 47 additions & 24 deletions cannon/mipsevm/testutil/mips.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,24 @@ import (
)

type MIPSEVM struct {
sender vm.AccountRef
startingGas uint64
env *vm.EVM
evmState *state.StateDB
addrs *Addresses
localOracle mipsevm.PreimageOracle
artifacts *Artifacts
// Track step execution for logging purposes
lastStep uint64
lastStepInput []byte
lastStep uint64
lastStepInput []byte
lastPreimageOracleInput []byte
}

func NewMIPSEVM(contracts *ContractMetadata) *MIPSEVM {
env, evmState := NewEVMEnv(contracts)
return &MIPSEVM{env, evmState, contracts.Addresses, nil, contracts.Artifacts, math.MaxUint64, nil}
sender := vm.AccountRef{0x13, 0x37}
startingGas := uint64(30_000_000)
return &MIPSEVM{sender, startingGas, env, evmState, contracts.Addresses, nil, contracts.Artifacts, math.MaxUint64, nil, nil}
}

func (m *MIPSEVM) SetTracer(tracer *tracing.Hooks) {
Expand All @@ -52,23 +57,23 @@ func (m *MIPSEVM) SetSourceMapTracer(t *testing.T, version MipsVersion) {
func (m *MIPSEVM) Step(t *testing.T, stepWitness *mipsevm.StepWitness, step uint64, stateHashFn mipsevm.HashFn) []byte {
m.lastStep = step
m.lastStepInput = nil
sender := common.Address{0x13, 0x37}
startingGas := uint64(30_000_000)
m.lastPreimageOracleInput = nil

// we take a snapshot so we can clean up the state, and isolate the logs of this instruction run.
snap := m.env.StateDB.Snapshot()

if stepWitness.HasPreimage() {
t.Logf("reading preimage key %x at offset %d", stepWitness.PreimageKey, stepWitness.PreimageOffset)
poInput, err := EncodePreimageOracleInput(t, stepWitness, mipsevm.LocalContext{}, m.localOracle, m.artifacts.Oracle)
poInput, err := m.encodePreimageOracleInput(t, stepWitness.PreimageKey, stepWitness.PreimageValue, stepWitness.PreimageOffset, mipsevm.LocalContext{})
m.lastPreimageOracleInput = poInput
require.NoError(t, err, "encode preimage oracle input")
_, leftOverGas, err := m.env.Call(vm.AccountRef(sender), m.addrs.Oracle, poInput, startingGas, common.U2560)
require.NoErrorf(t, err, "evm should not fail, took %d gas", startingGas-leftOverGas)
_, leftOverGas, err := m.env.Call(m.sender, m.addrs.Oracle, poInput, m.startingGas, common.U2560)
require.NoErrorf(t, err, "evm should not fail, took %d gas", m.startingGas-leftOverGas)
}

input := EncodeStepInput(t, stepWitness, mipsevm.LocalContext{}, m.artifacts.MIPS)
m.lastStepInput = input
ret, leftOverGas, err := m.env.Call(vm.AccountRef(sender), m.addrs.MIPS, input, startingGas, common.U2560)
ret, leftOverGas, err := m.env.Call(m.sender, m.addrs.MIPS, input, m.startingGas, common.U2560)
require.NoError(t, err, "evm should not fail")
require.Len(t, ret, 32, "expecting 32-byte state hash")
// remember state hash, to check it against state
Expand All @@ -82,7 +87,7 @@ func (m *MIPSEVM) Step(t *testing.T, stepWitness *mipsevm.StepWitness, step uint
require.Equal(t, stateHash, postHash, "logged state must be accurate")

m.env.StateDB.RevertToSnapshot(snap)
t.Logf("EVM step %d took %d gas, and returned stateHash %s", step, startingGas-leftOverGas, postHash)
t.Logf("EVM step %d took %d gas, and returned stateHash %s", step, m.startingGas-leftOverGas, postHash)
return evmPost
}

Expand All @@ -92,46 +97,48 @@ func EncodeStepInput(t *testing.T, wit *mipsevm.StepWitness, localContext mipsev
return input
}

func EncodePreimageOracleInput(t *testing.T, wit *mipsevm.StepWitness, localContext mipsevm.LocalContext, localOracle mipsevm.PreimageOracle, oracle *foundry.Artifact) ([]byte, error) {
if wit.PreimageKey == ([32]byte{}) {
func (m *MIPSEVM) encodePreimageOracleInput(t *testing.T, preimageKey [32]byte, preimageValue []byte, preimageOffset uint32, localContext mipsevm.LocalContext) ([]byte, error) {
if preimageKey == ([32]byte{}) {
return nil, errors.New("cannot encode pre-image oracle input, witness has no pre-image to proof")
}
localOracle := m.localOracle
oracle := m.artifacts.Oracle

switch preimage.KeyType(wit.PreimageKey[0]) {
switch preimage.KeyType(preimageKey[0]) {
case preimage.LocalKeyType:
if len(wit.PreimageValue) > 32+8 {
return nil, fmt.Errorf("local pre-image exceeds maximum size of 32 bytes with key 0x%x", wit.PreimageKey)
if len(preimageValue) > 32+8 {
return nil, fmt.Errorf("local pre-image exceeds maximum size of 32 bytes with key 0x%x", preimageKey)
}
preimagePart := wit.PreimageValue[8:]
preimagePart := preimageValue[8:]
var tmp [32]byte
copy(tmp[:], preimagePart)
input, err := oracle.ABI.Pack("loadLocalData",
new(big.Int).SetBytes(wit.PreimageKey[1:]),
new(big.Int).SetBytes(preimageKey[1:]),
localContext,
tmp,
new(big.Int).SetUint64(uint64(len(preimagePart))),
new(big.Int).SetUint64(uint64(wit.PreimageOffset)),
new(big.Int).SetUint64(uint64(preimageOffset)),
)
require.NoError(t, err)
return input, nil
case preimage.Keccak256KeyType:
input, err := oracle.ABI.Pack(
"loadKeccak256PreimagePart",
new(big.Int).SetUint64(uint64(wit.PreimageOffset)),
wit.PreimageValue[8:])
new(big.Int).SetUint64(uint64(preimageOffset)),
preimageValue[8:])
require.NoError(t, err)
return input, nil
case preimage.PrecompileKeyType:
if localOracle == nil {
return nil, errors.New("local oracle is required for precompile preimages")
}
preimage := localOracle.GetPreimage(preimage.Keccak256Key(wit.PreimageKey).PreimageKey())
preimage := localOracle.GetPreimage(preimage.Keccak256Key(preimageKey).PreimageKey())
precompile := common.BytesToAddress(preimage[:20])
requiredGas := binary.BigEndian.Uint64(preimage[20:28])
callInput := preimage[28:]
input, err := oracle.ABI.Pack(
"loadPrecompilePreimagePart",
new(big.Int).SetUint64(uint64(wit.PreimageOffset)),
new(big.Int).SetUint64(uint64(preimageOffset)),
precompile,
requiredGas,
callInput,
Expand All @@ -140,15 +147,23 @@ func EncodePreimageOracleInput(t *testing.T, wit *mipsevm.StepWitness, localCont
return input, nil
default:
return nil, fmt.Errorf("unsupported pre-image type %d, cannot prepare preimage with key %x offset %d for oracle",
wit.PreimageKey[0], wit.PreimageKey, wit.PreimageOffset)
preimageKey[0], preimageKey, preimageOffset)
}
}

func (m *MIPSEVM) assertPreimageOracleReverts(t *testing.T, preimageKey [32]byte, preimageValue []byte, preimageOffset uint32) {
poInput, err := m.encodePreimageOracleInput(t, preimageKey, preimageValue, preimageOffset, mipsevm.LocalContext{})
require.NoError(t, err, "encode preimage oracle input")
_, _, evmErr := m.env.Call(m.sender, m.addrs.Oracle, poInput, m.startingGas, common.U2560)

require.ErrorContains(t, evmErr, "execution reverted")
}

func LogStepFailureAtCleanup(t *testing.T, mipsEvm *MIPSEVM) {
t.Cleanup(func() {
if t.Failed() {
// Note: For easier debugging of a failing step, see MIPS.t.sol#test_step_debug_succeeds()
t.Logf("Failed while executing step %d with input: %x", mipsEvm.lastStep, mipsEvm.lastStepInput)
t.Logf("Failed while executing step %d with\n\tstep input: %x\n\tpreimageOracle input: %x", mipsEvm.lastStep, mipsEvm.lastStepInput, mipsEvm.lastPreimageOracleInput)
}
})
}
Expand Down Expand Up @@ -184,3 +199,11 @@ func AssertEVMReverts(t *testing.T, state mipsevm.FPVMState, contracts *Contract
logs := evmState.Logs()
require.Equal(t, 0, len(logs))
}

func AssertPreimageOracleReverts(t *testing.T, preimageKey [32]byte, preimageValue []byte, preimageOffset uint32, contracts *ContractMetadata, tracer *tracing.Hooks) {
evm := NewMIPSEVM(contracts)
evm.SetTracer(tracer)
LogStepFailureAtCleanup(t, evm)

evm.assertPreimageOracleReverts(t, preimageKey, preimageValue, preimageOffset)
}
15 changes: 15 additions & 0 deletions cannon/mipsevm/testutil/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/ethereum-optimism/optimism/cannon/mipsevm"
"github.com/ethereum-optimism/optimism/cannon/mipsevm/memory"
)

func CopyRegisters(state mipsevm.FPVMState) *[32]uint32 {
Expand Down Expand Up @@ -115,6 +116,7 @@ type ExpectedState struct {
LastHint hexutil.Bytes
Registers [32]uint32
MemoryRoot common.Hash
expectedMemory *memory.Memory
}

func NewExpectedState(fromState mipsevm.FPVMState) *ExpectedState {
Expand All @@ -132,9 +134,22 @@ func NewExpectedState(fromState mipsevm.FPVMState) *ExpectedState {
LastHint: fromState.GetLastHint(),
Registers: *fromState.GetRegistersRef(),
MemoryRoot: fromState.GetMemory().MerkleRoot(),
expectedMemory: fromState.GetMemory().Copy(),
}
}

func (e *ExpectedState) ExpectStep() {
// Set some standard expectations for a normal step
e.Step += 1
e.PC += 4
e.NextPC += 4
}

func (e *ExpectedState) ExpectMemoryWrite(addr uint32, val uint32) {
e.expectedMemory.SetMemory(addr, val)
e.MemoryRoot = e.expectedMemory.MerkleRoot()
}

type StateValidationFlags int

// TODO(cp-983) - Remove these validation hacks
Expand Down
Loading

0 comments on commit c8d6dbb

Please sign in to comment.