fix: bound ABI decoding in Zone transaction paths

Author: 0xalpharush

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]

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();

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
+                ));
+            }
+        }
+    }
 }

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<QueuedDeposit> for DecodedQueuedDeposit {
     type Error = ZonePrecompileError;
 
     fn try_from(queued: QueuedDeposit) -> Result<Self, Self::Error> {
+        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<T>(encoded: &[u8]) -> alloy_sol_types::Result<T>
-where
-    T: SolValue + From<<T::SolType as SolType>::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<QueuedDeposit>) -> ZoneResult<Vec<DecodedQueuedDeposit>> {
     deposits.into_iter().map(TryInto::try_into).collect()
 }

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;

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,