Unverified Commit 659911f6 authored by Shawn Tabrizi's avatar Shawn Tabrizi Committed by GitHub
Browse files

Harden XCM v1 for Recursions (#3586)



* Guard against XCM recursive bombs by setting a recursion limit

* Add test and set a lower recursion limit

* Use u32 instead of usize for recursion limit

* Make spellcheck happy

* Cargo fmt

* Limit XCM decoding depth in UMP message processing

* Modify test to check for recursion in BuyExecution

* Update xcm/xcm-simulator/example/src/lib.rs
Co-authored-by: default avatarKian Paimani <5588131+kianenigma@users.noreply.github.com>

* Make cargo fmt happy

* WIP for testing recursion limit in WASM

* Revert "WIP for testing recursion limit in WASM"

This reverts commit 39181b46

.

* Remove XCM recursion limit test

* Add recursion test for XCM message execution

* Set a more sensible recursion limit

* Cargo fmt

* Implement successful_origin for benchmarks

* Set recursion limit to 8 and create integration tests directory for xcm-executor

* Cargo fmt

* Add runtime-benchmarks feature to test-runtime

* Give up creating ConvertOriginToLocal and use EnsureXcm

* Re-add ConvertOriginToLocal

* Fix compilation

* Update xcm/xcm-executor/src/lib.rs
Co-authored-by: default avatarGavin Wood <gavin@parity.io>

* Add decoding limit to all versioned XCM decode calls

* Fix recursion limit test

* Set a lower recursion count for recursion test

* move integration tests to their own folder, fix recursion check in execute_effects

* Remove xcm-executor integration tests directory

* fix up

* Update Cargo.lock

* Update runtime/parachains/src/ump.rs

* use proper decode limit

* fix decode depth limit

* here too

* Update traits.rs

* fix compile

* fix test

* Revert `decode_all_with_depth_limit` changes in parachain.rs

* Remove unused imports in parachain.rs
Co-authored-by: Keith Yeung's avatarKeith Yeung <kungfukeith11@gmail.com>
Co-authored-by: default avatarKian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: default avatarBastian Köcher <info@kchr.de>
Co-authored-by: default avatarGavin Wood <gavin@parity.io>
parent 26aee2b2
Pipeline #151759 canceled with stages
in 7 minutes and 35 seconds
......@@ -7216,6 +7216,7 @@ dependencies = [
"pallet-transaction-payment",
"pallet-transaction-payment-rpc-runtime-api",
"pallet-vesting",
"pallet-xcm",
"parity-scale-codec",
"polkadot-parachain",
"polkadot-primitives",
......@@ -7244,6 +7245,9 @@ dependencies = [
"sp-version",
"substrate-wasm-builder",
"tiny-keccak",
"xcm",
"xcm-builder",
"xcm-executor",
]
[[package]]
......@@ -12303,6 +12307,26 @@ dependencies = [
"xcm",
]
[[package]]
name = "xcm-executor-integration-tests"
version = "0.9.9"
dependencies = [
"frame-support",
"frame-system",
"futures 0.3.16",
"pallet-xcm",
"polkadot-test-client",
"polkadot-test-runtime",
"polkadot-test-service",
"sp-consensus",
"sp-keyring",
"sp-runtime",
"sp-state-machine",
"sp-tracing",
"xcm",
"xcm-executor",
]
[[package]]
name = "xcm-simulator"
version = "0.9.9"
......
......@@ -39,6 +39,7 @@ members = [
"xcm",
"xcm/xcm-builder",
"xcm/xcm-executor",
"xcm/xcm-executor/integration-tests",
"xcm/xcm-simulator",
"xcm/xcm-simulator/example",
"xcm/pallet-xcm",
......
......@@ -84,14 +84,18 @@ impl<XcmExecutor: xcm::latest::ExecuteXcm<C::Call>, C: Config> UmpSink for XcmSi
data: &[u8],
max_weight: Weight,
) -> Result<Weight, (MessageId, Weight)> {
use parity_scale_codec::DecodeLimit;
use xcm::{
latest::{Error as XcmError, Junction, MultiLocation, Xcm},
VersionedXcm,
};
let id = sp_io::hashing::blake2_256(&data[..]);
let maybe_msg =
VersionedXcm::<C::Call>::decode(&mut &data[..]).map(Xcm::<C::Call>::try_from);
let maybe_msg = VersionedXcm::<C::Call>::decode_all_with_depth_limit(
xcm::MAX_XCM_DECODE_DEPTH,
&mut &data[..],
)
.map(Xcm::<C::Call>::try_from);
match maybe_msg {
Err(_) => {
Pallet::<C>::deposit_event(Event::InvalidFormat(id));
......
......@@ -55,8 +55,12 @@ pallet-vesting = { git = "https://github.com/paritytech/substrate", branch = "ma
runtime-common = { package = "polkadot-runtime-common", path = "../common", default-features = false }
primitives = { package = "polkadot-primitives", path = "../../primitives", default-features = false }
pallet-xcm = { path = "../../xcm/pallet-xcm", default-features = false }
polkadot-parachain = { path = "../../parachain", default-features = false }
polkadot-runtime-parachains = { path = "../parachains", default-features = false }
xcm-builder = { path = "../../xcm/xcm-builder", default-features = false }
xcm-executor = { path = "../../xcm/xcm-executor", default-features = false }
xcm = { path = "../../xcm", default-features = false }
[dev-dependencies]
hex-literal = "0.3.3"
......@@ -82,6 +86,10 @@ std = [
"inherents/std",
"sp-core/std",
"polkadot-parachain/std",
"pallet-xcm/std",
"xcm-builder/std",
"xcm-executor/std",
"xcm/std",
"sp-api/std",
"tx-pool-api/std",
"block-builder-api/std",
......
......@@ -79,6 +79,7 @@ pub use sp_runtime::BuildStorage;
/// Constant values used within the runtime.
pub mod constants;
pub mod xcm_config;
use constants::{currency::*, fee::*, time::*};
// Make the WASM binary available.
......@@ -488,6 +489,31 @@ impl parachains_ump::Config for Runtime {
type FirstMessageFactorPercent = FirstMessageFactorPercent;
}
parameter_types! {
pub const BaseXcmWeight: frame_support::weights::Weight = 1_000;
pub const AnyNetwork: xcm::latest::NetworkId = xcm::latest::NetworkId::Any;
}
pub type LocalOriginToLocation = xcm_builder::SignedToAccountId32<Origin, AccountId, AnyNetwork>;
impl pallet_xcm::Config for Runtime {
// The config types here are entirely configurable, since the only one that is sorely needed
// is `XcmExecutor`, which will be used in unit tests located in xcm-executor.
type Event = Event;
type ExecuteXcmOrigin = xcm_builder::EnsureXcmOrigin<Origin, LocalOriginToLocation>;
type LocationInverter = xcm_config::InvertNothing;
type SendXcmOrigin = xcm_builder::EnsureXcmOrigin<Origin, LocalOriginToLocation>;
type Weigher = xcm_builder::FixedWeightBounds<BaseXcmWeight, Call>;
type XcmRouter = xcm_config::DoNothingRouter;
type XcmExecuteFilter =
frame_support::traits::All<(xcm::latest::MultiLocation, xcm::latest::Xcm<Call>)>;
type XcmExecutor = xcm_executor::XcmExecutor<xcm_config::XcmConfig>;
type XcmTeleportFilter =
frame_support::traits::All<(xcm::latest::MultiLocation, Vec<xcm::latest::MultiAsset>)>;
type XcmReserveTransferFilter =
frame_support::traits::All<(xcm::latest::MultiLocation, Vec<xcm::latest::MultiAsset>)>;
}
impl parachains_hrmp::Config for Runtime {
type Event = Event;
type Origin = Origin;
......@@ -543,6 +569,7 @@ construct_runtime! {
Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event<T>},
Ump: parachains_ump::{Pallet, Call, Storage, Event},
Dmp: parachains_dmp::{Pallet, Call, Storage},
Xcm: pallet_xcm::{Pallet, Call, Event<T>, Origin},
ParasDisputes: parachains_disputes::{Pallet, Storage, Event<T>},
Sudo: pallet_sudo::{Pallet, Call, Storage, Config<T>, Event<T>},
......
// Copyright 2021 Parity Technologies (UK) Ltd.
// This file is part of Polkadot.
// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
use frame_support::{parameter_types, traits::All, weights::Weight};
use xcm::latest::{
Error as XcmError, Junction::*, MultiAsset, MultiLocation, MultiLocation::*, NetworkId,
Result as XcmResult, SendXcm, Xcm,
};
use xcm_builder::{AllowUnpaidExecutionFrom, FixedWeightBounds, SignedToAccountId32};
use xcm_executor::{
traits::{InvertLocation, TransactAsset, WeightTrader},
Assets,
};
parameter_types! {
pub const OurNetwork: NetworkId = NetworkId::Polkadot;
}
/// Type to convert an `Origin` type value into a `MultiLocation` value which represents an interior location
/// of this chain.
pub type LocalOriginToLocation = (
// And a usual Signed origin to be used in XCM as a corresponding AccountId32
SignedToAccountId32<crate::Origin, crate::AccountId, OurNetwork>,
);
pub struct DoNothingRouter;
impl SendXcm for DoNothingRouter {
fn send_xcm(_dest: MultiLocation, _msg: Xcm<()>) -> XcmResult {
Ok(())
}
}
pub type Barrier = AllowUnpaidExecutionFrom<All<MultiLocation>>;
pub struct DummyAssetTransactor;
impl TransactAsset for DummyAssetTransactor {
fn deposit_asset(_what: &MultiAsset, _who: &MultiLocation) -> XcmResult {
Ok(())
}
fn withdraw_asset(_what: &MultiAsset, _who: &MultiLocation) -> Result<Assets, XcmError> {
let asset: MultiAsset = (X1(Parent), 100_000).into();
Ok(asset.into())
}
}
pub struct DummyWeightTrader;
impl WeightTrader for DummyWeightTrader {
fn new() -> Self {
DummyWeightTrader
}
fn buy_weight(&mut self, _weight: Weight, _payment: Assets) -> Result<Assets, XcmError> {
Ok(Assets::default())
}
}
pub struct InvertNothing;
impl InvertLocation for InvertNothing {
fn invert_location(_: &MultiLocation) -> MultiLocation {
MultiLocation::Here
}
}
pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type Call = super::Call;
type XcmSender = DoNothingRouter;
type AssetTransactor = DummyAssetTransactor;
type OriginConverter = pallet_xcm::XcmPassthrough<super::Origin>;
type IsReserve = ();
type IsTeleporter = ();
type LocationInverter = InvertNothing;
type Barrier = Barrier;
type Weigher = FixedWeightBounds<super::BaseXcmWeight, super::Call>;
type Trader = DummyWeightTrader;
type ResponseHandler = ();
}
......@@ -14,12 +14,10 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
use crate::MAX_XCM_DECODE_DEPTH;
use alloc::vec::Vec;
use parity_scale_codec::{Decode, DecodeLimit, Encode};
/// Maximum nesting level for XCM decoding.
pub const MAX_XCM_DECODE_DEPTH: u32 = 8;
/// Wrapper around the encoded and decoded versions of a value.
/// Caches the decoded value once computed.
#[derive(Encode, Decode)]
......
......@@ -40,6 +40,9 @@ pub mod latest {
mod double_encoded;
pub use double_encoded::DoubleEncoded;
/// Maximum nesting level for XCM decoding.
pub const MAX_XCM_DECODE_DEPTH: u32 = 8;
#[derive(Clone, Eq, PartialEq, Debug)]
pub enum Unsupported {}
impl Encode for Unsupported {}
......
......@@ -87,6 +87,8 @@ pub enum Error {
TooExpensive,
/// The given asset is not handled.
AssetNotFound,
/// `execute_xcm` has been called too many times recursively.
RecursionLimitReached,
}
impl From<()> for Error {
......
......@@ -89,6 +89,8 @@ pub enum Error {
AssetNotFound,
/// The given message cannot be translated into a format that the destination can be expected to interpret.
DestinationUnsupported,
/// `execute_xcm` has been called too many times recursively.
RecursionLimitReached,
}
impl From<()> for Error {
......
[package]
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2018"
name = "xcm-executor-integration-tests"
description = "Integration tests for the XCM Executor"
version = "0.9.9"
[dependencies]
frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
frame-system = { git = "https://github.com/paritytech/substrate", branch = "master" }
futures = "0.3.15"
pallet-xcm = { path = "../../pallet-xcm" }
polkadot-test-client = { path = "../../../node/test/client" }
polkadot-test-runtime = { path = "../../../runtime/test-runtime" }
polkadot-test-service = { path = "../../../node/test/service" }
sp-consensus = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-state-machine = { git = "https://github.com/paritytech/substrate", branch = "master" }
xcm = { path = "../..", default-features = false }
xcm-executor = { path = ".." }
sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }
[features]
default = ["std"]
std = [
"xcm/std",
"sp-runtime/std",
"frame-support/std",
]
// Copyright 2021 Parity Technologies (UK) Ltd.
// This file is part of Polkadot.
// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg(test)]
use polkadot_test_client::{
BlockBuilderExt, ClientBlockImportExt, DefaultTestClientBuilderExt, ExecutionStrategy,
InitPolkadotBlockBuilder, TestClientBuilder, TestClientBuilderExt,
};
use polkadot_test_service::construct_extrinsic;
use sp_runtime::{generic::BlockId, traits::Block};
use sp_state_machine::InspectState;
use xcm::latest::{Error as XcmError, Junction::*, MultiLocation::*, Order, Outcome, Xcm::*};
use xcm_executor::MAX_RECURSION_LIMIT;
// This is the inflection point where the test should either fail or pass.
const MAX_RECURSION_CHECK: u32 = MAX_RECURSION_LIMIT / 2;
#[test]
fn execute_within_recursion_limit() {
sp_tracing::try_init_simple();
let mut client = TestClientBuilder::new()
.set_execution_strategy(ExecutionStrategy::AlwaysWasm)
.build();
let mut msg = WithdrawAsset { assets: (X1(Parent), 100).into(), effects: vec![] };
for _ in 0..MAX_RECURSION_CHECK {
msg = WithdrawAsset {
assets: (X1(Parent), 100).into(),
effects: vec![Order::BuyExecution {
fees: (X1(Parent), 1).into(),
weight: 0,
debt: 0,
halt_on_error: true,
orders: vec![],
// nest `msg` into itself on each iteration.
instructions: vec![msg],
}],
};
}
let mut block_builder = client.init_polkadot_block_builder();
let execute = construct_extrinsic(
&client,
polkadot_test_runtime::Call::Xcm(pallet_xcm::Call::execute(
Box::new(msg.clone()),
1_000_000_000,
)),
sp_keyring::Sr25519Keyring::Alice,
);
block_builder.push_polkadot_extrinsic(execute).expect("pushes extrinsic");
let block = block_builder.build().expect("Finalizes the block").block;
let block_hash = block.hash();
futures::executor::block_on(client.import(sp_consensus::BlockOrigin::Own, block))
.expect("imports the block");
client
.state_at(&BlockId::Hash(block_hash))
.expect("state should exist")
.inspect_state(|| {
assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!(
r.event,
polkadot_test_runtime::Event::Xcm(pallet_xcm::Event::Attempted(Outcome::Complete(
_
)),),
)));
});
}
#[test]
fn exceed_recursion_limit() {
sp_tracing::try_init_simple();
let mut client = TestClientBuilder::new()
.set_execution_strategy(ExecutionStrategy::AlwaysWasm)
.build();
let mut msg = WithdrawAsset { assets: (X1(Parent), 100).into(), effects: vec![] };
for _ in 0..(MAX_RECURSION_CHECK + 1) {
msg = WithdrawAsset {
assets: (X1(Parent), 100).into(),
effects: vec![Order::BuyExecution {
fees: (X1(Parent), 1).into(),
weight: 0,
debt: 0,
halt_on_error: true,
orders: vec![],
// nest `msg` into itself on each iteration.
instructions: vec![msg],
}],
};
}
let mut block_builder = client.init_polkadot_block_builder();
let execute = construct_extrinsic(
&client,
polkadot_test_runtime::Call::Xcm(pallet_xcm::Call::execute(
Box::new(msg.clone()),
1_000_000_000,
)),
sp_keyring::Sr25519Keyring::Alice,
);
block_builder.push_polkadot_extrinsic(execute).expect("pushes extrinsic");
let block = block_builder.build().expect("Finalizes the block").block;
let block_hash = block.hash();
futures::executor::block_on(client.import(sp_consensus::BlockOrigin::Own, block))
.expect("imports the block");
client
.state_at(&BlockId::Hash(block_hash))
.expect("state should exist")
.inspect_state(|| {
assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!(
r.event,
polkadot_test_runtime::Event::Xcm(pallet_xcm::Event::Attempted(
Outcome::Incomplete(_, XcmError::RecursionLimitReached),
)),
)));
});
}
......@@ -41,6 +41,9 @@ pub use config::Config;
/// The XCM executor.
pub struct XcmExecutor<Config>(PhantomData<Config>);
/// The maximum recursion limit for `execute_xcm` and `execute_effects`.
pub const MAX_RECURSION_LIMIT: u32 = 8;
impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
fn execute_xcm_in_credit(
origin: MultiLocation,
......@@ -56,7 +59,6 @@ impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
weight_limit,
weight_credit,
);
// TODO: #2841 #HARDENXCM We should identify recursive bombs here and bail.
let mut message = Xcm::<Config::Call>::from(message);
let shallow_weight = match Config::Weigher::shallow(&mut message) {
Ok(x) => x,
......@@ -81,6 +83,7 @@ impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
&mut weight_credit,
Some(shallow_weight),
&mut trader,
0,
);
drop(trader);
log::trace!(target: "xcm::execute_xcm", "result: {:?}", &result);
......@@ -110,16 +113,23 @@ impl<Config: config::Config> XcmExecutor<Config> {
weight_credit: &mut Weight,
maybe_shallow_weight: Option<Weight>,
trader: &mut Config::Trader,
num_recursions: u32,
) -> Result<Weight, XcmError> {
log::trace!(
target: "xcm::do_execute_xcm",
"origin: {:?}, top_level: {:?}, message: {:?}, weight_credit: {:?}, maybe_shallow_weight: {:?}",
"origin: {:?}, top_level: {:?}, message: {:?}, weight_credit: {:?}, maybe_shallow_weight: {:?}, recursion: {:?}",
origin,
top_level,
message,
weight_credit,
maybe_shallow_weight,
num_recursions,
);
if num_recursions > MAX_RECURSION_LIMIT {
return Err(XcmError::RecursionLimitReached)
}
// This is the weight of everything that cannot be paid for. This basically means all computation
// except any XCM which is behind an Order::BuyExecution.
let shallow_weight = maybe_shallow_weight
......@@ -237,8 +247,15 @@ impl<Config: config::Config> XcmExecutor<Config> {
ensure!(who.is_interior(), XcmError::EscalationOfPrivilege);
let mut origin = origin;
origin.append_with(who).map_err(|_| XcmError::MultiLocationFull)?;
let surplus =
Self::do_execute_xcm(origin, top_level, *message, weight_credit, None, trader)?;
let surplus = Self::do_execute_xcm(
origin,
top_level,
*message,
weight_credit,
None,
trader,
num_recursions + 1,
)?;
total_surplus = total_surplus.saturating_add(surplus);
None
},
......@@ -247,7 +264,13 @@ impl<Config: config::Config> XcmExecutor<Config> {
if let Some((mut holding, effects)) = maybe_holding_effects {
for effect in effects.into_iter() {
total_surplus += Self::execute_orders(&origin, &mut holding, effect, trader)?;
total_surplus += Self::execute_orders(
&origin,
&mut holding,
effect,
trader,
num_recursions + 1,
)?;
}
}
......@@ -259,14 +282,21 @@ impl<Config: config::Config> XcmExecutor<Config> {
holding: &mut Assets,
order: Order<Config::Call>,
trader: &mut Config::Trader,
num_recursions: u32,
) -> Result<Weight, XcmError> {
log::trace!(
target: "xcm::execute_orders",
"origin: {:?}, holding: {:?}, effect: {:?}",
"origin: {:?}, holding: {:?}, order: {:?}, recursion: {:?}",
origin,
holding,
order,
num_recursions,
);
if num_recursions > MAX_RECURSION_LIMIT {
return Err(XcmError::RecursionLimitReached)
}
let mut total_surplus = 0;
match order {
Order::DepositAsset { assets, max_assets, beneficiary } => {
......@@ -314,7 +344,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
let mut remaining_weight = weight;
for order in orders.into_iter() {
match Self::execute_orders(origin, holding, order, trader) {
match Self::execute_orders(origin, holding, order, trader, num_recursions + 1) {
Err(e) if halt_on_error => return Err(e),
Err(_) => {},
Ok(surplus) => total_surplus += surplus,
......@@ -328,6 +358,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
&mut remaining_weight,
None,
trader,
num_recursions + 1,
) {
Err(e) if halt_on_error => return Err(e),
Err(_) => {},
......
......@@ -24,4 +24,4 @@ xcm-builder = { path = "../../xcm-builder" }
pallet-xcm = { path = "../../pallet-xcm" }
polkadot-core-primitives = { path = "../../../core-primitives"}
polkadot-runtime-parachains = { path = "../../../runtime/parachains" }
polkadot-parachain = { path = "../../../parachain" }
\ No newline at end of file
polkadot-parachain = { path = "../../../parachain" }
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment