staking-async: prevent manual application of cancelled slashes (#9659)

Fix security vulnerability where the permissionless `apply_slash`
extrinsic could be used to manually apply slashes that governance had
cancelled via `cancel_deferred_slash`.

Related issue:
paritytech-secops/srlabs_findings#563

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: sigurpol

prdoc/pr_9659.prdoc

@@ -0,0 +1,8 @@
+title: 'staking-async: prevent manual application of cancelled slashes'
+doc:
+- audience: Runtime Dev
+  description: |-
+      Fix security vulnerability where the permissionless `apply_slash` extrinsic could be used to manually apply slashes that governance had cancelled via `cancel_deferred_slash`.
+crates:
+- name: pallet-staking-async
+  bump: major

substrate/frame/staking-async/src/pallet/impls.rs

@@ -178,6 +178,18 @@ impl<T: Config> Pallet<T> {
 		Self::slashable_balance_of_vote_weight(who, issuance)
 	}
 
+	/// Checks if a slash has been cancelled for the given era and slash parameters.
+	pub(crate) fn check_slash_cancelled(
+		era: EraIndex,
+		validator: &T::AccountId,
+		slash_fraction: Perbill,
+	) -> bool {
+		let cancelled_slashes = CancelledSlashes::<T>::get(&era);
+		cancelled_slashes.iter().any(|(cancelled_validator, cancel_fraction)| {
+			*cancelled_validator == *validator && *cancel_fraction >= slash_fraction
+		})
+	}
+
 	pub(super) fn do_bond_extra(stash: &T::AccountId, additional: BalanceOf<T>) -> DispatchResult {
 		let mut ledger = Self::ledger(StakingAccount::Stash(stash.clone()))?;
 

substrate/frame/staking-async/src/pallet/mod.rs

@@ -1307,6 +1307,8 @@ pub mod pallet {
 		UnappliedSlashesInPreviousEra,
 		/// The era is not eligible for pruning.
 		EraNotPrunable,
+		/// The slash has been cancelled and cannot be applied.
+		CancelledSlash,
 	}
 
 	impl<T: Config> Pallet<T> {
@@ -1322,12 +1324,7 @@ pub mod pallet {
 				);
 
 				// Check if this slash has been cancelled
-				let cancelled_slashes = CancelledSlashes::<T>::get(&active_era);
-				let is_cancelled = cancelled_slashes.iter().any(|(validator, cancel_fraction)| {
-					*validator == key.0 && *cancel_fraction >= key.1
-				});
-
-				if is_cancelled {
+				if Self::check_slash_cancelled(active_era, &key.0, key.1) {
 					crate::log!(
 						debug,
 						"🦹 slash for {:?} in era {:?} was cancelled, skipping",
@@ -2033,7 +2030,8 @@ pub mod pallet {
 		/// slashes.
 		///
 		/// ## Parameters
-		/// - `era`: The staking era for which slashes should be cancelled.
+		/// - `era`: The staking era for which slashes should be cancelled. This is the era where
+		///   the slash would be applied, not the era in which the offence was committed.
 		/// - `validator_slashes`: A list of validator stash accounts and their slash fractions to
 		///   be cancelled.
 		#[pallet::call_index(17)]
@@ -2668,6 +2666,13 @@ pub mod pallet {
 			let _ = ensure_signed(origin)?;
 			let active_era = ActiveEra::<T>::get().map(|a| a.index).unwrap_or_default();
 			ensure!(slash_era <= active_era, Error::<T>::EraNotStarted);
+
+			// Check if this slash has been cancelled
+			ensure!(
+				!Self::check_slash_cancelled(slash_era, &slash_key.0, slash_key.1),
+				Error::<T>::CancelledSlash
+			);
+
 			let unapplied_slash = UnappliedSlashes::<T>::take(&slash_era, &slash_key)
 				.ok_or(Error::<T>::InvalidSlashRecord)?;
 			slashing::apply_slash::<T>(unapplied_slash, slash_era);

substrate/frame/staking-async/src/tests/slashing.rs

@@ -1297,6 +1297,59 @@ fn cancel_all_slashes_with_100_percent() {
 		})
 }
 
+#[test]
+fn apply_slash_rejects_cancelled_slashes() {
+	ExtBuilder::default().slash_defer_duration(2).build_and_execute(|| {
+		// validator 11 has initial balance and is bonded
+		assert_eq!(asset::stakeable_balance::<T>(&11), 1000);
+
+		// Add a slash for validator 11 in era 1 (will be deferred to era 3)
+		add_slash(11);
+		Session::roll_next();
+		let _ = staking_events_since_last_call();
+
+		// Check current era
+		assert_eq!(active_era(), 1);
+
+		// Verify the slash is scheduled for era 3
+		let slash_era = 3;
+		let slash_key = (11, Perbill::from_percent(10), 0);
+		assert!(UnappliedSlashes::<T>::contains_key(&slash_era, &slash_key));
+
+		// Governance cancels this slash
+		assert_ok!(Staking::cancel_deferred_slash(
+			RuntimeOrigin::root(),
+			slash_era,
+			vec![(11, Perbill::from_percent(10))],
+		));
+
+		// Verify the cancellation event was emitted
+		assert_eq!(
+			staking_events_since_last_call(),
+			vec![Event::SlashCancelled { slash_era, validator: 11 }]
+		);
+
+		// Verify the slash is cancelled
+		let cancelled = CancelledSlashes::<T>::get(&slash_era);
+		assert_eq!(cancelled, vec![(11, Perbill::from_percent(10))]);
+
+		// Move to era 3 when the slash would be applied
+		Session::roll_until_active_era(3);
+
+		// Try to manually apply the cancelled slash - this should fail
+		assert_noop!(
+			Staking::apply_slash(RuntimeOrigin::signed(1), slash_era, slash_key),
+			Error::<T>::CancelledSlash
+		);
+
+		// Verify the slash is still in UnappliedSlashes
+		assert!(UnappliedSlashes::<T>::contains_key(&slash_era, &slash_key));
+
+		// Verify no actual slash was applied (balance remains unchanged)
+		assert_eq!(asset::stakeable_balance::<T>(&11), 1000);
+	})
+}
+
 #[test]
 fn proportional_slash_stop_slashing_if_remaining_zero() {
 	ExtBuilder::default().nominate(true).build_and_execute(|| {