From a708a93341a6a4d721ba7d329daa7fdb708b73ce Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Mon, 18 Mar 2024 14:33:41 -0300 Subject: [PATCH 1/3] fix: Changed Rings Wild All to AllCounted --- pallet-rings/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pallet-rings/src/lib.rs b/pallet-rings/src/lib.rs index de0e1d20..1c3a1b50 100644 --- a/pallet-rings/src/lib.rs +++ b/pallet-rings/src/lib.rs @@ -176,7 +176,7 @@ pub mod pallet { }, Instruction::RefundSurplus, Instruction::DepositAsset { - assets: MultiAssetFilter::Wild(WildMultiAsset::All), + assets: MultiAssetFilter::Wild(WildMultiAsset::AllCounted(1)), beneficiary, }, ]); @@ -264,7 +264,7 @@ pub mod pallet { // Refund unused fees Instruction::RefundSurplus, Instruction::DepositAsset { - assets: MultiAssetFilter::Wild(WildMultiAsset::All), + assets: MultiAssetFilter::Wild(WildMultiAsset::AllCounted(1)), beneficiary: core_multilocation, }, ]); @@ -385,19 +385,19 @@ pub mod pallet { weight_limit: WeightLimit::Unlimited, }, Instruction::DepositAsset { - assets: All.into(), + assets: AllCounted(1).into(), beneficiary, }, Instruction::RefundSurplus, Instruction::DepositAsset { - assets: All.into(), + assets: AllCounted(1).into(), beneficiary, }, ]), }, Instruction::RefundSurplus, Instruction::DepositAsset { - assets: All.into(), + assets: AllCounted(1).into(), beneficiary: core_multilocation, }, ]) @@ -419,7 +419,7 @@ pub mod pallet { weight_limit: WeightLimit::Unlimited, }, Instruction::DepositAsset { - assets: MultiAssetFilter::Wild(WildMultiAsset::All), + assets: MultiAssetFilter::Wild(WildMultiAsset::AllCounted(1)), beneficiary, }, ]), @@ -427,7 +427,7 @@ pub mod pallet { // Refund unused fees Instruction::RefundSurplus, Instruction::DepositAsset { - assets: MultiAssetFilter::Wild(WildMultiAsset::All), + assets: MultiAssetFilter::Wild(WildMultiAsset::AllCounted(1)), beneficiary: core_multilocation, }, ]) From a0273e1dea2a96ea1e3e08e51334fabf58103134 Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Mon, 18 Mar 2024 14:37:27 -0300 Subject: [PATCH 2/3] fix: INV4 stack exhaustion and unconditional decoding --- INV4/pallet-inv4/Cargo.toml | 1 + INV4/pallet-inv4/src/lib.rs | 3 +- INV4/pallet-inv4/src/multisig.rs | 12 ++-- INV4/pallet-inv4/src/tests/mod.rs | 94 +++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 5 deletions(-) diff --git a/INV4/pallet-inv4/Cargo.toml b/INV4/pallet-inv4/Cargo.toml index 7e0b40d3..e8fe555e 100644 --- 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 index e87571db..3f20965f 100644 --- 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> + 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 index 1488f5c1..1ef7d575 100644 --- 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 = ::RuntimeCall::decode(&mut &old_data.actual_call[..]) - .map_err(|_| Error::::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 = ::RuntimeCall::decode_all_with_depth_limit( + sp_api::MAX_EXTRINSIC_DEPTH / 4, + &mut &old_data.actual_call[..], + ) + .map_err(|_| Error::::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 index 73c9873b..fdb5f3a2 100644 --- 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, + <::Hashing as Hash>::hash_of(&nested_call) + ), + Some(MultisigOperation { + actual_call: BoundedCallBytes::::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, + <::Hashing as Hash>::hash_of(&nested_call), + false + ), + Error::::FailedDecodingCall + ); + }); +} From b9c8bb79b92b28aa345b8ab5b4d2f9bf80b2d740 Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Mon, 18 Mar 2024 14:55:34 -0300 Subject: [PATCH 3/3] fix: Fix stack exhaustion test --- INV4/pallet-inv4/src/tests/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/INV4/pallet-inv4/src/tests/mod.rs b/INV4/pallet-inv4/src/tests/mod.rs index fdb5f3a2..22e3451d 100644 --- a/INV4/pallet-inv4/src/tests/mod.rs +++ b/INV4/pallet-inv4/src/tests/mod.rs @@ -1304,7 +1304,7 @@ fn vote_multisig_stack_overflow() { } .into(); - for _ in 0..300 { + for _ in 0..(sp_api::MAX_EXTRINSIC_DEPTH / 4) + 1 { nested_call = pallet::Call::operate_multisig { core_id: 0u32, metadata: None, @@ -1362,7 +1362,7 @@ fn vote_multisig_stack_overflow() { RawOrigin::Signed(BOB).into(), 0u32, <::Hashing as Hash>::hash_of(&nested_call), - false + true ), Error::::FailedDecodingCall );