diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index 3aa5bed9d530..9bce23999f4d 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -6,41 +6,60 @@ import ( "github.com/holiman/uint256" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/libevm" "github.com/ethereum/go-ethereum/params" ) // evmCallArgs mirrors the parameters of the [EVM] methods Call(), CallCode(), // DelegateCall() and StaticCall(). Its fields are identical to those of the -// parameters, prepended with the receiver name. As {Delegate,Static}Call don't -// accept a value, they MUST set the respective field to nil. +// parameters, prepended with the receiver name and appended with additional +// values. As {Delegate,Static}Call don't accept a value, they MUST set the +// respective field to nil. // // Instantiation can be achieved by merely copying the parameter names, in // order, which is trivially achieved with AST manipulation: // // func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas uint64, value *uint256.Int) ... { -// ... -// args := &evmCallArgs{evm, caller, addr, input, gas, value} +// ... +// args := &evmCallArgs{evm, caller, addr, input, gas, value, false} type evmCallArgs struct { - evm *EVM + evm *EVM + // args:start caller ContractRef addr common.Address input []byte gas uint64 value *uint256.Int + // args:end + + // evm.interpreter.readOnly is only set to true via a call to + // EVMInterpreter.Run() so, if a precompile is called directly with + // StaticCall(), then readOnly might not be set yet. StaticCall() MUST set + // this to forceReadOnly and all other methods MUST set it to + // inheritReadOnly; i.e. equivalent to the boolean they each pass to + // EVMInterpreter.Run(). + readWrite rwInheritance } +type rwInheritance uint8 + +const ( + inheritReadOnly rwInheritance = iota + 1 + forceReadOnly +) + // run runs the [PrecompiledContract], differentiating between stateful and // regular types. func (args *evmCallArgs) run(p PrecompiledContract, input []byte) (ret []byte, err error) { if p, ok := p.(statefulPrecompile); ok { - return p.run(args.evm.StateDB, &args.evm.chainRules, args.caller.Address(), args.addr, input) + return p.run(args, input) } return p.Run(input) } // PrecompiledStatefulRun is the stateful equivalent of the Run() method of a // [PrecompiledContract]. -type PrecompiledStatefulRun func(_ StateDB, _ *params.Rules, caller, self common.Address, input []byte) ([]byte, error) +type PrecompiledStatefulRun func(env PrecompileEnvironment, input []byte) ([]byte, error) // NewStatefulPrecompile constructs a new PrecompiledContract that can be used // via an [EVM] instance but MUST NOT be called directly; a direct call to Run() @@ -69,6 +88,69 @@ func (p statefulPrecompile) Run([]byte) ([]byte, error) { panic(fmt.Sprintf("BUG: call to %T.Run(); MUST call %T", p, p.run)) } +// A PrecompileEnvironment provides information about the context in which a +// precompiled contract is being run. +type PrecompileEnvironment interface { + Rules() params.Rules + ReadOnly() bool + // StateDB will be non-nil i.f.f !ReadOnly(). + StateDB() StateDB + // ReadOnlyState will always be non-nil. + ReadOnlyState() libevm.StateReader + Addresses() *libevm.AddressContext +} + +// +// ****** SECURITY ****** +// +// If you are updating PrecompileEnvironment to provide the ability to call back +// into another contract, you MUST revisit the evmCallArgs.forceReadOnly flag. +// +// It is possible that forceReadOnly is true but evm.interpreter.readOnly is +// false. This is safe for now, but may not be if recursive calling *from* a +// precompile is enabled. +// +// ****** SECURITY ****** + +var _ PrecompileEnvironment = (*evmCallArgs)(nil) + +func (args *evmCallArgs) Rules() params.Rules { return args.evm.chainRules } + +func (args *evmCallArgs) ReadOnly() bool { + if args.readWrite == inheritReadOnly { + if args.evm.interpreter.readOnly { //nolint:gosimple // Clearer code coverage for difficult-to-test branch + return true + } + return false + } + // Even though args.readWrite may be some value other than forceReadOnly, + // that would be an invalid use of the API so we default to read-only as the + // safest failure mode. + return true +} + +func (args *evmCallArgs) StateDB() StateDB { + if args.ReadOnly() { + return nil + } + return args.evm.StateDB +} + +func (args *evmCallArgs) ReadOnlyState() libevm.StateReader { + // Even though we're actually returning a full state database, the user + // would have to actively circumvent the returned interface to use it. At + // that point they're off-piste and it's not our problem. + return args.evm.StateDB +} + +func (args *evmCallArgs) Addresses() *libevm.AddressContext { + return &libevm.AddressContext{ + Origin: args.evm.TxContext.Origin, + Caller: args.caller.Address(), + Self: args.addr, + } +} + var ( // These lock in the assumptions made when implementing [evmCallArgs]. If // these break then the struct fields SHOULD be changed to match these diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go index 3b2c98a887ff..29358900a9ca 100644 --- a/core/vm/contracts.libevm_test.go +++ b/core/vm/contracts.libevm_test.go @@ -2,6 +2,7 @@ package vm_test import ( "fmt" + "math/big" "testing" "github.com/holiman/uint256" @@ -86,18 +87,25 @@ func TestNewStatefulPrecompile(t *testing.T) { const gasLimit = 1e6 gasCost := rng.Uint64n(gasLimit) - makeOutput := func(caller, self common.Address, input []byte, stateVal common.Hash) []byte { + makeOutput := func(caller, self common.Address, input []byte, stateVal common.Hash, readOnly bool) []byte { return []byte(fmt.Sprintf( - "Caller: %v Precompile: %v State: %v Input: %#x", - caller, self, stateVal, input, + "Caller: %v Precompile: %v State: %v Read-only: %t, Input: %#x", + caller, self, stateVal, readOnly, input, )) } + run := func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) { + if got, want := env.StateDB() != nil, !env.ReadOnly(); got != want { + return nil, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want) + } + + addrs := env.Addresses() + val := env.ReadOnlyState().GetState(precompile, slot) + return makeOutput(addrs.Caller, addrs.Self, input, val, env.ReadOnly()), nil + } hooks := &hookstest.Stub{ PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{ precompile: vm.NewStatefulPrecompile( - func(state vm.StateDB, _ *params.Rules, caller, self common.Address, input []byte) ([]byte, error) { - return makeOutput(caller, self, input, state.GetState(precompile, slot)), nil - }, + run, func(b []byte) uint64 { return gasCost }, @@ -112,13 +120,175 @@ func TestNewStatefulPrecompile(t *testing.T) { state, evm := ethtest.NewZeroEVM(t) state.SetState(precompile, slot, value) - wantReturnData := makeOutput(caller, precompile, input, value) - wantGasLeft := gasLimit - gasCost - gotReturnData, gotGasLeft, err := evm.Call(vm.AccountRef(caller), precompile, input, gasLimit, uint256.NewInt(0)) - require.NoError(t, err) - assert.Equal(t, wantReturnData, gotReturnData) - assert.Equal(t, wantGasLeft, gotGasLeft) + tests := []struct { + name string + call func() ([]byte, uint64, error) + // Note that this only covers evm.readWrite being set to forceReadOnly, + // via StaticCall(). See TestInheritReadOnly for alternate case. + wantReadOnly bool + }{ + { + name: "EVM.Call()", + call: func() ([]byte, uint64, error) { + return evm.Call(vm.AccountRef(caller), precompile, input, gasLimit, uint256.NewInt(0)) + }, + wantReadOnly: false, + }, + { + name: "EVM.CallCode()", + call: func() ([]byte, uint64, error) { + return evm.CallCode(vm.AccountRef(caller), precompile, input, gasLimit, uint256.NewInt(0)) + }, + wantReadOnly: false, + }, + { + name: "EVM.DelegateCall()", + call: func() ([]byte, uint64, error) { + return evm.DelegateCall(vm.AccountRef(caller), precompile, input, gasLimit) + }, + wantReadOnly: false, + }, + { + name: "EVM.StaticCall()", + call: func() ([]byte, uint64, error) { + return evm.StaticCall(vm.AccountRef(caller), precompile, input, gasLimit) + }, + wantReadOnly: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wantReturnData := makeOutput(caller, precompile, input, value, tt.wantReadOnly) + wantGasLeft := gasLimit - gasCost + + gotReturnData, gotGasLeft, err := tt.call() + require.NoError(t, err) + assert.Equal(t, string(wantReturnData), string(gotReturnData)) + assert.Equal(t, wantGasLeft, gotGasLeft) + }) + } +} + +func TestInheritReadOnly(t *testing.T) { + // The regular test of stateful precompiles only checks the read-only state + // when called directly via vm.EVM.*Call*() methods. That approach will not + // result in a read-only state via inheritance, which occurs when already in + // a read-only environment there is a non-static call to a precompile. + // + // Test strategy: + // + // 1. Create a precompile that echoes its read-only status in the return + // data. We MUST NOT assert inside the precompile as we need proof that + // the precompile was actually called. + // + // 2. Create a bytecode contract that calls the precompile with CALL and + // propagates the return data. Using CALL (i.e. not STATICCALL) means + // that we know for certain that [forceReadOnly] isn't being used and, + // instead, the read-only state is being read from + // evm.interpreter.readOnly. + // + // 3. Assert that the returned input is as expected for the read-only state. + + // (1) + + var precompile common.Address + const precompileAddr = 255 + precompile[common.AddressLength-1] = precompileAddr + + const ( + ifReadOnly = iota + 1 // see contract bytecode for rationale + ifNotReadOnly + ) + hooks := &hookstest.Stub{ + PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{ + precompile: vm.NewStatefulPrecompile( + func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) { + if env.ReadOnly() { + return []byte{ifReadOnly}, nil + } + return []byte{ifNotReadOnly}, nil + }, + func([]byte) uint64 { return 0 }, + ), + }, + } + hookstest.Register(t, params.Extras[*hookstest.Stub, *hookstest.Stub]{ + NewRules: func(_ *params.ChainConfig, r *params.Rules, _ *hookstest.Stub, blockNum *big.Int, isMerge bool, timestamp uint64) *hookstest.Stub { + r.IsCancun = true // enable PUSH0 + return hooks + }, + }) + + // (2) + + // See CALL signature: https://www.evm.codes/#f1?fork=cancun + const p0 = vm.PUSH0 + contract := []vm.OpCode{ + vm.PUSH1, 1, // retSize (bytes) + p0, // retOffset + p0, // argSize + p0, // argOffset + p0, // value + vm.PUSH1, precompileAddr, + p0, // gas + vm.CALL, + // It's ok to ignore the return status. If the CALL failed then we'll + // return []byte{0} next, and both non-failure return buffers are + // non-zero because of the `iota + 1`. + vm.PUSH1, 1, // size (byte) + p0, + vm.RETURN, + } + + state, evm := ethtest.NewZeroEVM(t) + rng := ethtest.NewPseudoRand(42) + contractAddr := rng.Address() + state.CreateAccount(contractAddr) + state.SetCode(contractAddr, contractCode(contract)) + + // (3) + + caller := vm.AccountRef(rng.Address()) + tests := []struct { + name string + call func() ([]byte, uint64, error) + want byte + }{ + { + name: "EVM.Call()", + call: func() ([]byte, uint64, error) { + return evm.Call(caller, contractAddr, []byte{}, 1e6, uint256.NewInt(0)) + }, + want: ifNotReadOnly, + }, + { + name: "EVM.StaticCall()", + call: func() ([]byte, uint64, error) { + return evm.StaticCall(vm.AccountRef(rng.Address()), contractAddr, []byte{}, 1e6) + }, + want: ifReadOnly, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, _, err := tt.call() + require.NoError(t, err) + require.Equalf(t, []byte{tt.want}, got, "want %d if read-only, otherwise %d", ifReadOnly, ifNotReadOnly) + }) + } +} + +// contractCode converts a slice of op codes into a byte buffer for storage as +// contract code. +func contractCode(ops []vm.OpCode) []byte { + ret := make([]byte, len(ops)) + for i, o := range ops { + ret[i] = byte(o) + } + return ret } func TestCanCreateContract(t *testing.T) { diff --git a/core/vm/evm.go b/core/vm/evm.go index 7b830a343a7a..b846b20cbc3b 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -228,7 +228,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas } if isPrecompile { - args := &evmCallArgs{evm, caller, addr, input, gas, value} + args := &evmCallArgs{evm, caller, addr, input, gas, value, inheritReadOnly} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { // Initialise a new contract and set the code that is to be used by the EVM. @@ -292,7 +292,7 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte, // It is allowed to call precompiles, even via delegatecall if p, isPrecompile := evm.precompile(addr); isPrecompile { - args := &evmCallArgs{evm, caller, addr, input, gas, value} + args := &evmCallArgs{evm, caller, addr, input, gas, value, inheritReadOnly} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { addrCopy := addr @@ -338,7 +338,7 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by // It is allowed to call precompiles, even via delegatecall if p, isPrecompile := evm.precompile(addr); isPrecompile { - args := &evmCallArgs{evm, caller, addr, input, gas, nil} + args := &evmCallArgs{evm, caller, addr, input, gas, nil, inheritReadOnly} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { addrCopy := addr @@ -388,7 +388,7 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte } if p, isPrecompile := evm.precompile(addr); isPrecompile { - args := &evmCallArgs{evm, caller, addr, input, gas, nil} + args := &evmCallArgs{evm, caller, addr, input, gas, nil, forceReadOnly} ret, gas, err = args.RunPrecompiledContract(p, input, gas) } else { // At this point, we use a copy of address. If we don't, the go compiler will