Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Guard against XCM recursive bombs by setting a recursion limit #3555

Closed
wants to merge 42 commits into from
Closed
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
b0fc0a8
Guard against XCM recursive bombs by setting a recursion limit
KiChjang Aug 2, 2021
db667f6
Add test and set a lower recursion limit
KiChjang Aug 2, 2021
d7601e9
Use u32 instead of usize for recursion limit
KiChjang Aug 2, 2021
bd32a66
Make spellcheck happy
KiChjang Aug 2, 2021
07107bb
Cargo fmt
KiChjang Aug 2, 2021
2ea42b0
Limit XCM decoding depth in UMP message processing
KiChjang Aug 3, 2021
83becb3
Modify test to check for recursion in BuyExecution
KiChjang Aug 3, 2021
76d9c7a
Update xcm/xcm-simulator/example/src/lib.rs
KiChjang Aug 3, 2021
08379ba
Make cargo fmt happy
KiChjang Aug 3, 2021
39181b4
WIP for testing recursion limit in WASM
KiChjang Aug 5, 2021
d0adfc4
Revert "WIP for testing recursion limit in WASM"
KiChjang Aug 5, 2021
6281a80
Remove XCM recursion limit test
KiChjang Aug 5, 2021
0a6037d
Add recursion test for XCM message execution
KiChjang Aug 5, 2021
1550e44
Set a more sensible recursion limit
KiChjang Aug 5, 2021
1c75cdb
Cargo fmt
KiChjang Aug 5, 2021
5b65a65
Merge remote-tracking branch 'origin/master' into kckyeung/harden-xcm
KiChjang Aug 5, 2021
8f12346
Implement successful_origin for benchmarks
KiChjang Aug 5, 2021
c8f67dd
Set recursion limit to 8 and create integration tests directory for x…
KiChjang Aug 5, 2021
b9ba7b9
Cargo fmt
KiChjang Aug 5, 2021
39f327a
Add runtime-benchmarks feature to test-runtime
KiChjang Aug 5, 2021
560c6d0
Give up creating ConvertOriginToLocal and use EnsureXcm
KiChjang Aug 6, 2021
a795199
Re-add ConvertOriginToLocal
KiChjang Aug 6, 2021
62dd306
Fix compilation
bkchr Aug 6, 2021
b14ba31
Update xcm/xcm-executor/src/lib.rs
KiChjang Aug 6, 2021
ff2d0b7
Add decoding limit to all versioned XCM decode calls
KiChjang Aug 6, 2021
bfc97ce
Fix recursion limit test
KiChjang Aug 6, 2021
cba7709
Set a lower recursion count for recursion test
KiChjang Aug 6, 2021
c961264
move integration tests to their own folder, fix recursion check in ex…
shawntabrizi Aug 6, 2021
5255c29
Remove xcm-executor integration tests directory
KiChjang Aug 6, 2021
b20f0dd
fix up
shawntabrizi Aug 6, 2021
b9fcef5
Merge branch 'kckyeung/harden-xcm' of https://github.com/paritytech/p…
shawntabrizi Aug 6, 2021
7178a88
Update Cargo.lock
shawntabrizi Aug 6, 2021
411e3fc
Update runtime/parachains/src/ump.rs
shawntabrizi Aug 6, 2021
013cb2b
use proper decode limit
shawntabrizi Aug 6, 2021
1f8a1fc
fix decode depth limit
shawntabrizi Aug 6, 2021
82c3ce3
here too
shawntabrizi Aug 6, 2021
66dcd3a
Remove unused import
KiChjang Aug 6, 2021
216c798
remove dbg
shawntabrizi Aug 6, 2021
805ebed
Merge branch 'kckyeung/harden-xcm' of https://github.com/paritytech/p…
shawntabrizi Aug 6, 2021
0eb6603
Make CI happy
KiChjang Aug 6, 2021
70fd5e6
Revert decode_all_with_depth_limit changes
KiChjang Aug 7, 2021
206514c
Remove unused import
KiChjang Aug 7, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 6 additions & 2 deletions runtime/parachains/src/ump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,18 @@ impl<XcmExecutor: xcm::v0::ExecuteXcm<C::Call>, C: Config> UmpSink for XcmSink<X
data: &[u8],
max_weight: Weight,
) -> Result<Weight, (MessageId, Weight)> {
use parity_scale_codec::DecodeLimit;
use xcm::{
v0::{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));
Expand Down
8 changes: 8 additions & 0 deletions runtime/test-runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.1"
Expand All @@ -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",
Expand Down
27 changes: 27 additions & 0 deletions runtime/test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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::v0::NetworkId = xcm::v0::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_config::ConvertOriginToLocal;
type LocationInverter = xcm_config::InvertNothing;
type SendXcmOrigin = xcm_config::ConvertOriginToLocal;
type Weigher = xcm_builder::FixedWeightBounds<BaseXcmWeight, Call>;
type XcmRouter = xcm_config::DoNothingRouter;
type XcmExecuteFilter =
frame_support::traits::All<(xcm::v0::MultiLocation, xcm::v0::Xcm<Call>)>;
type XcmExecutor = xcm_executor::XcmExecutor<xcm_config::XcmConfig>;
type XcmTeleportFilter =
frame_support::traits::All<(xcm::v0::MultiLocation, Vec<xcm::v0::MultiAsset>)>;
type XcmReserveTransferFilter =
frame_support::traits::All<(xcm::v0::MultiLocation, Vec<xcm::v0::MultiAsset>)>;
}

impl parachains_hrmp::Config for Runtime {
type Event = Event;
type Origin = Origin;
Expand Down Expand Up @@ -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>},
Expand Down
88 changes: 88 additions & 0 deletions runtime/test-runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// 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::{
traits::{All, EnsureOrigin, OriginTrait},
weights::Weight,
};
use xcm::v0::{Error as XcmError, MultiAsset, MultiLocation, Result as XcmResult, SendXcm, Xcm};
use xcm_builder::{AllowUnpaidExecutionFrom, FixedWeightBounds};
use xcm_executor::{
traits::{InvertLocation, TransactAsset, WeightTrader},
Assets,
};

pub struct ConvertOriginToLocal;
impl<Origin: OriginTrait> EnsureOrigin<Origin> for ConvertOriginToLocal {
type Success = MultiLocation;

fn try_origin(_: Origin) -> Result<MultiLocation, Origin> {
Ok(MultiLocation::Null)
}
}

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> {
Ok(Assets::default())
}
}

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::Null
}
}

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 = ();
}
4 changes: 1 addition & 3 deletions xcm/src/double_encoded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 3 additions & 0 deletions xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ pub mod v0;
mod double_encoded;
pub use double_encoded::DoubleEncoded;

/// Maximum nesting level for XCM decoding.
pub const MAX_XCM_DECODE_DEPTH: u32 = 8;

/// A single XCM message, together with its version code.
#[derive(Derivative, Encode, Decode)]
#[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))]
Expand Down
2 changes: 2 additions & 0 deletions xcm/src/v0/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions xcm/xcm-executor/integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[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",
]
Loading