xcm-executor: validate destinations for ReserveWithdraw and Teleport …

…transfers (#5660)

This change adds the required validation for stronger UX guarantees when
using `InitiateReserveWithdraw` or `InitiateTeleport` XCM instructions.
Execution of the instructions will fail if the local chain is not
configured to trust the "destination" or "reserve" chain as a
reserve/trusted-teleporter for the provided "assets".

With this change, misuse of `InitiateReserveWithdraw`/`InitiateTeleport`
fails on origin with no overall side-effects, rather than failing on
destination (with side-effects to origin's assets issuance).

The commit also makes the same validations for pallet-xcm transfers, and
adds regression tests.

---------

Signed-off-by: Adrian Catangiu <[email protected]>
Co-authored-by: Branislav Kontur <[email protected]>

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/lib.rs

@@ -51,6 +51,7 @@ mod imports {
 	pub use rococo_system_emulated_network::{
 		asset_hub_rococo_emulated_chain::{
 			asset_hub_rococo_runtime::{
+				self,
 				xcm_config::{
 					self as ahr_xcm_config, TokenLocation as RelayLocation,
 					XcmConfig as AssetHubRococoXcmConfig,

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/hybrid_transfers.rs

@@ -449,7 +449,16 @@ fn transfer_foreign_assets_from_para_to_para_through_asset_hub() {
 	let sov_of_receiver_on_ah = AssetHubRococo::sovereign_account_id_of(receiver_as_seen_by_ah);
 	let wnd_to_send = ASSET_HUB_ROCOCO_ED * 10_000_000;
 
-	// Configure destination chain to trust AH as reserve of WND
+	// Configure source and destination chains to trust AH as reserve of WND
+	PenpalA::execute_with(|| {
+		assert_ok!(<PenpalA as Chain>::System::set_storage(
+			<PenpalA as Chain>::RuntimeOrigin::root(),
+			vec![(
+				PenpalCustomizableAssetFromSystemAssetHub::key().to_vec(),
+				Location::new(2, [GlobalConsensus(Westend)]).encode(),
+			)],
+		));
+	});
 	PenpalB::execute_with(|| {
 		assert_ok!(<PenpalB as Chain>::System::set_storage(
 			<PenpalB as Chain>::RuntimeOrigin::root(),

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/reserve_transfer.rs

@@ -1548,3 +1548,58 @@ fn reserve_transfer_usdt_from_para_to_para_through_asset_hub() {
 	// Receiver's balance is increased
 	assert!(receiver_assets_after > receiver_assets_before);
 }
+
+/// Reserve Withdraw Native Asset from AssetHub to Parachain fails.
+#[test]
+fn reserve_withdraw_from_untrusted_reserve_fails() {
+	// Init values for Parachain Origin
+	let destination = AssetHubRococo::sibling_location_of(PenpalA::para_id());
+	let signed_origin =
+		<AssetHubRococo as Chain>::RuntimeOrigin::signed(AssetHubRococoSender::get().into());
+	let roc_to_send: Balance = ROCOCO_ED * 10000;
+	let roc_location = RelayLocation::get();
+
+	// Assets to send
+	let assets: Vec<Asset> = vec![(roc_location.clone(), roc_to_send).into()];
+	let fee_id: AssetId = roc_location.into();
+
+	// this should fail
+	AssetHubRococo::execute_with(|| {
+		let result = <AssetHubRococo as AssetHubRococoPallet>::PolkadotXcm::transfer_assets_using_type_and_then(
+			signed_origin.clone(),
+			bx!(destination.clone().into()),
+			bx!(assets.clone().into()),
+			bx!(TransferType::DestinationReserve),
+			bx!(fee_id.into()),
+			bx!(TransferType::DestinationReserve),
+			bx!(VersionedXcm::from(Xcm::<()>::new())),
+			Unlimited,
+		);
+		assert_err!(
+			result,
+			DispatchError::Module(sp_runtime::ModuleError {
+				index: 31,
+				error: [22, 0, 0, 0],
+				message: Some("InvalidAssetUnsupportedReserve")
+			})
+		);
+	});
+
+	// this should also fail
+	AssetHubRococo::execute_with(|| {
+		let xcm: Xcm<asset_hub_rococo_runtime::RuntimeCall> = Xcm(vec![
+			WithdrawAsset(assets.into()),
+			InitiateReserveWithdraw {
+				assets: Wild(All),
+				reserve: destination,
+				xcm: Xcm::<()>::new(),
+			},
+		]);
+		let result = <AssetHubRococo as AssetHubRococoPallet>::PolkadotXcm::execute(
+			signed_origin,
+			bx!(xcm::VersionedXcm::V4(xcm)),
+			Weight::MAX,
+		);
+		assert!(result.is_err());
+	});
+}

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/teleport.rs

@@ -527,3 +527,54 @@ fn bidirectional_teleport_foreign_assets_between_para_and_asset_hub() {
 		system_para_to_para_transfer_assets,
 	);
 }
+
+/// Teleport Native Asset from AssetHub to Parachain fails.
+#[test]
+fn teleport_to_untrusted_chain_fails() {
+	// Init values for Parachain Origin
+	let destination = AssetHubRococo::sibling_location_of(PenpalA::para_id());
+	let signed_origin =
+		<AssetHubRococo as Chain>::RuntimeOrigin::signed(AssetHubRococoSender::get().into());
+	let roc_to_send: Balance = ROCOCO_ED * 10000;
+	let roc_location = RelayLocation::get();
+
+	// Assets to send
+	let assets: Vec<Asset> = vec![(roc_location.clone(), roc_to_send).into()];
+	let fee_id: AssetId = roc_location.into();
+
+	// this should fail
+	AssetHubRococo::execute_with(|| {
+		let result = <AssetHubRococo as AssetHubRococoPallet>::PolkadotXcm::transfer_assets_using_type_and_then(
+			signed_origin.clone(),
+			bx!(destination.clone().into()),
+			bx!(assets.clone().into()),
+			bx!(TransferType::Teleport),
+			bx!(fee_id.into()),
+			bx!(TransferType::Teleport),
+			bx!(VersionedXcm::from(Xcm::<()>::new())),
+			Unlimited,
+		);
+		assert_err!(
+			result,
+			DispatchError::Module(sp_runtime::ModuleError {
+				index: 31,
+				error: [2, 0, 0, 0],
+				message: Some("Filtered")
+			})
+		);
+	});
+
+	// this should also fail
+	AssetHubRococo::execute_with(|| {
+		let xcm: Xcm<asset_hub_rococo_runtime::RuntimeCall> = Xcm(vec![
+			WithdrawAsset(assets.into()),
+			InitiateTeleport { assets: Wild(All), dest: destination, xcm: Xcm::<()>::new() },
+		]);
+		let result = <AssetHubRococo as AssetHubRococoPallet>::PolkadotXcm::execute(
+			signed_origin,
+			bx!(xcm::VersionedXcm::V4(xcm)),
+			Weight::MAX,
+		);
+		assert!(result.is_err());
+	});
+}

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs

@@ -48,6 +48,7 @@ mod imports {
 	pub use westend_system_emulated_network::{
 		asset_hub_westend_emulated_chain::{
 			asset_hub_westend_runtime::{
+				self,
 				xcm_config::{
 					self as ahw_xcm_config, WestendLocation as RelayLocation,
 					XcmConfig as AssetHubWestendXcmConfig,

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/hybrid_transfers.rs

@@ -450,7 +450,16 @@ fn transfer_foreign_assets_from_para_to_para_through_asset_hub() {
 	let sov_of_receiver_on_ah = AssetHubWestend::sovereign_account_id_of(receiver_as_seen_by_ah);
 	let roc_to_send = ASSET_HUB_WESTEND_ED * 10_000_000;
 
-	// Configure destination chain to trust AH as reserve of ROC
+	// Configure source and destination chains to trust AH as reserve of ROC
+	PenpalA::execute_with(|| {
+		assert_ok!(<PenpalA as Chain>::System::set_storage(
+			<PenpalA as Chain>::RuntimeOrigin::root(),
+			vec![(
+				PenpalCustomizableAssetFromSystemAssetHub::key().to_vec(),
+				Location::new(2, [GlobalConsensus(Rococo)]).encode(),
+			)],
+		));
+	});
 	PenpalB::execute_with(|| {
 		assert_ok!(<PenpalB as Chain>::System::set_storage(
 			<PenpalB as Chain>::RuntimeOrigin::root(),

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/reserve_transfer.rs

@@ -1552,3 +1552,58 @@ fn reserve_transfer_usdt_from_para_to_para_through_asset_hub() {
 	// Receiver's balance is increased
 	assert!(receiver_assets_after > receiver_assets_before);
 }
+
+/// Reserve Withdraw Native Asset from AssetHub to Parachain fails.
+#[test]
+fn reserve_withdraw_from_untrusted_reserve_fails() {
+	// Init values for Parachain Origin
+	let destination = AssetHubWestend::sibling_location_of(PenpalA::para_id());
+	let signed_origin =
+		<AssetHubWestend as Chain>::RuntimeOrigin::signed(AssetHubWestendSender::get().into());
+	let roc_to_send: Balance = WESTEND_ED * 10000;
+	let roc_location = RelayLocation::get();
+
+	// Assets to send
+	let assets: Vec<Asset> = vec![(roc_location.clone(), roc_to_send).into()];
+	let fee_id: AssetId = roc_location.into();
+
+	// this should fail
+	AssetHubWestend::execute_with(|| {
+		let result = <AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::transfer_assets_using_type_and_then(
+			signed_origin.clone(),
+			bx!(destination.clone().into()),
+			bx!(assets.clone().into()),
+			bx!(TransferType::DestinationReserve),
+			bx!(fee_id.into()),
+			bx!(TransferType::DestinationReserve),
+			bx!(VersionedXcm::from(Xcm::<()>::new())),
+			Unlimited,
+		);
+		assert_err!(
+			result,
+			DispatchError::Module(sp_runtime::ModuleError {
+				index: 31,
+				error: [22, 0, 0, 0],
+				message: Some("InvalidAssetUnsupportedReserve")
+			})
+		);
+	});
+
+	// this should also fail
+	AssetHubWestend::execute_with(|| {
+		let xcm: Xcm<asset_hub_westend_runtime::RuntimeCall> = Xcm(vec![
+			WithdrawAsset(assets.into()),
+			InitiateReserveWithdraw {
+				assets: Wild(All),
+				reserve: destination,
+				xcm: Xcm::<()>::new(),
+			},
+		]);
+		let result = <AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::execute(
+			signed_origin,
+			bx!(xcm::VersionedXcm::V4(xcm)),
+			Weight::MAX,
+		);
+		assert!(result.is_err());
+	});
+}

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/teleport.rs

@@ -530,3 +530,54 @@ fn bidirectional_teleport_foreign_assets_between_para_and_asset_hub() {
 		system_para_to_para_transfer_assets,
 	);
 }
+
+/// Teleport Native Asset from AssetHub to Parachain fails.
+#[test]
+fn teleport_to_untrusted_chain_fails() {
+	// Init values for Parachain Origin
+	let destination = AssetHubWestend::sibling_location_of(PenpalA::para_id());
+	let signed_origin =
+		<AssetHubWestend as Chain>::RuntimeOrigin::signed(AssetHubWestendSender::get().into());
+	let roc_to_send: Balance = WESTEND_ED * 10000;
+	let roc_location = RelayLocation::get();
+
+	// Assets to send
+	let assets: Vec<Asset> = vec![(roc_location.clone(), roc_to_send).into()];
+	let fee_id: AssetId = roc_location.into();
+
+	// this should fail
+	AssetHubWestend::execute_with(|| {
+		let result = <AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::transfer_assets_using_type_and_then(
+			signed_origin.clone(),
+			bx!(destination.clone().into()),
+			bx!(assets.clone().into()),
+			bx!(TransferType::Teleport),
+			bx!(fee_id.into()),
+			bx!(TransferType::Teleport),
+			bx!(VersionedXcm::from(Xcm::<()>::new())),
+			Unlimited,
+		);
+		assert_err!(
+			result,
+			DispatchError::Module(sp_runtime::ModuleError {
+				index: 31,
+				error: [2, 0, 0, 0],
+				message: Some("Filtered")
+			})
+		);
+	});
+
+	// this should also fail
+	AssetHubWestend::execute_with(|| {
+		let xcm: Xcm<asset_hub_westend_runtime::RuntimeCall> = Xcm(vec![
+			WithdrawAsset(assets.into()),
+			InitiateTeleport { assets: Wild(All), dest: destination, xcm: Xcm::<()>::new() },
+		]);
+		let result = <AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::execute(
+			signed_origin,
+			bx!(xcm::VersionedXcm::V4(xcm)),
+			Weight::MAX,
+		);
+		assert!(result.is_err());
+	});
+}

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/lib.rs

@@ -16,6 +16,7 @@
 #[cfg(test)]
 mod imports {
 	// Substrate
+	pub use codec::Encode;
 	pub use frame_support::{assert_err, assert_ok, pallet_prelude::DispatchResult};
 	pub use sp_runtime::DispatchError;
 

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/asset_transfers.rs

@@ -432,6 +432,16 @@ fn send_back_wnds_from_penpal_rococo_through_asset_hub_rococo_to_asset_hub_weste
 		ASSET_MIN_BALANCE,
 		vec![(sender.clone(), amount * 2)],
 	);
+	// Configure source Penpal chain to trust local AH as reserve of bridged WND
+	PenpalA::execute_with(|| {
+		assert_ok!(<PenpalA as Chain>::System::set_storage(
+			<PenpalA as Chain>::RuntimeOrigin::root(),
+			vec![(
+				PenpalCustomizableAssetFromSystemAssetHub::key().to_vec(),
+				wnd_at_rococo_parachains.encode(),
+			)],
+		));
+	});
 
 	// fund the AHR's SA on AHW with the WND tokens held in reserve
 	let sov_ahr_on_ahw = AssetHubWestend::sovereign_account_of_parachain_on_other_global_consensus(

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/lib.rs

@@ -16,6 +16,7 @@
 #[cfg(test)]
 mod imports {
 	// Substrate
+	pub use codec::Encode;
 	pub use frame_support::{assert_err, assert_ok, pallet_prelude::DispatchResult};
 	pub use sp_runtime::DispatchError;
 
@@ -52,7 +53,10 @@ mod imports {
 			BridgeHubWestendParaPallet as BridgeHubWestendPallet, BridgeHubWestendXcmConfig,
 		},
 		penpal_emulated_chain::{
-			penpal_runtime::xcm_config::UniversalLocation as PenpalUniversalLocation,
+			penpal_runtime::xcm_config::{
+				CustomizableAssetFromSystemAssetHub as PenpalCustomizableAssetFromSystemAssetHub,
+				UniversalLocation as PenpalUniversalLocation,
+			},
 			PenpalAssetOwner, PenpalBParaPallet as PenpalBPallet,
 		},
 		westend_emulated_chain::{

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/asset_transfers.rs

@@ -452,6 +452,16 @@ fn send_back_rocs_from_penpal_westend_through_asset_hub_westend_to_asset_hub_roc
 		ASSET_MIN_BALANCE,
 		vec![(sender.clone(), amount * 2)],
 	);
+	// Configure source Penpal chain to trust local AH as reserve of bridged ROC
+	PenpalB::execute_with(|| {
+		assert_ok!(<PenpalB as Chain>::System::set_storage(
+			<PenpalB as Chain>::RuntimeOrigin::root(),
+			vec![(
+				PenpalCustomizableAssetFromSystemAssetHub::key().to_vec(),
+				roc_at_westend_parachains.encode(),
+			)],
+		));
+	});
 
 	// fund the AHW's SA on AHR with the ROC tokens held in reserve
 	let sov_ahw_on_ahr = AssetHubRococo::sovereign_account_of_parachain_on_other_global_consensus(

polkadot/xcm/pallet-xcm/src/lib.rs

@@ -1939,6 +1939,10 @@ impl<T: Config> Pallet<T> {
 	) -> Result<(Xcm<<T as Config>::RuntimeCall>, Xcm<()>), Error<T>> {
 		let value = (origin, vec![fees.clone()]);
 		ensure!(T::XcmReserveTransferFilter::contains(&value), Error::<T>::Filtered);
+		ensure!(
+			<T::XcmExecutor as XcmAssetTransfers>::IsReserve::contains(&fees, &dest),
+			Error::<T>::InvalidAssetUnsupportedReserve
+		);
 
 		let context = T::UniversalLocation::get();
 		let reanchored_fees = fees
@@ -1973,6 +1977,12 @@ impl<T: Config> Pallet<T> {
 		let value = (origin, assets);
 		ensure!(T::XcmReserveTransferFilter::contains(&value), Error::<T>::Filtered);
 		let (_, assets) = value;
+		for asset in assets.iter() {
+			ensure!(
+				<T::XcmExecutor as XcmAssetTransfers>::IsReserve::contains(&asset, &dest),
+				Error::<T>::InvalidAssetUnsupportedReserve
+			);
+		}
 
 		// max assets is `assets` (+ potentially separately handled fee)
 		let max_assets =
@@ -2079,6 +2089,10 @@ impl<T: Config> Pallet<T> {
 	) -> Result<(Xcm<<T as Config>::RuntimeCall>, Xcm<()>), Error<T>> {
 		let value = (origin, vec![fees.clone()]);
 		ensure!(T::XcmTeleportFilter::contains(&value), Error::<T>::Filtered);
+		ensure!(
+			<T::XcmExecutor as XcmAssetTransfers>::IsTeleporter::contains(&fees, &dest),
+			Error::<T>::Filtered
+		);
 
 		let context = T::UniversalLocation::get();
 		let reanchored_fees = fees
@@ -2134,6 +2148,12 @@ impl<T: Config> Pallet<T> {
 		let value = (origin, assets);
 		ensure!(T::XcmTeleportFilter::contains(&value), Error::<T>::Filtered);
 		let (_, assets) = value;
+		for asset in assets.iter() {
+			ensure!(
+				<T::XcmExecutor as XcmAssetTransfers>::IsTeleporter::contains(&asset, &dest),
+				Error::<T>::Filtered
+			);
+		}
 
 		// max assets is `assets` (+ potentially separately handled fee)
 		let max_assets =