diff --git a/.gitignore b/.gitignore index d48287657085..d5e0f1875da7 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,4 @@ substrate.code-workspace target/ *.scale justfile +python-venv diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs index 3b5dbdb77c89..a197aa59701b 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/mod.rs @@ -26,6 +26,7 @@ pub struct EthereumBlobExporter< OutboundQueue, AgentHashedDescription, ConvertAssetId, + AssetHubParaId, >( PhantomData<( UniversalLocation, @@ -33,17 +34,25 @@ pub struct EthereumBlobExporter< OutboundQueue, AgentHashedDescription, ConvertAssetId, + AssetHubParaId, )>, ); -impl - ExportXcm +impl< + UniversalLocation, + EthereumNetwork, + OutboundQueue, + AgentHashedDescription, + ConvertAssetId, + AssetHubParaId, + > ExportXcm for EthereumBlobExporter< UniversalLocation, EthereumNetwork, OutboundQueue, AgentHashedDescription, ConvertAssetId, + AssetHubParaId, > where UniversalLocation: Get, @@ -51,6 +60,7 @@ where OutboundQueue: SendMessage, AgentHashedDescription: ConvertLocation, ConvertAssetId: MaybeEquivalence, + AssetHubParaId: Get, { type Ticket = (Vec, XcmHash); @@ -94,11 +104,12 @@ where return Err(SendError::NotApplicable) } + // Check the location here can only be AssetHub sovereign let para_id = match local_sub.as_slice() { - [Parachain(para_id)] => *para_id, + [Parachain(para_id)] if ParaId::from(*para_id) == AssetHubParaId::get() => *para_id, _ => { - log::error!(target: "xcm::ethereum_blob_exporter", "could not get parachain id from universal source '{local_sub:?}'."); - return Err(SendError::NotApplicable) + log::debug!(target: "xcm::ethereum_blob_exporter", "only supports Asset Hub root location as the universal source '{local_sub:?}'."); + return Err(SendError::NotApplicable); }, }; diff --git a/bridges/snowbridge/primitives/router/src/outbound/tests.rs b/bridges/snowbridge/primitives/router/src/outbound/tests.rs index 44f81ce31b3a..fba7ce058d00 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/tests.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/tests.rs @@ -18,6 +18,7 @@ parameter_types! { UniversalLocation: InteriorLocation = [GlobalConsensus(RelayNetwork::get()), Parachain(1013)].into(); const BridgedNetwork: NetworkId = Ethereum{ chain_id: 1 }; const NonBridgedNetwork: NetworkId = Ethereum{ chain_id: 2 }; + pub AssetHubParaId: ParaId = ParaId::from(1000); } struct MockOkOutboundQueue; @@ -86,6 +87,7 @@ fn exporter_validate_with_unknown_network_yields_not_applicable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -105,6 +107,7 @@ fn exporter_validate_with_invalid_destination_yields_missing_argument() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -127,6 +130,7 @@ fn exporter_validate_with_x8_destination_yields_not_applicable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -146,6 +150,7 @@ fn exporter_validate_without_universal_source_yields_missing_argument() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -165,6 +170,7 @@ fn exporter_validate_without_global_universal_location_yields_not_applicable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -184,6 +190,7 @@ fn exporter_validate_without_global_bridge_location_yields_not_applicable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -204,6 +211,7 @@ fn exporter_validate_with_remote_universal_source_yields_not_applicable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -223,6 +231,7 @@ fn exporter_validate_without_para_id_in_source_yields_not_applicable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -243,6 +252,7 @@ fn exporter_validate_complex_para_id_in_source_yields_not_applicable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -263,6 +273,7 @@ fn exporter_validate_without_xcm_message_yields_missing_argument() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -310,6 +321,7 @@ fn exporter_validate_with_max_target_fee_yields_unroutable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::Unroutable)); @@ -337,6 +349,7 @@ fn exporter_validate_with_unparsable_xcm_yields_unroutable() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::Unroutable)); @@ -383,6 +396,7 @@ fn exporter_validate_xcm_success_case_1() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert!(result.is_ok()); @@ -396,6 +410,7 @@ fn exporter_deliver_with_submit_failure_yields_unroutable() { MockErrOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::deliver((hex!("deadbeef").to_vec(), XcmHash::default())); assert_eq!(result, Err(XcmSendError::Transport("other transport error"))) } @@ -1208,6 +1223,7 @@ fn exporter_validate_with_invalid_dest_does_not_alter_destination() { MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate( network, channel, &mut universal_source_wrapper, &mut dest_wrapper, &mut msg_wrapper ); @@ -1261,6 +1277,7 @@ fn exporter_validate_with_invalid_universal_source_does_not_alter_universal_sour MockOkOutboundQueue, AgentIdOf, MockTokenIdConvert, + AssetHubParaId, >::validate( network, channel, &mut universal_source_wrapper, &mut dest_wrapper, &mut msg_wrapper ); diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/mod.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/mod.rs index 6c1cdb98e8b2..460e37bcd321 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/mod.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/mod.rs @@ -20,6 +20,7 @@ mod claim_assets; mod register_bridged_assets; mod send_xcm; mod snowbridge; +mod snowbridge_edge_case; mod teleport; mod transact; diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs index ffa60a4f52e7..dfb3febe51db 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs @@ -31,7 +31,7 @@ use xcm_executor::traits::ConvertLocation; const INITIAL_FUND: u128 = 5_000_000_000_000; pub const CHAIN_ID: u64 = 11155111; pub const WETH: [u8; 20] = hex!("87d1f7fdfEe7f651FaBc8bFCB6E086C278b77A7d"); -const ETHEREUM_DESTINATION_ADDRESS: [u8; 20] = hex!("44a57ee2f2FCcb85FDa2B0B18EBD0D8D2333700e"); +pub const ETHEREUM_DESTINATION_ADDRESS: [u8; 20] = hex!("44a57ee2f2FCcb85FDa2B0B18EBD0D8D2333700e"); const XCM_FEE: u128 = 100_000_000_000; const TOKEN_AMOUNT: u128 = 100_000_000_000; diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge_edge_case.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge_edge_case.rs new file mode 100644 index 000000000000..7bf24e39f1f0 --- /dev/null +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge_edge_case.rs @@ -0,0 +1,227 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +use crate::imports::*; +use bridge_hub_westend_runtime::xcm_config::LocationToAccountId; +use frame_support::traits::fungibles::Mutate; +use snowbridge_router_primitives::inbound::EthereumLocationsConverterFor; +use testnet_parachains_constants::westend::snowbridge::EthereumNetwork; +use xcm_executor::traits::ConvertLocation; + +use crate::tests::snowbridge::{CHAIN_ID, ETHEREUM_DESTINATION_ADDRESS, WETH}; + +const INITIAL_FUND: u128 = 5_000_000_000_000; +const TOKEN_AMOUNT: u128 = 100_000_000_000; + +pub fn bridge_hub() -> Location { + Location::new(1, Parachain(BridgeHubWestend::para_id().into())) +} + +pub fn beneficiary() -> Location { + Location::new(0, [AccountKey20 { network: None, key: ETHEREUM_DESTINATION_ADDRESS.into() }]) +} + +pub fn ethereum() -> Location { + Location::new(2, [GlobalConsensus(EthereumNetwork::get())]) +} + +pub fn weth_location() -> Location { + Location::new( + 2, + [GlobalConsensus(EthereumNetwork::get()), AccountKey20 { network: None, key: WETH }], + ) +} + +pub fn fund_on_bh() { + let asset_hub_sovereign = BridgeHubWestend::sovereign_account_id_of(Location::new( + 1, + [Parachain(AssetHubWestend::para_id().into())], + )); + BridgeHubWestend::fund_accounts(vec![(asset_hub_sovereign.clone(), INITIAL_FUND)]); +} + +pub fn fund_on_ah() { + AssetHubWestend::fund_accounts(vec![(AssetHubWestendSender::get(), INITIAL_FUND)]); + AssetHubWestend::fund_accounts(vec![(AssetHubWestendReceiver::get(), INITIAL_FUND)]); + + AssetHubWestend::execute_with(|| { + assert_ok!(::ForeignAssets::mint_into( + weth_location().try_into().unwrap(), + &AssetHubWestendReceiver::get(), + INITIAL_FUND, + )); + assert_ok!(::ForeignAssets::mint_into( + weth_location().try_into().unwrap(), + &AssetHubWestendSender::get(), + INITIAL_FUND, + )); + }); + + let ethereum_sovereign: AccountId = + EthereumLocationsConverterFor::<[u8; 32]>::convert_location(&Location::new( + 2, + [GlobalConsensus(EthereumNetwork::get())], + )) + .unwrap() + .into(); + AssetHubWestend::fund_accounts(vec![(ethereum_sovereign.clone(), INITIAL_FUND)]); +} + +pub fn register_weth_on_ah() { + let ethereum_sovereign: AccountId = + EthereumLocationsConverterFor::<[u8; 32]>::convert_location(&Location::new( + 2, + [GlobalConsensus(EthereumNetwork::get())], + )) + .unwrap() + .into(); + + AssetHubWestend::execute_with(|| { + type RuntimeOrigin = ::RuntimeOrigin; + + assert_ok!(::ForeignAssets::force_create( + RuntimeOrigin::root(), + weth_location().try_into().unwrap(), + ethereum_sovereign.clone().into(), + true, + 1, + )); + + assert!(::ForeignAssets::asset_exists( + weth_location().try_into().unwrap(), + )); + }); +} + +#[test] +fn user_send_message_bypass_exporter_on_ah_will_fail() { + let sov_account_for_assethub_sender = LocationToAccountId::convert_location(&Location::new( + 1, + [ + Parachain(AssetHubWestend::para_id().into()), + AccountId32 { + network: Some(ByGenesis(WESTEND_GENESIS_HASH)), + id: AssetHubWestendSender::get().into(), + }, + ], + )) + .unwrap(); + BridgeHubWestend::fund_accounts(vec![(sov_account_for_assethub_sender, INITIAL_FUND)]); + + AssetHubWestend::execute_with(|| { + type RuntimeEvent = ::RuntimeEvent; + type RuntimeOrigin = ::RuntimeOrigin; + + let local_fee_asset = + Asset { id: AssetId(Location::parent()), fun: Fungible(1_000_000_000_000) }; + + let weth_location_reanchored = + Location::new(0, [AccountKey20 { network: None, key: WETH.into() }]); + + let weth_asset = Asset { + id: AssetId(weth_location_reanchored.clone()), + fun: Fungible(TOKEN_AMOUNT * 1_000_000_000), + }; + + assert_ok!(::PolkadotXcm::send( + RuntimeOrigin::signed(AssetHubWestendSender::get()), + bx!(VersionedLocation::from(bridge_hub())), + bx!(VersionedXcm::from(Xcm(vec![ + WithdrawAsset(local_fee_asset.clone().into()), + BuyExecution { fees: local_fee_asset.clone(), weight_limit: Unlimited }, + ExportMessage { + network: Ethereum { chain_id: CHAIN_ID }, + destination: Here, + xcm: Xcm(vec![ + WithdrawAsset(weth_asset.clone().into()), + DepositAsset { assets: Wild(All), beneficiary: beneficiary() }, + SetTopic([0; 32]), + ]), + }, + ]))), + )); + + assert_expected_events!( + AssetHubWestend, + vec![RuntimeEvent::PolkadotXcm(pallet_xcm::Event::Sent{ .. }) => {},] + ); + }); + + BridgeHubWestend::execute_with(|| { + type RuntimeEvent = ::RuntimeEvent; + assert_expected_events!( + BridgeHubWestend, + vec![RuntimeEvent::MessageQueue(pallet_message_queue::Event::Processed{ success:false, .. }) => {},] + ); + }); +} + +#[test] +fn user_exploit_with_arbitrary_message_will_fail() { + fund_on_bh(); + register_weth_on_ah(); + fund_on_ah(); + AssetHubWestend::execute_with(|| { + type RuntimeEvent = ::RuntimeEvent; + type RuntimeOrigin = ::RuntimeOrigin; + + let remote_fee_asset_location: Location = Location::new( + 2, + [EthereumNetwork::get().into(), AccountKey20 { network: None, key: WETH }], + ) + .into(); + + let remote_fee_asset: Asset = (remote_fee_asset_location.clone(), 1).into(); + + let assets = VersionedAssets::from(vec![remote_fee_asset]); + + let exploited_weth = Asset { + id: AssetId(Location::new(0, [AccountKey20 { network: None, key: WETH.into() }])), + // A big amount without burning + fun: Fungible(TOKEN_AMOUNT * 1_000_000_000), + }; + + assert_ok!(::PolkadotXcm::transfer_assets_using_type_and_then( + RuntimeOrigin::signed(AssetHubWestendSender::get()), + bx!(VersionedLocation::from(ethereum())), + bx!(assets), + bx!(TransferType::DestinationReserve), + bx!(VersionedAssetId::from(remote_fee_asset_location.clone())), + bx!(TransferType::DestinationReserve), + bx!(VersionedXcm::from(Xcm(vec![ + // Instructions inner are user provided and untrustworthy/dangerous! + // Currently it depends on EthereumBlobExporter on BH to check the message is legal + // and convert to Ethereum command, should be very careful to handle that. + // Or we may move the security check ahead to AH to fail earlier if possible. + WithdrawAsset(exploited_weth.clone().into()), + DepositAsset { assets: Wild(All), beneficiary: beneficiary() }, + SetTopic([0; 32]), + ]))), + Unlimited + )); + + assert_expected_events!( + AssetHubWestend, + vec![RuntimeEvent::PolkadotXcm(pallet_xcm::Event::Sent{ .. }) => {},] + ); + }); + + BridgeHubWestend::execute_with(|| { + type RuntimeEvent = ::RuntimeEvent; + assert_expected_events!( + BridgeHubWestend, + vec![RuntimeEvent::MessageQueue(pallet_message_queue::Event::Processed{ success:false, .. }) => {},] + ); + }); +} diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs index be7005b5379a..180795e76a22 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs @@ -35,6 +35,7 @@ use testnet_parachains_constants::rococo::{ use crate::xcm_config::RelayNetwork; #[cfg(feature = "runtime-benchmarks")] use benchmark_helpers::DoNothingRouter; +use cumulus_primitives_core::ParaId; use frame_support::{parameter_types, weights::ConstantMultiplier}; use pallet_xcm::EnsureXcm; use sp_runtime::{ @@ -50,11 +51,13 @@ pub type SnowbridgeExporter = EthereumBlobExporter< snowbridge_pallet_outbound_queue::Pallet, snowbridge_core::AgentIdOf, EthereumSystem, + AssetHubParaId, >; // Ethereum Bridge parameter_types! { pub storage EthereumGatewayAddress: H160 = H160(hex_literal::hex!("EDa338E4dC46038493b885327842fD3E301CaB39")); + pub AssetHubParaId: ParaId = ParaId::from(rococo_runtime_constants::system_parachain::ASSET_HUB_ID); } parameter_types! { diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs index 94921fd8af9a..fe298f45f3c0 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs @@ -36,6 +36,7 @@ use testnet_parachains_constants::westend::{ use crate::xcm_config::RelayNetwork; #[cfg(feature = "runtime-benchmarks")] use benchmark_helpers::DoNothingRouter; +use cumulus_primitives_core::ParaId; use frame_support::{parameter_types, weights::ConstantMultiplier}; use pallet_xcm::EnsureXcm; use sp_runtime::{ @@ -53,11 +54,13 @@ pub type SnowbridgeExporter = EthereumBlobExporter< snowbridge_pallet_outbound_queue::Pallet, snowbridge_core::AgentIdOf, EthereumSystem, + AssetHubParaId, >; // Ethereum Bridge parameter_types! { pub storage EthereumGatewayAddress: H160 = H160(hex_literal::hex!("EDa338E4dC46038493b885327842fD3E301CaB39")); + pub AssetHubParaId: ParaId = ParaId::from(westend_runtime_constants::system_parachain::ASSET_HUB_ID); } parameter_types! { diff --git a/prdoc/pr_6838.prdoc b/prdoc/pr_6838.prdoc new file mode 100644 index 000000000000..988c465e6783 --- /dev/null +++ b/prdoc/pr_6838.prdoc @@ -0,0 +1,9 @@ +title: 'Snowbridge: Ensure source always from Assethub for exported message' +doc: +- audience: Runtime Dev + description: |- + Ensure the last hop of exported message is always from AH. + So user can't export message from non system parachain directly. +crates: +- name: snowbridge-router-primitives + bump: patch