alloy-rs / alloy-rs/alloy

[Bug] 0x00-tagged legacy transaction is accepted by decode_2718 and re-encodes untagged

Ouverte
#4,165 3 commentaires 0 réactions 1 personne assignée Réclamée par @mattsse Voir sur GitHub
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.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.