fix(rpc): close signed-read replay gap via decoder-level rejection (Veridise-1083) - #383
fix(rpc): close signed-read replay gap via decoder-level rejection (Veridise-1083)#383samlaf wants to merge 0 commit into
Conversation
|
Based on my analysis of the diff and the commit message, I can now provide a comprehensive review. Moves signed-read validation from mempool to decoder level to close replay attack vulnerability. The changes look correct and address an important security issue where attackers could replay intercepted Phase 1
Phase 2
The refactoring successfully consolidates transaction handling by removing the parallel |
d9c5647 to
b4a85e6
Compare
|
Closed since this was included as part of #386 which was merged into the veridise-audit-april-2026 branch already. |
…(Veridise 1083) (#422) Fixes Veridise-1083. Depends on SeismicSystems/seismic-alloy#104. Signed-read seismic txs are an eth_call-only construct and must never be executed as a state transition. PR #383 (#383) added a decode-time rejection for this, but only on the pooled-type decoder SeismicTxEnvelope::typed_decode — the RPC / mempool / tx-gossip path. Block-ingestion paths decode via the consensus type SeismicTransactionSigned::decode_2718 instead (engine newPayload, the block executor's tx iterator, and p2p block bodies), which never goes through the pooled decoder. So even with #383 merged, a block proposer could still include a signed-read tx directly in a block and have every node execute it as a state-changing tx — replaying an intercepted signed eth_call payload as a write. This is the block-ingestion gap Veridise 1083 flagged. Close it by adding the same rejection to the consensus decoder, gated on signed_read alone (regardless of `to`). Such txs are now non-decodable from the wire, so the Arbitrary impl clears the flag to keep generating valid wire txs. Add regression tests for the reject/accept cases.
…(Veridise 1083) (#422) Fixes Veridise-1083. Depends on SeismicSystems/seismic-alloy#104. Signed-read seismic txs are an eth_call-only construct and must never be executed as a state transition. PR #383 (#383) added a decode-time rejection for this, but only on the pooled-type decoder SeismicTxEnvelope::typed_decode — the RPC / mempool / tx-gossip path. Block-ingestion paths decode via the consensus type SeismicTransactionSigned::decode_2718 instead (engine newPayload, the block executor's tx iterator, and p2p block bodies), which never goes through the pooled decoder. So even with #383 merged, a block proposer could still include a signed-read tx directly in a block and have every node execute it as a state-changing tx — replaying an intercepted signed eth_call payload as a write. This is the block-ingestion gap Veridise 1083 flagged. Close it by adding the same rejection to the consensus decoder, gated on signed_read alone (regardless of `to`). Such txs are now non-decodable from the wire, so the Arbitrary impl clears the flag to keep generating valid wire txs. Add regression tests for the reject/accept cases.
Fixes veridise-1083.
Depends on SeismicSystems/seismic-alloy#104.
Signed-read seismic transactions were previously rejected only at mempool admission. This might (?) work in a TEE world but is fragile, and given that we are planning to go to mainnet without TEEs it was a real issue. We might also one day want to enable external block building via builder API, which would bypass the mempool.
signed_readcheck is now done as part of 2718 decoding, which happens in:eth_calluses a special purposerecover_raw_seismic_call_txfunction which allowssigned_read=truetxs.Side note
In
send_raw_transaction, theTypedDataarm now decodes the EIP-712 payload into aSeismicTxEnvelope, re-encodes as RLP, and delegates toEthTransactions::send_raw_transaction(bytes). This makes sure all ingestion paths go through the 2718 decoding function. Also added a TODO mentioning that this ingestion path is not needed, and we could update our clients to send via the Bytes path directly.More generally, this is a first step in the right direction, but I think the even cleaner design is to enforce signed_reads cryptoraphically instead. See the "Future hard-fork requiring change to SeismicTx" section in SeismicSystems/seismic-alloy#104