core/vm: add read only protection for opcodes (#33637)

This PR reverts a part of changes brought by https://github.com/ethereum/go-ethereum/pull/33281/changes

Specifically, read-only protection should always be enforced at the opcode level, 
regardless of whether the check has already been performed during gas metering.

It should act as a gatekeeper, otherwise, it is easy to introduce errors by adding
new gas measurement logic without consistently applying the read-only protection.
This commit is contained in:
rjl493456442 2026-01-19 20:43:14 +08:00 committed by GitHub
parent ef815c59a2
commit 500931bc82
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 15 additions and 4 deletions

View file

@ -518,6 +518,9 @@ func opSload(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) {
} }
func opSstore(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) { func opSstore(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) {
if evm.readOnly {
return nil, ErrWriteProtection
}
loc := scope.Stack.pop() loc := scope.Stack.pop()
val := scope.Stack.pop() val := scope.Stack.pop()
evm.StateDB.SetState(scope.Contract.Address(), loc.Bytes32(), val.Bytes32()) evm.StateDB.SetState(scope.Contract.Address(), loc.Bytes32(), val.Bytes32())
@ -740,6 +743,9 @@ func opCall(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) {
// Get the arguments from the memory. // Get the arguments from the memory.
args := scope.Memory.GetPtr(inOffset.Uint64(), inSize.Uint64()) args := scope.Memory.GetPtr(inOffset.Uint64(), inSize.Uint64())
if evm.readOnly && !value.IsZero() {
return nil, ErrWriteProtection
}
if !value.IsZero() { if !value.IsZero() {
gas += params.CallStipend gas += params.CallStipend
} }
@ -876,13 +882,15 @@ func opStop(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) {
} }
func opSelfdestruct(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) { func opSelfdestruct(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) {
if evm.readOnly {
return nil, ErrWriteProtection
}
var ( var (
this = scope.Contract.Address() this = scope.Contract.Address()
balance = evm.StateDB.GetBalance(this) balance = evm.StateDB.GetBalance(this)
top = scope.Stack.pop() top = scope.Stack.pop()
beneficiary = common.Address(top.Bytes20()) beneficiary = common.Address(top.Bytes20())
) )
// The funds are burned immediately if the beneficiary is the caller itself, // The funds are burned immediately if the beneficiary is the caller itself,
// in this case, the beneficiary's balance is not increased. // in this case, the beneficiary's balance is not increased.
if this != beneficiary { if this != beneficiary {
@ -904,15 +912,16 @@ func opSelfdestruct(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) {
} }
func opSelfdestruct6780(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) { func opSelfdestruct6780(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) {
if evm.readOnly {
return nil, ErrWriteProtection
}
var ( var (
this = scope.Contract.Address() this = scope.Contract.Address()
balance = evm.StateDB.GetBalance(this) balance = evm.StateDB.GetBalance(this)
top = scope.Stack.pop() top = scope.Stack.pop()
beneficiary = common.Address(top.Bytes20()) beneficiary = common.Address(top.Bytes20())
newContract = evm.StateDB.IsNewContract(this) newContract = evm.StateDB.IsNewContract(this)
) )
// Contract is new and will actually be deleted. // Contract is new and will actually be deleted.
if newContract { if newContract {
if this != beneficiary { // Skip no-op transfer when self-destructing to self. if this != beneficiary { // Skip no-op transfer when self-destructing to self.

View file

@ -237,8 +237,10 @@ func makeSelfdestructGasFn(refundsEnabled bool) gasFunc {
evm.StateDB.AddAddressToAccessList(address) evm.StateDB.AddAddressToAccessList(address)
gas = params.ColdAccountAccessCostEIP2929 gas = params.ColdAccountAccessCostEIP2929
// Terminate the gas measurement if the leftover gas is not sufficient,
// it can effectively prevent accessing the states in the following steps
if contract.Gas < gas { if contract.Gas < gas {
return gas, nil return 0, ErrOutOfGas
} }
} }
// if empty and transfers value // if empty and transfers value