[runtime]: Bind source chain to relayer redirect signature (#881)

Author: dharjeezy

modules/pallets/relayer/src/accumulate.rs

@@ -22,7 +22,7 @@
 //! Substrate), and the per-leaf result validation that ties source-side fee
 //! metadata to destination-side delivery receipts.
 
-use crate::{withdrawal::WithdrawalProof, Config, Error, Event, Fees, Pallet};
+use crate::{withdrawal::WithdrawalProof, Config, Error, Event, Fees, Nonce, Pallet};
 use alloc::{collections::BTreeMap, vec::Vec};
 use alloy_primitives::Address;
 use codec::Encode;
@@ -107,7 +107,8 @@ where
 		let beneficiary_address = if let Some((beneficiary_address, signature)) =
 			withdrawal_proof.beneficiary_details
 		{
-			let msg = sp_io::hashing::keccak_256(&beneficiary_address);
+			let nonce = Nonce::<T>::get(&delivery_address, state_machine);
+			let msg = beneficiary_message(nonce, state_machine, &beneficiary_address);
 			match &signature {
 				Signature::Evm { .. } => {
 					let eth_address =
@@ -119,11 +120,17 @@ where
 				Signature::Sr25519 { .. } | Signature::Ed25519 { .. } => {
 					// verify the signature with the delivery address from the state proof
 					let _ = signature
-						.verify(&msg, Some(delivery_address))
+						.verify(&msg, Some(delivery_address.clone()))
 						.map_err(|_| Error::<T>::InvalidSignature)?;
 				},
 			}
 
+			Nonce::<T>::try_mutate(&delivery_address, state_machine, |value| {
+				*value += 1;
+				Ok::<(), ()>(())
+			})
+			.map_err(|_: ()| Error::<T>::ErrorCompletingCall)?;
+
 			let _ = Fees::<T>::try_mutate(state_machine, beneficiary_address.clone(), |inner| {
 				*inner += total_fee;
 				Ok::<(), ()>(())
@@ -294,6 +301,18 @@ where
 	}
 }
 
+/// Signed payload authorising a beneficiary redirect on a specific source chain.
+/// Including the relayer nonce alongside the state machine keeps the signature usable for
+/// exactly one accumulate call on that chain, mirroring how `withdraw_fees` binds its signed
+/// payload.
+pub fn beneficiary_message(
+	nonce: u64,
+	state_machine: StateMachine,
+	beneficiary: &[u8],
+) -> [u8; 32] {
+	sp_io::hashing::keccak_256(&(nonce, state_machine, beneficiary).encode())
+}
+
 impl<T: Config> Pallet<T> {
 	pub fn accumulate_fee_and_deposit_event(
 		state_machine: StateMachine,

modules/pallets/relayer/src/lib.rs

@@ -24,6 +24,7 @@ pub mod accumulate;
 pub mod outbound_consensus;
 pub mod withdrawal;
 
+pub use accumulate::beneficiary_message;
 pub use outbound_consensus::*;
 pub use withdrawal::message;
 

modules/pallets/testsuite/src/tests/pallet_ismp_relayer.rs

@@ -31,11 +31,11 @@ use pallet_ismp::{
 };
 use pallet_ismp_host_executive::{EvmHostParam, HostParam};
 use pallet_ismp_relayer::{
-	self as pallet_ismp_relayer, message,
+	self as pallet_ismp_relayer, beneficiary_message, message,
 	withdrawal::{Signature, WithdrawalInputData, WithdrawalProof},
 	OutboundConsensusDeliveryClaim,
 };
-use sp_core::{keccak_256, Pair, H256, U256};
+use sp_core::{Pair, H256, U256};
 use sp_trie::LayoutV0;
 use std::time::Duration;
 use substrate_state_machine::{HashAlgorithm, StateMachineProof, SubstrateStateProof};
@@ -220,7 +220,13 @@ fn test_accumulate_fees() {
 
 		let beneficiary_address = H256::random();
 
-		let signature = pair.sign(&keccak_256(beneficiary_address.as_bytes())).to_vec();
+		let signature = pair
+			.sign(&beneficiary_message(
+				0,
+				StateMachine::Kusama(2000),
+				beneficiary_address.as_bytes(),
+			))
+			.to_vec();
 
 		let withdrawal_proof = WithdrawalProof {
 			commitments: keys,
@@ -699,7 +705,13 @@ fn test_accumulate_fees_evm_signatures() {
 
 		let beneficiary_address = H256::random();
 
-		let signature = pair.sign_prehashed(&keccak_256(beneficiary_address.as_bytes())).to_vec();
+		let signature = pair
+			.sign_prehashed(&beneficiary_message(
+				0,
+				StateMachine::Kusama(2000),
+				beneficiary_address.as_bytes(),
+			))
+			.to_vec();
 
 		let withdrawal_proof = WithdrawalProof {
 			commitments: keys,
@@ -725,7 +737,7 @@ fn test_accumulate_fees_evm_signatures() {
 			},
 			beneficiary_details: Some((
 				beneficiary_address.0.to_vec().clone(),
-				Signature::Evm { address: eth_address, signature },
+				Signature::Evm { address: eth_address.clone(), signature },
 			)),
 		};
 
@@ -742,6 +754,188 @@ fn test_accumulate_fees_evm_signatures() {
 			),
 			U256::from(5000u128)
 		);
+		assert_eq!(
+			pallet_ismp_relayer::Nonce::<Test>::get(eth_address, StateMachine::Kusama(2000)),
+			1,
+			"a successful redirect must consume the signer's nonce"
+		);
+	})
+}
+
+/// A redirect signature signed for one source chain must not satisfy verification
+/// when reused on another. The state machine is folded into the signed payload
+/// so the recovered address only matches the delivery address on the chain the
+/// signature was issued for.
+#[test]
+fn test_accumulate_fees_rejects_replay_from_other_chain() {
+	let mut ext = new_test_ext();
+	ext.execute_with(|| {
+		set_timestamp::<Test>(10_000_000_000);
+		let requests = (0u64..2)
+			.into_iter()
+			.map(|nonce| {
+				let post = PostRequest {
+					source: StateMachine::Kusama(2000),
+					dest: StateMachine::Kusama(2001),
+					nonce,
+					from: vec![],
+					to: vec![],
+					timeout_timestamp: 0,
+					body: vec![],
+				};
+				hash_request::<Ismp>(&Request::Post(post))
+			})
+			.collect::<Vec<_>>();
+
+		let pair = sp_core::ecdsa::Pair::from_seed_slice(H256::random().as_bytes()).unwrap();
+		let eth_address = pair.public().to_eth_address().unwrap().to_vec();
+
+		let mut source_root = H256::default();
+		let mut source_db = MemoryDB::<KeccakHasher>::default();
+		let mut source_trie =
+			TrieDBMutBuilder::<LayoutV0<KeccakHasher>>::new(&mut source_db, &mut source_root)
+				.build();
+		let mut dest_root = H256::default();
+		let mut dest_db = MemoryDB::<KeccakHasher>::default();
+		let mut dest_trie =
+			TrieDBMutBuilder::<LayoutV0<KeccakHasher>>::new(&mut dest_db, &mut dest_root).build();
+
+		for request in &requests {
+			let request_commitment_key = RequestCommitments::<Test>::storage_key(*request);
+			let request_receipt_key = RequestReceipts::<Test>::storage_key(*request);
+			let fee_metadata = FeeMetadata::<Test> { payer: [0; 32].into(), fee: 1000u128.into() };
+			let leaf_meta = RequestMetadata {
+				offchain: LeafIndexAndPos { leaf_index: 0, pos: 0 },
+				fee: fee_metadata,
+				claimed: false,
+			};
+			RequestCommitments::<Test>::insert(*request, leaf_meta.clone());
+			source_trie.insert(&request_commitment_key, &leaf_meta.encode()).unwrap();
+			dest_trie.insert(&request_receipt_key, &eth_address.encode()).unwrap();
+		}
+		drop(source_trie);
+		drop(dest_trie);
+
+		let mut source_recorder = Recorder::<LayoutV0<KeccakHasher>>::default();
+		let mut dest_recorder = Recorder::<LayoutV0<KeccakHasher>>::default();
+		let source_trie = TrieDBBuilder::<LayoutV0<KeccakHasher>>::new(&source_db, &source_root)
+			.with_recorder(&mut source_recorder)
+			.build();
+		let dest_trie = TrieDBBuilder::<LayoutV0<KeccakHasher>>::new(&dest_db, &dest_root)
+			.with_recorder(&mut dest_recorder)
+			.build();
+
+		let mut keys = vec![];
+		for request in requests.iter() {
+			let request_commitment_key = RequestCommitments::<Test>::storage_key(*request);
+			let request_receipt_key = RequestReceipts::<Test>::storage_key(*request);
+			source_trie.get(&request_commitment_key).unwrap();
+			dest_trie.get(&request_receipt_key).unwrap();
+			keys.push(*request);
+		}
+
+		let source_state_proof = SubstrateStateProof::OverlayProof(StateMachineProof {
+			hasher: HashAlgorithm::Keccak,
+			storage_proof: source_recorder.drain().into_iter().map(|f| f.data).collect::<Vec<_>>(),
+		});
+		let dest_state_proof = SubstrateStateProof::OverlayProof(StateMachineProof {
+			hasher: HashAlgorithm::Keccak,
+			storage_proof: dest_recorder.drain().into_iter().map(|f| f.data).collect::<Vec<_>>(),
+		});
+
+		let host = Ismp::default();
+		for chain in [StateMachine::Kusama(2000), StateMachine::Kusama(2001)] {
+			host.store_state_machine_commitment(
+				StateMachineHeight {
+					id: StateMachineId {
+						state_id: chain,
+						consensus_state_id: MOCK_CONSENSUS_STATE_ID,
+					},
+					height: 1,
+				},
+				StateCommitment {
+					timestamp: 100,
+					overlay_root: Some(if chain == StateMachine::Kusama(2000) {
+						source_root
+					} else {
+						dest_root
+					}),
+					state_root: Default::default(),
+				},
+			)
+			.unwrap();
+			host.store_state_machine_update_time(
+				StateMachineHeight {
+					id: StateMachineId {
+						state_id: chain,
+						consensus_state_id: MOCK_CONSENSUS_STATE_ID,
+					},
+					height: 1,
+				},
+				Duration::from_secs(100),
+			)
+			.unwrap();
+			host.store_challenge_period(
+				StateMachineId { state_id: chain, consensus_state_id: MOCK_CONSENSUS_STATE_ID },
+				0,
+			)
+			.unwrap();
+		}
+		host.store_consensus_state(MOCK_CONSENSUS_STATE_ID, Default::default()).unwrap();
+		host.store_consensus_state_id(MOCK_CONSENSUS_STATE_ID, MOCK_CONSENSUS_CLIENT_ID)
+			.unwrap();
+		host.store_unbonding_period(MOCK_CONSENSUS_STATE_ID, 10_000_000_000).unwrap();
+
+		let beneficiary_address = H256::random();
+
+		// A signature the relayer would have produced for a different source chain.
+		// The byte payload is reused as if captured from that chain and replayed here.
+		let foreign_chain = StateMachine::Kusama(7777);
+		let signature = pair
+			.sign_prehashed(&beneficiary_message(0, foreign_chain, beneficiary_address.as_bytes()))
+			.to_vec();
+
+		let withdrawal_proof = WithdrawalProof {
+			commitments: keys,
+			source_proof: Proof {
+				height: StateMachineHeight {
+					id: StateMachineId {
+						state_id: StateMachine::Kusama(2000),
+						consensus_state_id: MOCK_CONSENSUS_STATE_ID,
+					},
+					height: 1,
+				},
+				proof: source_state_proof.encode(),
+			},
+			dest_proof: Proof {
+				height: StateMachineHeight {
+					id: StateMachineId {
+						state_id: StateMachine::Kusama(2001),
+						consensus_state_id: MOCK_CONSENSUS_STATE_ID,
+					},
+					height: 1,
+				},
+				proof: dest_state_proof.encode(),
+			},
+			beneficiary_details: Some((
+				beneficiary_address.0.to_vec(),
+				Signature::Evm { address: eth_address, signature },
+			)),
+		};
+
+		let result = pallet_ismp_relayer::Pallet::<Test>::accumulate_fees(
+			RuntimeOrigin::none(),
+			withdrawal_proof,
+		);
+
+		assert!(result.is_err(), "replayed signature from a different chain must be rejected");
+		assert_eq!(
+			pallet_ismp_relayer::Fees::<Test>::get(
+				StateMachine::Kusama(2000),
+				beneficiary_address.0.to_vec()
+			),
+			U256::zero()
+		);
 	})
 }