erigontech / erigontech/erigon
txpool: malformed sender signature on any tx drops sibling txs in the same Transactions/PooledTransactions packet
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
## Summary
Same shape of bug as #20809 / [ethereum-bounty/erigon#7](https://github.com/ethereum-bounty/erigon/issues/7), but on the *sender* signature path rather than the EIP-7702 auth-tuple path — and it applies to any transaction type, not just SetCode.
A `Transactions (0x02)` or `PooledTransactions (0x0a)` devp2p packet containing one transaction with an unrecoverable sender signature (e.g. `r=0`) causes Erigon's parser to return `ErrParseTxn`. The batch loop in `pool_txn_packets.go` only swallows `ErrRejected`; on `ErrParseTxn` it aborts, dropping every sibling transaction that had already been parsed in the same packet, and `fetch.go` then kicks the peer.
A malicious peer can craft a cheap dummy tx with `r=0`, splice it into a packet alongside whatever sibling txs they want to suppress, and selectively censor those siblings while looking honest at the gossip layer.
## Code
Sender recovery error path — [`pool_txn_parser.go:436-438`](https://github.com/erigontech/erigon/blob/main/txnprovider/txpool/pool_txn_parser.go#L436-L438):
```go
addr, err := txn.Sender(*signer)
if err != nil {
return 0, fmt.Errorf("%w: recovering sender from signature: %s", ErrParseTxn, err) //nolint
}
```
Batch loops that only continue on `ErrRejected` — [`pool_txn_packets.go:181-187`](https://github.com/erigontech/erigon/blob/main/txnprovider/txpool/pool_txn_packets.go#L181-L187) and [`pool_txn_packets.go:212-218`](https://github.com/erigontech/erigon/blob/main/txnprovider/txpool/pool_txn_packets.go#L212-L218):
```go
pos, err = ctx.ParseTransaction(payload, pos, txnSlots.Txns[i], …)
if err != nil {
if errors.Is(err, ErrRejected) {
txnSlots.Resize(uint(i))
i--
continue
}
return 0, err
}
```
Geth, by contrast, defers sender recovery (typically lazy / at admission), so a malformed sender signature on one tx does not poison its packet siblings.
## Suggested fix
Mirror the EIP-7702 fix in #20809 for sender-sig recovery. Two reasonable options:
1. **Narrow**: change the wrap from `ErrParseTxn` to `ErrRejected` and return `p` (the position past the consumed tx) instead of `0`, so `ParseTransactions` / `ParsePooledTransactions66` skip the bad tx and continue with siblings. The position past the tx is already known by the time Step 8 runs. Bad tx is dropped, siblings survive, peer is not kicked.
2. **Proper**: defer sender recovery to the txpool's `validateTx` / admission stage — closer to Geth's design — so parsing is a pure structural transform that never touches signatures. More invasive.
I'd start with the narrow fix and consider (2) as a separate refactor.
## Repro sketch
The same approach as the test added in #20809 (`TestEIP7702BatchPoisoning`), but with the middle tx being a plain `DynamicFeeTransaction` whose `R` is set to zero post-signing instead of a SetCode tx with a poisoned auth tuple. With the current code `ParseTransactions` returns `ErrParseTxn` and `slots.Txns` ends up populated only up to the bad tx; with a fix all three should survive.
## Scope
Out of scope of #20809 because the bounty issue ([ethereum-bounty/erigon#7](https://github.com/ethereum-bounty/erigon/issues/7)) was scoped to EIP-7702. Filing this so the broader pattern doesn't get lost.
Contributor guide
Assessment
This issue has not been assessed yet.