diff --git a/Cargo.lock b/Cargo.lock index af1e2c5bf5..b1582e0e6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2862,8 +2862,7 @@ dependencies = [ [[package]] name = "ethereum" version = "0.18.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ee371ebb7479ed3258617557ab0b3247e741075cb6b02b820d188f68da44441" +source = "git+https://github.com/rust-ethereum/ethereum.git?rev=5185fe8efa62add7e989660a69c6945c8a3ae64e#5185fe8efa62add7e989660a69c6945c8a3ae64e" dependencies = [ "bytes", "ethereum-types", diff --git a/Cargo.toml b/Cargo.toml index adf36ff37c..1436c87d82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -228,6 +228,10 @@ precompile-utils = { path = "precompiles", default-features = false } # Frontier Template frontier-template-runtime = { path = "template/runtime", default-features = false } +# TODO: Remove this patch before merging once ethereum 0.19.0 is released +[patch.crates-io] +ethereum = { git = "https://github.com/rust-ethereum/ethereum.git", rev = "5185fe8efa62add7e989660a69c6945c8a3ae64e" } + [profile.release] # Substrate runtime requires unwinding. panic = "unwind" diff --git a/client/rpc-core/src/types/mod.rs b/client/rpc-core/src/types/mod.rs index f824cb1143..74981378f2 100644 --- a/client/rpc-core/src/types/mod.rs +++ b/client/rpc-core/src/types/mod.rs @@ -63,7 +63,9 @@ pub use self::{ Peers, PipProtocolInfo, SyncInfo, SyncStatus, TransactionStats, }, transaction::{LocalTransactionStatus, RichRawTransaction, Transaction}, - transaction_request::{TransactionMessage, TransactionRequest}, + transaction_request::{ + TransactionMessage, TransactionRequest, DEFAULT_MAX_TX_INPUT_BYTES, TX_SLOT_BYTE_SIZE, + }, work::Work, }; diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 4eea1cdb01..e20f6748e1 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -25,6 +25,13 @@ use serde::{Deserialize, Deserializer}; use crate::types::Bytes; +/// The default byte size of a transaction slot (32 KiB). +pub const TX_SLOT_BYTE_SIZE: usize = 32 * 1024; + +/// The default maximum size a single transaction can have (128 KiB). +/// This is the RLP-encoded size of the signed transaction. +pub const DEFAULT_MAX_TX_INPUT_BYTES: usize = 4 * TX_SLOT_BYTE_SIZE; + /// Transaction request from the RPC. #[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] @@ -174,6 +181,27 @@ impl TransactionRequest { fn chain_id_u64(&self) -> u64 { self.chain_id.map(|id| id.as_u64()).unwrap_or_default() } + + /// Calculates the RLP-encoded size of the signed transaction for DoS protection. + /// + /// This converts the request to a `TransactionMessage` using default values for any + /// missing fields, then delegates to `TransactionMessage::encoded_length()`. + /// + /// **Note:** For `eth_sendTransaction`, prefer calling `TransactionMessage::validate_size()` + /// on the fully-populated message (after nonce, gas, fees, chain ID are filled in) + /// to get an accurate size measurement. + /// + /// Returns an error if the transaction request cannot be converted to a valid message type. + pub fn encoded_length(&self) -> Result { + let message: Option = self.clone().into(); + + match message { + Some(msg) => Ok(msg.encoded_length()), + None => Err( + "invalid transaction parameters: unable to determine transaction type".to_string(), + ), + } + } } /// Additional data of the transaction. @@ -238,6 +266,57 @@ pub enum TransactionMessage { EIP7702(EIP7702TransactionMessage), } +impl TransactionMessage { + /// RLP overhead for signature fields (yParity + r + s) + /// - yParity: 1 byte (0x00 or 0x01 encoded as single byte) + /// - r: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) + /// - s: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) + const SIGNATURE_RLP_OVERHEAD: usize = 1 + 33 + 33; + + /// Calculates the RLP-encoded size of the signed transaction for DoS protection. + /// + /// This mirrors geth's `tx.Size()` and reth's `transaction.encoded_length()` which use + /// actual RLP encoding to determine transaction size. We use the `encoded_len()` method + /// from the ethereum crate on the fully-populated message. + pub fn encoded_length(&self) -> usize { + match self { + // Legacy: RLP([nonce, gasPrice, gasLimit, to, value, data, v, r, s]) + TransactionMessage::Legacy(msg) => msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD, + // EIP-2930: 0x01 || RLP([chainId, nonce, gasPrice, gasLimit, to, value, data, accessList, yParity, r, s]) + TransactionMessage::EIP2930(msg) => { + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + // EIP-1559: 0x02 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, yParity, r, s]) + TransactionMessage::EIP1559(msg) => { + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + // EIP-7702: 0x04 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, authorizationList, yParity, r, s]) + TransactionMessage::EIP7702(msg) => { + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + } + } + + /// Validates that the estimated signed transaction size is within limits. + /// + /// This prevents DoS attacks via oversized transactions before they enter the pool. + /// The limit matches geth's `txMaxSize` and reth's `DEFAULT_MAX_TX_INPUT_BYTES`. + /// + /// This should be called on the fully-populated message (after nonce, gas limit, + /// gas price, chain ID, etc. have been filled in) to ensure the final transaction + /// size is accurately measured. + pub fn validate_size(&self) -> Result<(), String> { + let size = self.encoded_length(); + + if size > DEFAULT_MAX_TX_INPUT_BYTES { + return Err(format!( + "oversized data: transaction size {size} exceeds limit {DEFAULT_MAX_TX_INPUT_BYTES}" + )); + } + Ok(()) + } +} + impl From for Option { fn from(req: TransactionRequest) -> Self { // Common fields extraction - these are used by all transaction types @@ -446,4 +525,168 @@ mod tests { } ); } + + #[test] + fn test_message_size_validation_large_access_list() { + use ethereum::AccessListItem; + use ethereum_types::{H160, H256}; + + // Create access list that exceeds 128KB (131,072 bytes) + // Each storage key RLP-encodes to ~33 bytes + // 4000 keys * 33 bytes = 132,000 bytes > 128KB + let storage_keys: Vec = (0..4000).map(|_| H256::default()).collect(); + let access_list = vec![AccessListItem { + address: H160::default(), + storage_keys, + }]; + let request = TransactionRequest { + access_list: Some(access_list), + ..Default::default() + }; + let message: Option = request.into(); + assert!(message.unwrap().validate_size().is_err()); + } + + #[test] + fn test_message_size_validation_valid() { + use ethereum::AccessListItem; + use ethereum_types::{H160, H256}; + + // 100 storage keys is well under 128KB + let request = TransactionRequest { + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![H256::default(); 100], + }]), + ..Default::default() + }; + let message: Option = request.into(); + assert!(message.unwrap().validate_size().is_ok()); + } + + #[test] + fn test_encoded_length_includes_signature_overhead() { + // A minimal EIP-1559 transaction should include signature overhead + // Default TransactionRequest converts to EIP-1559 (no gas_price, no access_list) + let request = TransactionRequest::default(); + let size = request + .encoded_length() + .expect("should encode valid EIP-1559 request"); + + // EIP-1559 message RLP: ~11 bytes for minimal fields (all zeros/empty) + // + 1 byte type prefix + 67 bytes signature overhead = ~79 bytes minimum + // The signature overhead (67 bytes) is the key verification + assert!( + size >= TransactionMessage::SIGNATURE_RLP_OVERHEAD, + "Size {} should be at least signature overhead {}", + size, + TransactionMessage::SIGNATURE_RLP_OVERHEAD + ); + + // Verify it's a reasonable size for a minimal transaction + assert!( + size < 200, + "Size {size} should be reasonable for minimal tx" + ); + } + + #[test] + fn test_encoded_length_typed_transaction_overhead() { + use ethereum::AccessListItem; + use ethereum_types::H160; + + // EIP-1559 transaction (has max_fee_per_gas) + let request = TransactionRequest { + max_fee_per_gas: Some(U256::from(1000)), + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![], + }]), + ..Default::default() + }; + let typed_size = request + .encoded_length() + .expect("should encode valid EIP-1559 request"); + + // Legacy transaction + let legacy_request = TransactionRequest { + gas_price: Some(U256::from(1000)), + ..Default::default() + }; + let legacy_size = legacy_request + .encoded_length() + .expect("should encode valid legacy request"); + + // Typed transaction should be larger due to: + // - Type byte (+1) + // - Chain ID (+9) + // - max_priority_fee_per_gas (+33) + // - Access list overhead + assert!( + typed_size > legacy_size, + "Typed tx {typed_size} should be larger than legacy {legacy_size}" + ); + } + + #[test] + fn test_encoded_length_access_list_scaling() { + use ethereum::AccessListItem; + use ethereum_types::{H160, H256}; + + // Transaction with 10 storage keys + let request_10 = TransactionRequest { + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![H256::default(); 10], + }]), + ..Default::default() + }; + + // Transaction with 100 storage keys + let request_100 = TransactionRequest { + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![H256::default(); 100], + }]), + ..Default::default() + }; + + let size_10 = request_10 + .encoded_length() + .expect("should encode valid EIP-1559 request with 10 keys"); + let size_100 = request_100 + .encoded_length() + .expect("should encode valid EIP-1559 request with 100 keys"); + + // Size should scale roughly linearly with storage keys + // 90 additional keys * ~34 bytes each ≈ 3060 bytes difference + let diff = size_100 - size_10; + assert!( + diff > 2500 && diff < 4000, + "Size difference {diff} should be proportional to storage keys" + ); + } + + #[test] + fn test_constants_match_geth_reth() { + // Verify our constants match geth/reth exactly + assert_eq!(TX_SLOT_BYTE_SIZE, 32 * 1024); // 32 KiB + assert_eq!(DEFAULT_MAX_TX_INPUT_BYTES, 128 * 1024); // 128 KiB + assert_eq!(DEFAULT_MAX_TX_INPUT_BYTES, 4 * TX_SLOT_BYTE_SIZE); + } + + #[test] + fn test_encoded_length_invalid_parameters() { + // Transaction with both max_fee_per_gas and gas_price set (invalid combination) + let request = TransactionRequest { + max_fee_per_gas: Some(U256::from(1000)), + gas_price: Some(U256::from(500)), + ..Default::default() + }; + + assert!(request.encoded_length().is_err()); + // Invalid parameters also fail conversion, so validate_size is unreachable + let message: Option = request.into(); + assert!(message.is_none()); + } } diff --git a/client/rpc/src/eth/submit.rs b/client/rpc/src/eth/submit.rs index 8e26a83a1a..9f35c15024 100644 --- a/client/rpc/src/eth/submit.rs +++ b/client/rpc/src/eth/submit.rs @@ -29,7 +29,7 @@ use sp_core::H160; use sp_inherents::CreateInherentDataProviders; use sp_runtime::{traits::Block as BlockT, transaction_validity::TransactionSource}; // Frontier -use fc_rpc_core::types::*; +use fc_rpc_core::types::{TransactionRequest, DEFAULT_MAX_TX_INPUT_BYTES, *}; use fp_rpc::{ConvertTransaction, ConvertTransactionRuntimeApi, EthereumRuntimeRPCApi}; use crate::{ @@ -128,6 +128,12 @@ where _ => return Err(internal_err("invalid transaction parameters")), }; + // Validate size on the fully-populated message (after nonce, gas, fees, chain ID + // have been filled in) to accurately enforce the 128 KiB limit. + message + .validate_size() + .map_err(|msg| crate::err(jsonrpsee::types::error::INVALID_PARAMS_CODE, &msg, None))?; + let mut transaction = None; for signer in &self.signers { if signer.accounts().contains(&from) { @@ -160,6 +166,19 @@ where return Err(internal_err("transaction data is empty")); } + // Validate transaction size to prevent DoS attacks. + // This matches geth/reth pool validation which rejects transactions > 128 KB. + if bytes.len() > DEFAULT_MAX_TX_INPUT_BYTES { + return Err(crate::err( + jsonrpsee::types::error::INVALID_PARAMS_CODE, + format!( + "oversized data: transaction size {} exceeds limit {DEFAULT_MAX_TX_INPUT_BYTES}", + bytes.len() + ), + None, + )); + } + let transaction: ethereum::TransactionV3 = match ethereum::EnvelopedDecodable::decode(&bytes) { Ok(transaction) => transaction,