Limit recursion when checking for balance transfers or contract creation

autonomys · Jan 17, 2025 · c8cd63a · c8cd63a
1 parent 8302355
commit c8cd63a
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 12 deletions.
diff --git a/crates/subspace-runtime/src/signed_extensions.rs b/crates/subspace-runtime/src/signed_extensions.rs
@@ -7,6 +7,8 @@ use sp_runtime::transaction_validity::{
 };
 use sp_std::prelude::*;
 
+const MAX_BALANCE_RECURSION_DEPTH: u16 = 5;
+
 /// Disable balance transfers, if configured in the runtime.
 #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, Default, TypeInfo)]
 pub struct DisablePallets;
@@ -30,7 +32,9 @@ impl SignedExtension for DisablePallets {
         _len: usize,
     ) -> TransactionValidity {
         // Disable normal balance transfers.
-        if !RuntimeConfigs::enable_balance_transfers() && contains_balance_transfer(call) {
+        if !RuntimeConfigs::enable_balance_transfers()
+            && contains_balance_transfer(call, MAX_BALANCE_RECURSION_DEPTH)
+        {
             InvalidTransaction::Call.into()
         } else {
             Ok(ValidTransaction::default())
@@ -61,7 +65,14 @@ impl SignedExtension for DisablePallets {
     }
 }
 
-fn contains_balance_transfer(call: &RuntimeCall) -> bool {
+fn contains_balance_transfer(call: &RuntimeCall, mut recursion_depth_left: u16) -> bool {
+    if recursion_depth_left == 0 {
+        // If the recursion depth is reached, we assume the call contains a balance transfer.
+        return true;
+    }
+
+    recursion_depth_left -= 1;
+
     match call {
         RuntimeCall::Balances(
             pallet_balances::Call::transfer_allow_death { .. }
@@ -71,12 +82,14 @@ fn contains_balance_transfer(call: &RuntimeCall) -> bool {
         RuntimeCall::Utility(utility_call) => match utility_call {
             pallet_utility::Call::batch { calls }
             | pallet_utility::Call::batch_all { calls }
-            | pallet_utility::Call::force_batch { calls } => {
-                calls.iter().any(contains_balance_transfer)
-            }
+            | pallet_utility::Call::force_batch { calls } => calls
+                .iter()
+                .any(|call| contains_balance_transfer(call, recursion_depth_left)),
             pallet_utility::Call::as_derivative { call, .. }
             | pallet_utility::Call::dispatch_as { call, .. }
-            | pallet_utility::Call::with_weight { call, .. } => contains_balance_transfer(call),
+            | pallet_utility::Call::with_weight { call, .. } => {
+                contains_balance_transfer(call, recursion_depth_left)
+            }
             pallet_utility::Call::__Ignore(..) => false,
         },
         _ => false,

diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs
@@ -148,12 +148,15 @@ pub type Executive = domain_pallet_executive::Executive<
     AllPalletsWithSystem,
 >;
 
+const MAX_CONTRACT_RECURSION_DEPTH: u16 = 5;
+
 /// Rejects contracts that can't be created under the current allow list.
 /// Returns false if the call is a contract call, and the account is *not* allowed to call it.
 /// Otherwise, returns true.
 pub fn is_create_contract_allowed(call: &RuntimeCall, signer: &AccountId) -> bool {
-    if is_create_contract(call)
-        && !pallet_evm_nonce_tracker::Pallet::<Runtime>::is_allowed_to_create_contracts(signer)
+    // Only enter recursive code if this account can't create contracts
+    if !pallet_evm_nonce_tracker::Pallet::<Runtime>::is_allowed_to_create_contracts(signer)
+        && is_create_contract(call, MAX_CONTRACT_RECURSION_DEPTH)
     {
         return false;
     }
@@ -163,7 +166,14 @@ pub fn is_create_contract_allowed(call: &RuntimeCall, signer: &AccountId) -> boo
 }
 
 /// Returns true if the call is a contract creation call.
-pub fn is_create_contract(call: &RuntimeCall) -> bool {
+pub fn is_create_contract(call: &RuntimeCall, mut recursion_depth_left: u16) -> bool {
+    if recursion_depth_left == 0 {
+        // If the recursion depth is reached, we assume the call contains a contract.
+        return true;
+    }
+
+    recursion_depth_left -= 1;
+
     match call {
         RuntimeCall::EVM(pallet_evm::Call::create { .. })
         | RuntimeCall::EVM(pallet_evm::Call::create2 { .. }) => true,
@@ -179,14 +189,17 @@ pub fn is_create_contract(call: &RuntimeCall) -> bool {
             transaction: EthereumTransaction::EIP1559(transaction),
             ..
         }) => transaction.action == TransactionAction::Create,
-        // TODO: does this need a recursion limit?
         RuntimeCall::Utility(utility_call) => match utility_call {
             pallet_utility::Call::batch { calls }
             | pallet_utility::Call::batch_all { calls }
-            | pallet_utility::Call::force_batch { calls } => calls.iter().any(is_create_contract),
+            | pallet_utility::Call::force_batch { calls } => calls
+                .iter()
+                .any(|call| is_create_contract(call, recursion_depth_left)),
             pallet_utility::Call::as_derivative { call, .. }
             | pallet_utility::Call::dispatch_as { call, .. }
-            | pallet_utility::Call::with_weight { call, .. } => is_create_contract(call),
+            | pallet_utility::Call::with_weight { call, .. } => {
+                is_create_contract(call, recursion_depth_left)
+            }
             pallet_utility::Call::__Ignore(..) => false,
         },
         _ => false,