Make tasks local only.

prdoc/pr_10162.prdoc

@@ -0,0 +1,16 @@
+title: Make tasks local only.
+doc:
+- audience:
+  - Runtime Dev
+  - Node Dev
+  description: |-
+    In frame-system: tasks are now only valid from local source, external source is invalid.
+
+    The reason is that transaction are identified by their provided tag in the transaction pool, tasks provided tag is simply the hash of the tasks. Therefore, depending on how tasks are written, it may be possible to have a very large number of tasks that actually do the same operation but where the hash is different, thus it can be used to spam the transaction pool. A simple solution against this is to accept tasks only from local source.
+
+    Fix: https://github.com/paritytech/polkadot-sdk/issues/9693
+crates:
+- name: frame-support-procedural
+  bump: major
+- name: frame-system
+  bump: major

substrate/frame/support/procedural/src/lib.rs

@@ -1120,10 +1120,9 @@ pub fn composite_enum(_: TokenStream, _: TokenStream) -> TokenStream {
 	pallet_macro_stub()
 }
 
-/// Allows you to define some service work that can be recognized by a script or an
-/// off-chain worker.
+/// Allows you to define some service work that can be recognized by the off-chain worker.
 ///
-/// Such a script can then create and submit all such work items at any given time.
+/// The off-chain worker can then create and submit all such work items at any given time.
 ///
 /// These work items are defined as instances of the `Task` trait (found at
 /// `frame_support::traits::Task`). [`pallet:tasks_experimental`](macro@tasks_experimental) when
@@ -1140,11 +1139,11 @@ pub fn composite_enum(_: TokenStream, _: TokenStream) -> TokenStream {
 /// All of such Tasks are then aggregated into a `RuntimeTask` by
 /// [`construct_runtime`](macro@construct_runtime).
 ///
-/// Finally, the `RuntimeTask` can then used by a script or off-chain worker to create and
-/// submit such tasks via an extrinsic defined in `frame_system` called `do_task`.
+/// Finally, the `RuntimeTask` can then be used by the off-chain worker to create and
+/// submit such tasks via an extrinsic defined in `frame_system` called `do_task` which accepts
+/// unsigned transaction from local source.
 ///
-/// When submitted as unsigned transactions (for example via an off-chain workder), note
-/// that the tasks will be executed in a random order.
+/// When submitted as unsigned transactions, note that the tasks will be executed in a random order.
 ///
 /// ## Example
 #[doc = docify::embed!("examples/proc_main/tasks.rs", tasks_example)]

substrate/frame/support/test/tests/tasks.rs

@@ -133,3 +133,23 @@ fn tasks_work() {
 		assert_eq!(my_pallet_2::SomeStorage::<Runtime, Instance1>::get(), (0, 2));
 	});
 }
+
+#[test]
+fn do_task_unsigned_validation_rejects_external_source() {
+	new_test_ext().execute_with(|| {
+		use frame_support::pallet_prelude::{
+			InvalidTransaction, TransactionSource, TransactionValidityError, ValidateUnsigned,
+		};
+
+		let task = RuntimeTask::MyPallet(my_pallet::Task::<Runtime>::Foo { i: 0u32, j: 2u64 });
+		let call = frame_system::Call::do_task { task };
+
+		assert!(matches!(
+			System::validate_unsigned(TransactionSource::External, &call),
+			Err(TransactionValidityError::Invalid(InvalidTransaction::Call))
+		));
+
+		assert!(System::validate_unsigned(TransactionSource::InBlock, &call).is_ok());
+		assert!(System::validate_unsigned(TransactionSource::Local, &call).is_ok());
+	});
+}

substrate/frame/system/src/lib.rs

@@ -1136,7 +1136,7 @@ pub mod pallet {
 	#[pallet::validate_unsigned]
 	impl<T: Config> sp_runtime::traits::ValidateUnsigned for Pallet<T> {
 		type Call = Call<T>;
-		fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
+		fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity {
 			if let Call::apply_authorized_upgrade { ref code } = call {
 				if let Ok(res) = Self::validate_code_is_authorized(&code[..]) {
 					if Self::can_set_code(&code, false).is_ok() {
@@ -1153,17 +1153,29 @@ pub mod pallet {
 
 			#[cfg(feature = "experimental")]
 			if let Call::do_task { ref task } = call {
-				if task.is_valid() {
-					return Ok(ValidTransaction {
-						priority: u64::max_value(),
-						requires: Vec::new(),
-						provides: vec![T::Hashing::hash_of(&task.encode()).as_ref().to_vec()],
-						longevity: TransactionLongevity::max_value(),
-						propagate: true,
-					})
+				// If valid, the tasks provides the tag: hash of task.
+				// But it is allowed to have many task for a single process, e.g. a task that takes
+				// a limit on the number of item to migrate is valid from 1 to the limit while
+				// actually advancing a single migration process.
+				// In the transaction pool, transaction are identified by their provides tag.
+				// So in order to protect the transaction pool against spam, we only accept tasks
+				// from local source.
+				if source == TransactionSource::InBlock || source == TransactionSource::Local {
+					if task.is_valid() {
+						return Ok(ValidTransaction {
+							priority: u64::max_value(),
+							requires: Vec::new(),
+							provides: vec![T::Hashing::hash_of(&task.encode()).as_ref().to_vec()],
+							longevity: TransactionLongevity::max_value(),
+							propagate: false,
+						})
+					}
 				}
 			}
 
+			#[cfg(not(feature = "experimental"))]
+			let _ = source;
+
 			Err(InvalidTransaction::Call.into())
 		}
 	}