Missing overflow check for AccessList cost addition in IntrinsicGas function

[Kourin1996]

Geth version: 1.14.13-unstable
Commit hash : master branch (08e6bdb550712503873fb2a138b30132cc36c481)

IntrinsicGas in core/state_transition.go calculates intrinsic gas for a given transaction. Cost calculation of data field take care of overflow while the calculation for Access List doesn't. (Please see code below)

go-ethereum/core/state_transition.go

Lines 104 to 115 in 08e6bdb

	if isContractCreation && isEIP3860 {
	lenWords := toWordSize(dataLen)
	if (math.MaxUint64-gas)/params.InitCodeWordGas < lenWords {
	return 0, ErrGasUintOverflow
	}
	gas += lenWords * params.InitCodeWordGas
	}
	}
	if accessList != nil {
	gas += uint64(len(accessList)) * params.TxAccessListAddressGas
	gas += uint64(accessList.StorageKeys()) * params.TxAccessListStorageKeyGas
	}

In general, a transaction causing overflow at this point might be blocked by another check (e.g. TxPool slot limit). But evm transaction command which validates a transaction offline might return wrong result by this problem.

Ideally, IntrinsicGas should check overflow in calculation for AccessList cost like this:

if accessList != nil {
	numItems := uint64(len(accessList))
	if (math.MaxUint64-gas)/params.TxAccessListAddressGas < numItems {
		return 0, ErrGasUintOverflow
	}
	gas += numItems * params.TxAccessListAddressGas

	numStorageKeys := uint64(accessList.StorageKeys())
	if (math.MaxUint64-gas)/params.TxAccessListStorageKeyGas < numStorageKeys {
		return 0, ErrGasUintOverflow
	}
	gas += numStorageKeys * params.TxAccessListStorageKeyGas
}

[holiman]

Well.. for that to overflow, the length of the accesslist would have to be somewhere on the order of 7e15?

>>> 0xffffffffffffffff // 2400
7686143364045646

Might be that the preceding calculation on data is overly paranoid: I don't see a reason to make the accessList implementation go to the same lengths.

?

[rjl493456442]

Yeah, but to be fair, the TxDataZeroGas overflow checking is also theoretical needed but in practice unnecessary.

I would lean to align the behaviors.