Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

fix(TypedTransaction::decode): do not panic on bad rlp input #2694

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions ethers-core/src/types/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,10 @@ impl Chain {
("https://alpha-blockscout.scroll.io/api", "https://alpha-blockscout.scroll.io/")
}

Metis => {
("https://api.routescan.io/v2/network/mainnet/evm/1088/etherscan/api", "https://explorer.metis.io/")
}
Metis => (
"https://api.routescan.io/v2/network/mainnet/evm/1088/etherscan/api",
"https://explorer.metis.io/",
),

Chiado => {
("https://blockscout.chiadochain.net/api", "https://blockscout.chiadochain.net")
Expand Down Expand Up @@ -599,7 +600,7 @@ impl Chain {
AnvilHardhat | Dev | Morden | MoonbeamDev | FilecoinMainnet => {
// this is explicitly exhaustive so we don't forget to add new urls when adding a
// new chain
return None;
return None
}
};

Expand Down
51 changes: 27 additions & 24 deletions ethers-core/src/types/transaction/eip2718.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,34 +416,29 @@ impl TypedTransaction {
/// Get a TypedTransaction directly from a rlp encoded byte stream
impl Decodable for TypedTransaction {
fn decode(rlp: &rlp::Rlp) -> Result<Self, rlp::DecoderError> {
let tx_type: Option<U64> = match rlp.is_data() {
true => Some(rlp.data()?.into()),
false => None,
};
let rest = rlp::Rlp::new(
// We check for the transaction type based on the encoding format described in
// <https://eips.ethereum.org/EIPS/eip-2718/>.
// We expect the format to be `TransactionType || TransactionPayload`, where
// TransactionType is a raw single byte, and the TransactionPayload is assumed to be rlp
// encoded.
let tx_type =
rlp.data()?.first().ok_or(rlp::DecoderError::Custom("no transaction type byte"))?;

let tx_payload = rlp::Rlp::new(
rlp.as_raw().get(1..).ok_or(rlp::DecoderError::Custom("no transaction payload"))?,
);

match tx_type {
Some(x) if x == U64::from(1) => {
// EIP-2930 (0x01)
Ok(Self::Eip2930(Eip2930TransactionRequest::decode(&rest)?))
}
Some(x) if x == U64::from(2) => {
// EIP-1559 (0x02)
Ok(Self::Eip1559(Eip1559TransactionRequest::decode(&rest)?))
}
let typed_tx = match tx_type {
0x01 => Self::Eip2930(Eip2930TransactionRequest::decode(&tx_payload)?),
0x02 => Self::Eip1559(Eip1559TransactionRequest::decode(&tx_payload)?),
#[cfg(feature = "optimism")]
Some(x) if x == U64::from(0x7E) => {
// Optimism Deposited (0x7E)
Ok(Self::DepositTransaction(DepositTransaction::decode(&rest)?))
}
_ => {
// Legacy (0x00)
// use the original rlp
Ok(Self::Legacy(TransactionRequest::decode(rlp)?))
}
}
0x7e => Self::DepositTransaction(DepositTransaction::decode(&tx_payload)?),
// this must be a legacy tx so we expect the first byte to be part of the rlp encoding
_ if rlp.is_list() => Self::Legacy(TransactionRequest::decode(rlp)?),
_ => return Err(rlp::DecoderError::Custom("invalid tx type")),
};

Ok(typed_tx)
}
}

Expand Down Expand Up @@ -909,4 +904,12 @@ mod tests {
assert_eq!(tx0, tx1);
}
}

#[test]
fn malformed_transaction_does_not_panic() {
let tx = hex::decode("b9011e02f9011a0180850ba43b7400850df8475864830493e0947a250d5630b4cf539739df2c5dacb4c659f2488d88016345785d8a0000b8e4b6f9de95000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000005bd929b84c3dac16515c6b6c95f70fdae9c05c0c00000000000000000000000000000000000000000000000000000000650267140000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec7c0808080").unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where's this coming from?

Copy link
Author

@emostov emostov Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was given to me - apparently it was spit out from go-ethereum. But I also have encountered the same issue while fuzzing inputs to this function. Regardless of the exact payload, I don't think think we want it to panic. If you run the test on the commit prior (9ad0740) to the fix being introduced, it will panic and the test will fail

let tx_rlp = rlp::Rlp::new(&tx);
let result = TypedTransaction::decode(&tx_rlp);
assert_eq!(result.unwrap_err(), rlp::DecoderError::RlpExpectedToBeList);
}
}
2 changes: 1 addition & 1 deletion ethers-core/src/utils/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ where
return Numeric::from_str(num)
.map(U256::from)
.map(Some)
.map_err(serde::de::Error::custom);
.map_err(serde::de::Error::custom)
}

if let serde_json::Value::Number(num) = val {
Expand Down
Loading