-
Notifications
You must be signed in to change notification settings - Fork 21.5k
Description
System information
Geth version: geth version
CL client & version: e.g. lighthouse/nimbus/prysm@v1.0.0
OS & Version: Windows/Linux/OSX
Commit hash : (if develop)
Expected behaviour
the core.ParseDepositLogs should only include ABI-valid deposit events in the EIP-7685 requests list.
and the types.DepositLogToRequest should verify the deposit event’s ABI head offsets and dynamic lengths (for bytes,bytes,bytes,bytes,bytes) before slicing:
- head offsets: 160, 256, 320, 384, 512
- lengths: 48, 32, 8, 96, 8
with full bounds checks.
Actual behaviour
the types.DepositLogToRequest only checks len(data) == 576, then copies fixed slices using hard-coded increments.
It does not validate the 5 head offsets or the 5 length words.
so Any 576-byte payload (with the correct topic/address filter at the call site) is treated as a valid deposit, which can change the derived EIP-7685 requestsHash compared to a stricter client that enforces the ABI.
- Note: This is not exploitable on Ethereum mainnet because is alerdy talk to the teams about this and this say to open a issue here, the mainnet not affected because logs come only from the canonical deposit contract. It is, however, a spec-compliance gap and a potential cross-client divergence for other chains/configs or future stricter decoders.
Steps to reproduce the behaviour
here is the Vulnerable call chain:
in the --->
go-ethereum/core/state_processor.go
Line 322 in 8ce2047
| request, err := types.DepositLogToRequest(log.Data) |
https://github.com/ethereum/go-ethereum/blob/8ce204734879580a0a38e13708c8f473967eac83/core/types/deposit.go#L27C1-L32C1
- here in the core/state_processor.go: ParseDepositLogs(&requests, allLogs, cfg) --> types.DepositLogToRequest(log.Data)
- here in the core/types/deposit.go: only checks len(data)==576, then slices fixed positions (no ABI validation)
Impact
So With EIP-7685, the deposits contribute to the header’s requestsHash.
A lenient vs strict decoder can derive different requestsHash for the same block if the deposit payload is malformed (relevant for non-mainnet chains/configs that point DepositContractAddress elsewhere, devnets, forks, or future stricter clients).
Backtrace
here is a proposed fix
In types.DepositLogToRequest:
- need to Keep len(data) == 576 check.
- and need to Read the 5 head offsets (first 5 words); require exactly 160/256/320/384/512 and in-bounds.
- At each head, read the 32-byte length; require 48/32/8/96/8 and in-bounds.
- Bounds-check all slices; parse amount/index (8 bytes) as per the spec.
- Return error on any mismatch.
(Defense-in-depth in ParseDepositLogs: optionally require len(log.Topics) == 1 in addition to topic0 == depositTopic, since the deposit receipt is a single-topic event.)
Backtrace
N/A (logic issue; reproducible via unit test)