[Tracker] EVM internal audit

[preston-evans98]

Description

This issue tracks internal audit progress.

Areas to Audit

• EVM module initialization
• EVM authenticator
• EVM execute call
• EVM RPC handlers

Items to Fix by Severity

Critical

• StorageInspector rebates gas costs as they happen (since these accesses are charged by the SDK later). This means that a caller can run a (roughly) infinite loop of SSTOREs, causing the client to hang. Instead, costs should be rebated after the EVM returns. Move EVM storage rebate after execution #2291

High

• No practical gas limit is enforced on EVM txs when the paymaster is active. We should set a configurable hard limit on EVM tx consumption, defaulting to 30M gas. Re-enable ETH block gas limit #2256

Moderate:

• No way to update EvmRuntimeConfig after genesis: Allow updating EVM chain configs #2292
  • Support add/remove allowed contract deployers
  • Support modifying hardforks, limit_contract_code_size and block_gas_limit.
  • Determine if coinbase address is used. If so, determine whether to allow changes.
• EVM Authenticator accepts all tx types and coerces them to EIP-1559. Only EIP-1559 txs should be accepted Reject txs other than EIP-1559 #2294
• CfgEnv::memory_limit should be set since we've raised the gas limit significantly. 50MB seems reasonable. Set memory limit on EVM as fallback if gas limit is raised #2293
• Our use of CfgEnv.disable_balance_checks is incorrect. This option should be left with its default value Re-enable ETH block gas limit #2256
• Rollup may panic if exactly the right amount of funds is sent to cover the prechecks. Add test for money printing vuln. Fix panic at genesis #2270

Low

• validate_chain_id doesn't respect EIP-7702 (should accept 0) Allow chainid 0 for compatibility with EIP7702 #2295
• We allow the code_hash to be set to an unrelated value for accounts set at genesis. We should compute the hash from the code instead.
• RawTx, FullyBakedTx, and RlpEvmTransaction are all missing length limits. Set RawTx and FullyBakedTx to 1MB. Leave RlpEvmTransaction as is. Add limits on RawTx and FullyBakedTx sizes #2296
• Re-examine TODOS in EVM DB; should be fixed, removed, or marked as non-security critical
  • TODO move to new_raw_with_hash for better performance
  • TODO: Here we generate a default account and set the balance from bank
• (Optional) EvmDb::code_by_hash doesn't charge gas when the code is in temp_cache. This technically violates the API of the temp_cache, which states that using the cache should be indistinguishable from accessing storage because the cache can be cleared at any time and for any reason
• (Optional) Add BLS precompile support

Won't Fix Before Ale Mainnet

• Access lists are silently dropped by the transaction deserializer
• Authorization lists are silently dropped by the transaction deserializer
• CfgEnv constructor for ETH call doesn't match accept_tx.
• SovHandler::validate_against_state_and_deduct_caller doesn't enforce EIP-3607 or 7702.
• SovHandler::validate_against_state_and_deduct_caller won't get support for new EIPs as they are added to Reth.

Open Questions

• Why do we pass RlpEvmTransaction to the EVM module rather than, say, Signed<Eip1559Tx>. Answer: We want the exact tx bytes from the user to go on chain
• Do we need to support EIP-7702? If so, what changes are needed besides enabling authorization lists and un-gating the tx type?