From 6482149e449920c0cb8524062f87fba90970b3da Mon Sep 17 00:00:00 2001 From: 0xalpharush <0xalpharush@protonmail.com> Date: Thu, 3 Sep 2026 11:20:15 -0500 Subject: [PATCH] fix: bound ABI decoding in Zone transaction paths --- crates/evm/src/executor.rs | 83 ++++++++++++++++++---- crates/node/src/replication.rs | 12 +++- crates/precompiles/src/account_keychain.rs | 43 ++++++++++- crates/precompiles/src/inbox/mod.rs | 24 +++---- crates/precompiles/src/lib.rs | 2 +- crates/precompiles/src/outbox/mod.rs | 9 --- 6 files changed, 133 insertions(+), 40 deletions(-) diff --git a/crates/evm/src/executor.rs b/crates/evm/src/executor.rs index b46d37b4b..854e05fe0 100644 --- a/crates/evm/src/executor.rs +++ b/crates/evm/src/executor.rs @@ -13,7 +13,7 @@ use alloy_evm::{ }, eth::{EthBlockExecutor, EthTxResult}, }; -use alloy_sol_types::SolEvent as _; +use alloy_sol_types::{SolCall as _, SolEvent as _}; use reth_evm::block::StateDB; use reth_revm::{Inspector, context::result::ResultAndState}; use tempo_evm::{TempoBlockExecutionCtx, TempoReceiptBuilder}; @@ -22,9 +22,7 @@ use tempo_revm::evm::TempoContext; use tempo_zone_contracts::IZoneOutbox; use zone_chainspec::ZoneChainSpec; use zone_l1::state::L1StateProvider; -use zone_precompiles::{ - ADVANCE_TEMPO_SELECTOR, L1StorageReader, is_finalize_withdrawal_batch_calldata, -}; +use zone_precompiles::{ADVANCE_TEMPO_SELECTOR, L1StorageReader}; use zone_primitives::constants::{ZONE_INBOX_ADDRESS, ZONE_OUTBOX_ADDRESS}; use crate::{L1OverlayDB, ZoneEvm}; @@ -106,7 +104,8 @@ impl ZoneTransactionKind { } if tx.calls().any(|(kind, input)| { - kind.to() == Some(&ZONE_OUTBOX_ADDRESS) && is_finalize_withdrawal_batch_calldata(input) + kind.to() == Some(&ZONE_OUTBOX_ADDRESS) + && input.starts_with(&IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR) }) { return Self::FinalizeWithdrawalBatch; } @@ -285,7 +284,7 @@ mod tests { use reth_chainspec::EthChainSpec as _; use reth_primitives_traits::Recovered; use revm::database::{CacheDB, EmptyDB}; - use tempo_chainspec::spec::DEV; + use tempo_chainspec::{hardfork::TempoHardfork, spec::DEV}; use tempo_evm::TempoBlockExecutionCtx; use tempo_precompiles::{ DEFAULT_FEE_TOKEN, TIP_FEE_MANAGER_ADDRESS, @@ -461,14 +460,74 @@ mod tests { ZONE_OUTBOX_ADDRESS, Bytes::copy_from_slice(&IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR), ); - let error = ZoneBlockPhase::Executing - .validate_transaction(&malformed_finalize) - .unwrap_err(); assert_eq!( - error.to_string(), - "system transactions after advanceTempo must call \ - ZoneOutbox.finalizeWithdrawalBatch" + ZoneBlockPhase::Executing + .validate_transaction(&malformed_finalize) + .unwrap(), + ZoneBlockPhase::WithdrawalsFinalized + ); + + let mut trailing_calldata = IZoneOutbox::finalizeWithdrawalBatchCall { + count: U256::ZERO, + blockNumber: 1, + encryptedSenders: vec![], + } + .abi_encode(); + trailing_calldata.extend_from_slice(&[0; 32]); + let trailing_finalize = system_tx(ZONE_OUTBOX_ADDRESS, trailing_calldata.into()); + assert_eq!( + ZoneBlockPhase::Executing + .validate_transaction(&trailing_finalize) + .unwrap(), + ZoneBlockPhase::WithdrawalsFinalized + ); + } + + #[test] + fn malformed_t11_finalization_does_not_advance_block_phase() { + let mut zone_genesis = DEV.genesis().clone(); + zone_genesis.config.chain_id = zone_chain_id(DEV.chain().id(), 2).unwrap(); + let chain_spec = std::sync::Arc::new(ZoneChainSpec::from_genesis(zone_genesis).unwrap()); + let factory = + ZoneEvmFactory::new(chain_spec.clone(), MockL1Reader::default(), Address::ZERO); + let mut env = EvmEnv::default(); + env.cfg_env.spec = TempoHardfork::T11; + let evm = factory.create_evm(CacheDB::new(EmptyDB::default()), env); + let ctx = TempoBlockExecutionCtx { + inner: EthBlockExecutionCtx { + parent_hash: B256::ZERO, + parent_beacon_block_root: None, + ommers: &[], + withdrawals: None, + extra_data: Bytes::new(), + tx_count_hint: Some(1), + slot_number: None, + }, + general_gas_limit: 0, + shared_gas_limit: 0, + validator_set: None, + consensus_context: None, + subblock_fee_recipients: Default::default(), + }; + let mut executor = ZoneBlockExecutor::new(evm, ctx, &chain_spec); + executor.phase = ZoneBlockPhase::Executing; + + let tx = Recovered::new_unchecked( + system_tx( + ZONE_OUTBOX_ADDRESS, + Bytes::copy_from_slice(&IZoneOutbox::finalizeWithdrawalBatchCall::SELECTOR), + ), + TEMPO_SYSTEM_TX_SENDER, + ); + let error = executor.execute_transaction_without_commit(tx).unwrap_err(); + + assert!( + error + .to_string() + .contains("system transaction execution failed"), + "unexpected error: {error}" ); + assert_eq!(executor.phase, ZoneBlockPhase::Executing); } #[test] diff --git a/crates/node/src/replication.rs b/crates/node/src/replication.rs index a1f8baa22..4a8f984c7 100644 --- a/crates/node/src/replication.rs +++ b/crates/node/src/replication.rs @@ -6,7 +6,7 @@ use alloy_primitives::B256; use alloy_provider::DynProvider; use alloy_rlp::Decodable as _; use alloy_rpc_types_engine::ForkchoiceState; -use alloy_sol_types::SolCall as _; +use alloy_sol_types::{SolCall as _, abi::AbiDecoderConfig}; use futures::{StreamExt as _, stream::BoxStream}; use reth_chain_state::PersistedBlockSubscriptions; use reth_node_api::{ConsensusEngineHandle, PayloadTypes as _}; @@ -18,6 +18,7 @@ use std::{ time::Duration, }; use tempo_alloy::TempoNetwork; +use tempo_precompiles::dispatch::ABI_DECODER_MEMORY_LIMIT; use tempo_primitives::{Block, TempoHeader, TempoTxEnvelope}; use tokio::sync::{mpsc, oneshot}; use tokio_util::sync; @@ -1375,8 +1376,13 @@ fn decode_advance_tempo( if signed.tx().to != ZONE_INBOX_ADDRESS.into() { eyre::bail!("first Tempo system transaction is not sent to IZoneInbox") } - let call = IZoneInbox::advanceTempoCall::abi_decode(signed.tx().input.as_ref()) - .map_err(|err| eyre::eyre!("first transaction does not decode as advanceTempo: {err}"))?; + let call = IZoneInbox::advanceTempoCall::abi_decode_with_config( + signed.tx().input.as_ref(), + AbiDecoderConfig::new() + .memory_limit(ABI_DECODER_MEMORY_LIMIT) + .strict(true), + ) + .map_err(|err| eyre::eyre!("first transaction does not decode as advanceTempo: {err}"))?; // 3. the system tx is valid. let mut header_rlp = call.header.as_ref(); diff --git a/crates/precompiles/src/account_keychain.rs b/crates/precompiles/src/account_keychain.rs index 8457b6857..89455d7f4 100644 --- a/crates/precompiles/src/account_keychain.rs +++ b/crates/precompiles/src/account_keychain.rs @@ -1,7 +1,7 @@ //! Zone read-privacy rules for the upstream Tempo AccountKeychain precompile. use alloy_primitives::Address; -use alloy_sol_types::SolInterface; +use alloy_sol_types::{SolCall, SolInterface}; use tempo_contracts::precompiles::IAccountKeychain; use tempo_precompiles::dispatch::abi_decoder_config_for_spec; @@ -15,11 +15,36 @@ use crate::{ #[derive(Clone)] pub(crate) struct AccountKeychainRules; +const UNRESTRICTED_SELECTORS: &[[u8; 4]] = &[ + IAccountKeychain::authorizeKey_0Call::SELECTOR, + IAccountKeychain::authorizeKey_1Call::SELECTOR, + IAccountKeychain::authorizeKey_2Call::SELECTOR, + IAccountKeychain::authorizeAdminKeyCall::SELECTOR, + IAccountKeychain::burnKeyAuthorizationWitnessCall::SELECTOR, + IAccountKeychain::revokeKeyCall::SELECTOR, + IAccountKeychain::updateSpendingLimitCall::SELECTOR, + IAccountKeychain::setAllowedCallsCall::SELECTOR, + IAccountKeychain::removeAllowedCallsCall::SELECTOR, + IAccountKeychain::getTransactionKeyCall::SELECTOR, +]; + impl CallRules for AccountKeychainRules { fn admit(&self, data: &[u8], caller: Address) -> CallCheck { + let spec = StorageCtx::default().spec(); + + // These calls have no Zone-specific privacy policy. Defer directly to the upstream + // dispatcher for selector scheduling and ABI decoding. + if data.get(..4).is_some_and(|selector| { + UNRESTRICTED_SELECTORS + .iter() + .any(|allowed| selector == allowed.as_slice()) + }) { + return CallCheck::Continue; + } + let Ok(call) = IAccountKeychain::IAccountKeychainCalls::abi_decode_with_config( data, - abi_decoder_config_for_spec(StorageCtx::default().spec()), + abi_decoder_config_for_spec(spec), ) else { // Preserve the upstream error and gas behavior for malformed or unknown calldata. return CallCheck::Continue; @@ -229,4 +254,18 @@ mod tests { CallCheck::Continue )); } + + #[test] + fn malformed_unrestricted_calls_remain_deferred_to_upstream() { + let rules = AccountKeychainRules; + + for data in UNRESTRICTED_SELECTORS { + for spec in [TempoHardfork::T10, TempoHardfork::T11] { + assert!(matches!( + admit_at(&rules, data, Address::ZERO, spec), + CallCheck::Continue + )); + } + } + } } diff --git a/crates/precompiles/src/inbox/mod.rs b/crates/precompiles/src/inbox/mod.rs index 992c382d8..e688aa205 100644 --- a/crates/precompiles/src/inbox/mod.rs +++ b/crates/precompiles/src/inbox/mod.rs @@ -20,9 +20,10 @@ use alloc::vec::Vec; use alloy_evm::precompiles::DynPrecompile; use alloy_primitives::{Address, B256, U256}; -use alloy_sol_types::{SolCall, SolType, SolValue}; +use alloy_sol_types::{SolCall, SolValue, abi::AbiDecoderConfig}; use tempo_precompiles::{ PATH_USD_ADDRESS, + dispatch::ABI_DECODER_MEMORY_LIMIT, error::TempoPrecompileError, storage::{Handler, Mapping, Slot, StorageCtx}, tip20::{ISSUER_ROLE, ITIP20, TIP20Error, TIP20Token}, @@ -378,27 +379,24 @@ impl TryFrom for DecodedQueuedDeposit { type Error = ZonePrecompileError; fn try_from(queued: QueuedDeposit) -> Result { + let config = AbiDecoderConfig::new() + .memory_limit(ABI_DECODER_MEMORY_LIMIT) + .strict(true); + match queued.depositType { DepositType::WithdrawalBounceBack => { - decode_canonical(&queued.depositData).map(Self::WithdrawalBounceBack) + WithdrawalBounceBackDeposit::abi_decode_with_config(&queued.depositData, config) + .map(Self::WithdrawalBounceBack) + } + DepositType::Deposit => { + Deposit::abi_decode_with_config(&queued.depositData, config).map(Self::Deposit) } - DepositType::Deposit => decode_canonical(&queued.depositData).map(Self::Deposit), _ => return Err(ZonePrecompileError::MalformedCalldata), } .map_err(|_| ZonePrecompileError::MalformedCalldata) } } -fn decode_canonical(encoded: &[u8]) -> alloy_sol_types::Result -where - T: SolValue + From<::RustType>, -{ - let value = T::abi_decode(encoded)?; - (value.abi_encode().as_slice() == encoded) - .then_some(value) - .ok_or(alloy_sol_types::Error::ReserMismatch) -} - fn decode_deposits(deposits: Vec) -> ZoneResult> { deposits.into_iter().map(TryInto::try_into).collect() } diff --git a/crates/precompiles/src/lib.rs b/crates/precompiles/src/lib.rs index e0a4ae50e..38a621f4a 100644 --- a/crates/precompiles/src/lib.rs +++ b/crates/precompiles/src/lib.rs @@ -84,7 +84,7 @@ pub mod ztip20; pub use aes_gcm::AesGcmDecrypt; pub use chaum_pedersen::ChaumPedersenVerify; pub use inbox::{ADVANCE_TEMPO_SELECTOR, ZoneInbox}; -pub use outbox::{ZoneOutbox, is_finalize_withdrawal_batch_calldata}; +pub use outbox::ZoneOutbox; pub use storage::{L1State, L1StateError, L1StorageReader}; pub use tempo_contracts::precompiles::TIP403_REGISTRY_ADDRESS; pub use tempo_state::TempoState; diff --git a/crates/precompiles/src/outbox/mod.rs b/crates/precompiles/src/outbox/mod.rs index e393c3f34..f38447f42 100644 --- a/crates/precompiles/src/outbox/mod.rs +++ b/crates/precompiles/src/outbox/mod.rs @@ -7,7 +7,6 @@ mod tests; use alloc::vec::Vec; use alloy_primitives::{Address, B256, Bytes, U256}; -use alloy_sol_types::SolCall; use tempo_precompiles::{ Result as TempoResult, error::TempoPrecompileError, @@ -32,14 +31,6 @@ use crate::{ pub const MAX_CALLBACK_DATA_SIZE: usize = 1024; const WITHDRAWAL_BASE_GAS: u64 = 50_000; -/// Returns whether `calldata` is a canonical `finalizeWithdrawalBatch` call. -pub fn is_finalize_withdrawal_batch_calldata(calldata: &[u8]) -> bool { - let Ok(call) = IZoneOutbox::finalizeWithdrawalBatchCall::abi_decode(calldata) else { - return false; - }; - call.abi_encode() == calldata -} - #[contract(addr = ZONE_OUTBOX_ADDRESS)] pub struct ZoneOutbox { tempo_gas_rate: u128,