Broker pallet: fix interlacing

prdoc/pr_2811.prdoc

@@ -0,0 +1,13 @@
+title: "Interlacing removes the region on which it is performed."
+
+doc:
+ - audience: Runtime User
+ description: |
+ The current implementation of the broker pallet does not remove
+ the region on which the interlacing is performed. This can create
+ a vulnerability, as the original region owner is still allowed to 
+ assign a task to the region even after transferring an interlaced
+ part of it.
+
+crates:
+ - name: "pallet-broker"

substrate/frame/broker/src/dispatchable_impls.rs

@@ -234,6 +234,9 @@ impl<T: Config> Pallet<T> {
  ensure!(!pivot.is_void(), Error::<T>::VoidPivot);
  ensure!(pivot != region_id.mask, Error::<T>::CompletePivot);
 
+ // The old region should be removed.
+ Regions::<T>::remove(&region_id);
+
  let one = RegionId { mask: pivot, ..region_id };
  Regions::<T>::insert(&one, &region);
  let other = RegionId { mask: region_id.mask ^ pivot, ..region_id };

substrate/frame/broker/src/tests.rs

@@ -602,6 +602,43 @@ fn interlace_works() {
  });
 }
 
+#[test]
+fn cant_assign_unowned_region() {
+ TestExt::new().endow(1, 1000).execute_with(|| {
+ assert_ok!(Broker::do_start_sales(100, 1));
+ advance_to(2);
+ let region = Broker::do_purchase(1, u64::max_value()).unwrap();
+ let (region1, region2) =
+ Broker::do_interlace(region, Some(1), CoreMask::from_chunk(0, 30)).unwrap();
+
+ // Transfer the interlaced region to account 2.
+ assert_ok!(Broker::do_transfer(region2, Some(1), 2));
+
+ // The initial owner should not be able to assign the non-interlaced region, since they have
+ // just transferred an interlaced part of it to account 2.
+ assert_noop!(Broker::do_assign(region, Some(1), 1001, Final), Error::<Test>::UnknownRegion);
+
+ // Account 1 can assign only the interlaced region that they did not transfer.
+ assert_ok!(Broker::do_assign(region1, Some(1), 1001, Final));
+ // Account 2 can assign the region they received.
+ assert_ok!(Broker::do_assign(region2, Some(2), 1002, Final));
+
+ advance_to(10);
+ assert_eq!(
+ CoretimeTrace::get(),
+ vec![(
+ 6,
+ AssignCore {
+ core: 0,
+ begin: 8,
+ assignment: vec![(Task(1001), 21600), (Task(1002), 36000)],
+ end_hint: None
+ }
+ ),]
+ );
+ });
+}
+
 #[test]
 fn interlace_then_partition_works() {
  TestExt::new().endow(1, 1000).execute_with(|| {