Audit issue 9: Check if max nonce reached in outbound-queue pallet (#…

…1052)

* Check if max nonce reached in outbound-queue pallet

* merge

* review feedback

parachain/pallets/outbound-queue/Cargo.toml

@@ -29,7 +29,7 @@ bridge-hub-common = { path = "../../../polkadot-sdk/cumulus/parachains/runtimes/
 
 snowbridge-core = { path = "../../primitives/core", features = ["serde"], default-features = false }
 snowbridge-outbound-queue-merkle-tree = { path = "merkle-tree", default-features = false }
-ethabi = { git = "https://github.com/Snowfork/ethabi-decode.git", package = "ethabi-decode", branch = "master", default-features = false }
+ethabi = { git = "https://github.com/snowfork/ethabi-decode.git", package = "ethabi-decode", branch = "master", default-features = false }
 
 xcm = { package = "staging-xcm", path = "../../../polkadot-sdk/polkadot/xcm", default-features = false }
 

parachain/pallets/outbound-queue/src/lib.rs

@@ -308,10 +308,16 @@ pub mod pallet {
 			let queued_message: QueuedMessage =
 				versioned_queued_message.try_into().map_err(|_| Unsupported)?;
 
-			let pricing_params = T::PricingParameters::get();
-
-			let next_nonce = Nonce::<T>::get(queued_message.channel_id).saturating_add(1);
+			// Obtain next nonce
+			let nonce = <Nonce<T>>::try_mutate(
+				queued_message.channel_id,
+				|nonce| -> Result<u64, ProcessMessageError> {
+					*nonce = nonce.checked_add(1).ok_or(Unsupported)?;
+					Ok(*nonce)
+				},
+			)?;
 
+			let pricing_params = T::PricingParameters::get();
 			let command = queued_message.command.index();
 			let params = queued_message.command.abi_encode();
 			let max_dispatch_gas =
@@ -321,7 +327,7 @@ pub mod pallet {
 			// Construct the final committed message
 			let message = CommittedMessage {
 				channel_id: queued_message.channel_id,
-				nonce: next_nonce,
+				nonce,
 				command,
 				params,
 				max_dispatch_gas,
@@ -339,12 +345,8 @@ pub mod pallet {
 
 			Messages::<T>::append(Box::new(message));
 			MessageLeaves::<T>::append(message_abi_encoded_hash);
-			Nonce::<T>::set(queued_message.channel_id, next_nonce);
 
-			Self::deposit_event(Event::MessageAccepted {
-				id: queued_message.id,
-				nonce: next_nonce,
-			});
+			Self::deposit_event(Event::MessageAccepted { id: queued_message.id, nonce });
 
 			Ok(true)
 		}

parachain/pallets/outbound-queue/src/test.rs

@@ -99,16 +99,46 @@ fn process_message_yields_on_max_messages_per_block() {
 	})
 }
 
+#[test]
+fn process_message_fails_on_max_nonce_reached() {
+	new_tester().execute_with(|| {
+		let sibling_id = 1000;
+		let channel_id: ChannelId = ParaId::from(sibling_id).into();
+		let origin = AggregateMessageOrigin::Snowbridge(channel_id.into());
+		let message: QueuedMessage = QueuedMessage {
+			id: H256::zero(),
+			channel_id,
+			command: mock_message(sibling_id).command,
+		};
+		let versioned_queued_message: VersionedQueuedMessage = message.try_into().unwrap();
+		let encoded = versioned_queued_message.encode();
+		let mut meter = WeightMeter::with_limit(Weight::MAX);
+
+		Nonce::<Test>::set(channel_id, u64::MAX);
+
+		assert_noop!(
+			OutboundQueue::process_message(&encoded.as_slice(), origin, &mut meter, &mut [0u8; 32]),
+			ProcessMessageError::Unsupported
+		);
+	})
+}
+
 #[test]
 fn process_message_fails_on_overweight_message() {
 	new_tester().execute_with(|| {
 		let sibling_id = 1000;
 		let channel_id: ChannelId = ParaId::from(sibling_id).into();
 		let origin = AggregateMessageOrigin::Snowbridge(channel_id);
-		let message = mock_message(sibling_id).encode();
+		let message: QueuedMessage = QueuedMessage {
+			id: H256::zero(),
+			channel_id,
+			command: mock_message(sibling_id).command,
+		};
+		let versioned_queued_message: VersionedQueuedMessage = message.try_into().unwrap();
+		let encoded = versioned_queued_message.encode();
 		let mut meter = WeightMeter::with_limit(Weight::from_parts(1, 1));
 		assert_noop!(
-			OutboundQueue::process_message(message.as_slice(), origin, &mut meter, &mut [0u8; 32]),
+			OutboundQueue::process_message(&encoded.as_slice(), origin, &mut meter, &mut [0u8; 32]),
 			ProcessMessageError::Overweight(<Test as Config>::WeightInfo::do_process_message())
 		);
 	})