From 55ab72cdd10e9d572ab7211168db96e7cf9d985a Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Fri, 4 Oct 2024 20:16:44 +0100 Subject: [PATCH] fix: `vm.WithUNSAFECallerAddressProxying` under `DELEGATECALL` --- core/vm/contracts.libevm.go | 22 +++------------------- core/vm/environment.libevm.go | 30 +++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go index 78b634ad4bf6..76c5fcdaf2a9 100644 --- a/core/vm/contracts.libevm.go +++ b/core/vm/contracts.libevm.go @@ -153,25 +153,9 @@ func (args *evmCallArgs) env() *environment { } return &environment{ - evm: args.evm, - self: contract, - forceReadOnly: args.readOnly(), - } -} - -func (args *evmCallArgs) readOnly() bool { - // A switch statement provides clearer code coverage for difficult-to-test - // cases. - switch { - case args.callType == staticCall: - // 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. - return true - case args.evm.interpreter.readOnly: - return true - default: - return false + evm: args.evm, + self: contract, + callType: args.callType, } } diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go index 75a10c0d8570..7e810de10b42 100644 --- a/core/vm/environment.libevm.go +++ b/core/vm/environment.libevm.go @@ -31,18 +31,33 @@ import ( var _ PrecompileEnvironment = (*environment)(nil) type environment struct { - evm *EVM - self *Contract - forceReadOnly bool + evm *EVM + self *Contract + callType callType } func (e *environment) ChainConfig() *params.ChainConfig { return e.evm.chainConfig } func (e *environment) Rules() params.Rules { return e.evm.chainRules } -func (e *environment) ReadOnly() bool { return e.forceReadOnly || e.evm.interpreter.readOnly } func (e *environment) ReadOnlyState() libevm.StateReader { return e.evm.StateDB } func (e *environment) BlockNumber() *big.Int { return new(big.Int).Set(e.evm.Context.BlockNumber) } func (e *environment) BlockTime() uint64 { return e.evm.Context.Time } +func (e *environment) ReadOnly() bool { + // A switch statement provides clearer code coverage for difficult-to-test + // cases. + switch { + case e.callType == staticCall: + // 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. + return true + case e.evm.interpreter.readOnly: + return true + default: + return false + } +} + func (e *environment) Addresses() *libevm.AddressContext { return &libevm.AddressContext{ Origin: e.evm.Origin, @@ -83,7 +98,7 @@ func (e *environment) callContract(typ callType, addr common.Address, input []by in.evm.depth++ defer func() { in.evm.depth-- }() - if e.forceReadOnly && !in.readOnly { // i.e. the precompile was StaticCall()ed + if e.ReadOnly() && !in.readOnly { // i.e. the precompile was StaticCall()ed in.readOnly = true defer func() { in.readOnly = false }() } @@ -95,6 +110,11 @@ func (e *environment) callContract(typ callType, addr common.Address, input []by // Note that, in addition to being unsafe, this breaks an EVM // assumption that the caller ContractRef is always a *Contract. caller = AccountRef(e.self.CallerAddress) + if e.callType == delegateCall { + // self was created with AsDelegate(), which means that + // CallerAddress was inherited. + caller = AccountRef(e.self.Address()) + } case nil: default: return nil, gas, fmt.Errorf("unsupported option %T", o)