fix: INV4 stack exhaustion and unconditional decoding

Abstracted-Labs · Mar 18, 2024 · a0273e1 · a0273e1
1 parent a708a93
commit a0273e1
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 5 deletions.
diff --git a/INV4/pallet-inv4/Cargo.toml b/INV4/pallet-inv4/Cargo.toml
@@ -11,6 +11,7 @@ version = '0.1.0-dev'
 [dependencies]
 serde = { version = "1.0.132", optional = true }
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false }
+sp-api = { git = "https://github.com/paritytech/substrate", default-features = false , branch = "polkadot-v0.9.43" }
 sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false , branch = "polkadot-v0.9.43" }
 sp-arithmetic = { git = "https://github.com/paritytech/substrate", default-features = false , branch = "polkadot-v0.9.43" }
 sp-std = { git = "https://github.com/paritytech/substrate", default-features = false , branch = "polkadot-v0.9.43" }

diff --git a/INV4/pallet-inv4/src/lib.rs b/INV4/pallet-inv4/src/lib.rs
@@ -61,6 +61,7 @@ pub mod pallet {
     };
 
     use super::*;
+    use codec::FullCodec;
     use frame_support::{
         dispatch::{Dispatchable, GetDispatchInfo, Pays, PostDispatchInfo},
         pallet_prelude::*,
@@ -119,7 +120,7 @@ pub mod pallet {
             > + GetDispatchInfo
             + From<frame_system::Call<Self>>
             + GetCallMetadata
-            + Encode;
+            + FullCodec;
 
         /// The maximum numbers of caller accounts on a single multisig proposal
         #[pallet::constant]

diff --git a/INV4/pallet-inv4/src/multisig.rs b/INV4/pallet-inv4/src/multisig.rs
@@ -17,6 +17,7 @@ use crate::{
     origin::{ensure_multisig, INV4Origin},
     voting::{Tally, Vote},
 };
+use codec::DecodeLimit;
 use core::{
     convert::{TryFrom, TryInto},
     iter::Sum,
@@ -239,12 +240,15 @@ where
             let support = old_data.tally.support(core_id);
             let approval = old_data.tally.approval(core_id);
 
-            // Decode the call
-            let decoded_call = <T as Config>::RuntimeCall::decode(&mut &old_data.actual_call[..])
-                .map_err(|_| Error::<T>::FailedDecodingCall)?;
-
             // Check if the multisig proposal passes the thresholds with the added vote
             if (support >= minimum_support) && (approval >= required_approval) {
+                // Decode the call
+                let decoded_call = <T as Config>::RuntimeCall::decode_all_with_depth_limit(
+                    sp_api::MAX_EXTRINSIC_DEPTH / 4,
+                    &mut &old_data.actual_call[..],
+                )
+                .map_err(|_| Error::<T>::FailedDecodingCall)?;
+
                 // If the proposal thresholds are met, remove proposal from storage
                 *data = None;
 

diff --git a/INV4/pallet-inv4/src/tests/mod.rs b/INV4/pallet-inv4/src/tests/mod.rs
@@ -1274,3 +1274,97 @@ fn core_address_matches() {
 
     assert_eq!(core_account_bytes, ACCOUNT_IN_ASSET_HUB);
 }
+
+// SRLabs tests.
+#[test]
+fn vote_multisig_stack_overflow() {
+    ExtBuilder::default().build().execute_with(|| {
+        INV4::create_core(
+            RawOrigin::Signed(ALICE).into(),
+            vec![].try_into().unwrap(),
+            Perbill::from_percent(100),
+            Perbill::from_percent(100),
+            FeeAsset::Native,
+        )
+        .unwrap();
+
+        System::set_block_number(1);
+
+        let call1: RuntimeCall = pallet::Call::token_mint {
+            amount: CoreSeedBalance::get(),
+            target: BOB,
+        }
+        .into();
+
+        let mut nested_call: RuntimeCall = pallet::Call::operate_multisig {
+            core_id: 0u32,
+            metadata: None,
+            fee_asset: FeeAsset::Native,
+            call: Box::new(call1.clone()),
+        }
+        .into();
+
+        for _ in 0..300 {
+            nested_call = pallet::Call::operate_multisig {
+                core_id: 0u32,
+                metadata: None,
+                fee_asset: FeeAsset::Native,
+                call: Box::new(nested_call.clone()),
+            }
+            .into();
+        }
+
+        INV4::operate_multisig(
+            RawOrigin::Signed(ALICE).into(),
+            0u32,
+            None,
+            FeeAsset::Native,
+            Box::new(call1.clone()),
+        )
+        .unwrap();
+
+        System::set_block_number(2);
+
+        INV4::operate_multisig(
+            RawOrigin::Signed(ALICE).into(),
+            0u32,
+            None,
+            FeeAsset::Native,
+            Box::new(nested_call.clone()),
+        )
+        .unwrap();
+
+        assert_eq!(
+            INV4::multisig(
+                0u32,
+                <<Test as frame_system::Config>::Hashing as Hash>::hash_of(&nested_call)
+            ),
+            Some(MultisigOperation {
+                actual_call: BoundedCallBytes::<Test>::try_from(nested_call.clone().encode())
+                    .unwrap(),
+                fee_asset: FeeAsset::Native,
+                original_caller: ALICE,
+                metadata: None,
+                tally: Tally::from_parts(
+                    CoreSeedBalance::get(),
+                    Zero::zero(),
+                    BoundedBTreeMap::try_from(BTreeMap::from([(
+                        ALICE,
+                        Vote::Aye(CoreSeedBalance::get())
+                    )]))
+                    .unwrap()
+                ),
+            })
+        );
+
+        assert_err!(
+            INV4::vote_multisig(
+                RawOrigin::Signed(BOB).into(),
+                0u32,
+                <<Test as frame_system::Config>::Hashing as Hash>::hash_of(&nested_call),
+                false
+            ),
+            Error::<Test>::FailedDecodingCall
+        );
+    });
+}