[Bug] 0x00-tagged legacy transaction is accepted by decode_2718 and re-encodes untagged
- Langage dominant
- Rust
- Étoiles
- 1.3k
- Forks
- 668
- Merge moyen
- 2 j 2 h
- PR mergées (30 j)
- 29
Description
**Issue.** `TxType::try_from(0)` returns `TxType::Legacy`, so the `TransactionEnvelope`-generated `Decodable2718::typed_decode` accepts an EIP-2718 envelope whose type byte is `0x00` and decodes the body as a legacy transaction. Per EIP-2718 a legacy transaction is the untagged form and `0x00` is not an assigned `TransactionType`, so this should be rejected the way `0x05` and `0x7f` already are. `EthereumTxEnvelope` has a single `Legacy` variant carrying no record of whether a tag was present, so `encode_2718` then emits the untagged form and the transaction does not round-trip.
This contradicts the type's own documentation (`alloy-consensus/src/transaction/envelope.rs`, doc comment on `EthereumTxEnvelope`):
> This enum distinguishes between tagged and untagged legacy transactions, as the in-protocol merkle tree may commit to EITHER 0-prefixed or raw. Therefore we must ensure that encoding returns the precise byte-array that was decoded, preserving the presence or absence of the `TransactionType` flag.
The distinction the comment describes is not representable: `Legacy` is declared `#[envelope(ty = 0)]`, which is also what makes the macro-generated `TryFrom` map `0` to it.
**Repro** (alloy-consensus 2.4.1, the current release):
```rust
use alloy_consensus::{SignableTransaction, Typed2718, TxEnvelope, TxLegacy};
use alloy_eips::eip2718::{Decodable2718, Encodable2718};
use alloy_primitives::{Signature, TxKind, U256};
fn main() {
let tx = TxLegacy {
chain_id: None,
nonce: 0,
gas_price: 10,
gas_limit: 21_000,
to: TxKind::Call([0x11; 20].into()),
value: U256::ZERO,
input: Default::default(),
};
let sig = Signature::new(U256::from(1), U256::from(2), false);
let untagged = tx.into_signed(sig).encoded_2718();
let mut tagged = vec![0x00u8];
tagged.extend_from_slice(&untagged);
for (label, bytes) in [("untagged ", &untagged), ("tagged 0x00", &tagged)] {
match TxEnvelope::decode_2718_exact(bytes) {
Ok(env) => {
let re = env.encoded_2718();
println!(
"{label} -> Ok(ty={}), {} bytes in, {} bytes out, round-trips: {}",
env.ty(), bytes.len(), re.len(), re == **bytes
);
}
Err(e) => println!("{label} -> Err({e})"),
}
}
for ty in [0x05u8, 0x7f] {
let mut b = vec![ty];
b.extend_from_slice(&untagged);
println!("tagged {ty:#04x} -> {:?}", TxEnvelope::decode_2718_exact(&b).map(|_| ()));
}
}
```
```
untagged -> Ok(ty=0), 32 bytes in, 32 bytes out, round-trips: true
tagged 0x00 -> Ok(ty=0), 33 bytes in, 32 bytes out, round-trips: false
tagged 0x05 -> Err(RlpError(Custom("unexpected tx type")))
tagged 0x7f -> Err(RlpError(Custom("unexpected tx type")))
```
Found by evmone-fuzz.
Same scope caveat as #4089: this is decoder conformance, not a consensus split. A block carrying such a transaction is still rejected by reth, because the engine path derives `transactions_root` over the raw payload bytes (`ExecutionPayload::into_block_raw` -> `ordered_trie_root_encoded`) while `validate_body_against_header` recomputes it with `calculate_tx_root` over re-encoded transactions, and the two differ. Confirmed end-to-end with hive `consume engine` against real reth and real geth: both return INVALID. Consumers that decode and re-encode without such a cross-check would silently alter the transaction.
Related: #4090 fixed the mirror case on the untagged side (`Signed::fallback_decode` accepting a non-legacy list). This is the tagged side of the same distinction.
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.