Merge pull request #41 from hashed-io/feature/benchmark/ticket/29

Out of bounds marketplace application

Author: sebastianmontero

pallets/gated-marketplace/src/functions.rs

@@ -135,7 +135,7 @@ impl<T: Config> Pallet<T> {
 		// ensure the origin is owner or admin
 		Self::is_authorized(authority.clone(), &marketplace_id, Permission::Enroll)?;
 
-		let (custodian, fields) = Self::set_up_application(fields, custodian_fields);
+		let (custodian, fields) = Self::set_up_application(fields, custodian_fields)?;
 
 		let application = Application::<T> {
 			status: ApplicationStatus::default(),
@@ -794,11 +794,12 @@ impl<T: Config> Pallet<T> {
 	}
 
 	/* ---- Helper functions ---- */
-
 	pub fn set_up_application(
 		fields: Fields<T>,
 		custodian_fields: Option<CustodianFields<T>>,
-	) -> (Option<T::AccountId>, BoundedVec<ApplicationField, T::MaxFiles>) {
+	) -> Result<(Option<T::AccountId>, BoundedVec<ApplicationField, T::MaxFiles>), DispatchError> {	
+		ensure!(!fields.is_empty(), Error::<T>::FieldsNotProvided);
+
 		let mut f: Vec<ApplicationField> = fields
 			.iter()
 			.map(|tuple| ApplicationField {
@@ -807,17 +808,23 @@ impl<T: Config> Pallet<T> {
 				custodian_cid: None,
 			})
 			.collect();
+
 		let custodian = match custodian_fields {
 			Some(c_fields) => {
+				if fields.len() != c_fields.1.len() {
+					return Err(Error::<T>::InsufficientCustodianFields.into())
+				}
 				for (i, field) in f.iter_mut().enumerate() {
 					field.custodian_cid = Some(c_fields.1[i].clone());
 				}
-
 				Some(c_fields.0)
 			},
 			_ => None,
 		};
-		(custodian, BoundedVec::<ApplicationField, T::MaxFiles>::try_from(f).unwrap_or_default())
+
+		let fields = BoundedVec::<ApplicationField, T::MaxFiles>::try_from(f).
+			map_err(|_| Error::<T>::ExceedMaxFilesApplication)?;
+		Ok((custodian, fields))
 	}
 
 	fn insert_in_auth_market_lists(

pallets/gated-marketplace/src/lib.rs

@@ -338,6 +338,12 @@ pub mod pallet {
 		OwnerNotInMarketplace,
 		/// MappedAssetId not found
 		AssetNotFound,
+		/// Not enough custodian fields provided
+		InsufficientCustodianFields,
+		/// Fields not provided for the application
+		FieldsNotProvided,
+		/// Exceeds the maximum number of files
+		ExceedMaxFilesApplication,
 	}
 
 	#[pallet::call]
@@ -435,7 +441,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			let who = ensure_signed(origin)?;
 
-			let (custodian, fields) = Self::set_up_application(fields, custodian_fields);
+			let (custodian, fields) = Self::set_up_application(fields, custodian_fields)?;
 
 			let application = Application::<T> {
 				status: ApplicationStatus::default(),
@@ -473,7 +479,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			let who = ensure_signed(origin)?;
 
-			let (custodian, fields) = Self::set_up_application(fields, custodian_fields);
+			let (custodian, fields) = Self::set_up_application(fields, custodian_fields)?;
 
 			let application = Application::<T> {
 				status: ApplicationStatus::default(),

pallets/gated-marketplace/src/mock.rs

@@ -1,5 +1,4 @@
 use crate as pallet_gated_marketplace;
-use frame_system::RawOrigin;
 use sp_runtime::traits::Lookup;
 
 use frame_support::{
@@ -22,6 +21,7 @@ use sp_runtime::{
 use system::EnsureSigned;
 type AccountId = u64;
 type AssetId = u32;
+
 // Configure a mock runtime to test the pallet.
 frame_support::construct_runtime!(
   pub enum Test
@@ -163,24 +163,25 @@ impl pallet_balances::Config for Test {
 }
 
 parameter_types! {
-  pub const MaxScopesPerPallet: u32 = 2;
-  pub const MaxRolesPerPallet: u32 = 6;
-  pub const RoleMaxLen: u32 = 25;
-  pub const PermissionMaxLen: u32 = 25;
-  pub const MaxPermissionsPerRole: u32 = 30;
-  pub const MaxRolesPerUser: u32 = 2;
-  pub const MaxUsersPerRole: u32 = 2;
+	pub const MaxScopesPerPallet: u32 = 2;
+	pub const MaxRolesPerPallet: u32 = 6;
+	pub const RoleMaxLen: u32 = 25;
+	pub const PermissionMaxLen: u32 = 25;
+	pub const MaxPermissionsPerRole: u32 = 30;
+	pub const MaxRolesPerUser: u32 = 2;
+	pub const MaxUsersPerRole: u32 = 2;
 }
 impl pallet_rbac::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
+	type RemoveOrigin = EnsureRoot<Self::AccountId>;
 	type MaxScopesPerPallet = MaxScopesPerPallet;
 	type MaxRolesPerPallet = MaxRolesPerPallet;
 	type RoleMaxLen = RoleMaxLen;
 	type PermissionMaxLen = PermissionMaxLen;
 	type MaxPermissionsPerRole = MaxPermissionsPerRole;
 	type MaxRolesPerUser = MaxRolesPerUser;
 	type MaxUsersPerRole = MaxUsersPerRole;
-	type RemoveOrigin = EnsureRoot<Self::AccountId>;
+	type WeightInfo = ();
 }
 
 // Build genesis storage according to the mock runtime.

pallets/gated-marketplace/src/tests.rs

@@ -244,6 +244,33 @@ fn apply_to_marketplace_works() {
 	});
 }
 
+#[test]
+fn apply_to_marketplace_without_fields_shouldnt_work() {
+	new_test_ext().execute_with(|| {
+		Balances::make_free_balance_be(&1, 100);
+		// Dispatch a signed extrinsic.
+		let m_label = create_label("my marketplace");
+		assert_ok!(GatedMarketplace::create_marketplace(
+			RuntimeOrigin::signed(1),
+			2,
+			m_label.clone(),
+			500,
+			600,
+			1,
+		));
+		let m_id = get_marketplace_id("my marketplace", 500, 600, 1);
+		assert_noop!(
+			GatedMarketplace::apply(
+				RuntimeOrigin::signed(3),
+				m_id,
+				create_application_fields(0),
+				None
+			),
+			Error::<Test>::FieldsNotProvided
+		);
+	});
+}
+
 #[test]
 fn apply_with_custodian_works() {
 	new_test_ext().execute_with(|| {
@@ -273,6 +300,60 @@ fn apply_with_custodian_works() {
 	});
 }
 
+#[test]
+fn apply_with_mismatched_number_of_fields_and_custodian_fields_shouldnt_work() {
+	new_test_ext().execute_with(|| {
+		Balances::make_free_balance_be(&1, 100);
+		// Dispatch a signed extrinsic.
+		let m_label = create_label("my marketplace");
+		assert_ok!(GatedMarketplace::create_marketplace(
+			RuntimeOrigin::signed(1),
+			2,
+			m_label.clone(),
+			500,
+			600,
+			1,
+		));
+		let m_id = get_marketplace_id("my marketplace", 500, 600, 1);
+		assert_noop!(
+			GatedMarketplace::apply(
+				RuntimeOrigin::signed(3),
+				m_id,
+				create_application_fields(2),
+				create_custodian_fields(4, 1)
+			),
+			Error::<Test>::InsufficientCustodianFields
+		);
+	});
+}
+
+#[test]
+fn apply_with_custodian_but_no_custodian_fields_shouldnt_work() {
+	new_test_ext().execute_with(|| {
+		Balances::make_free_balance_be(&1, 100);
+		// Dispatch a signed extrinsic.
+		let m_label = create_label("my marketplace");
+		assert_ok!(GatedMarketplace::create_marketplace(
+			RuntimeOrigin::signed(1),
+			2,
+			m_label.clone(),
+			500,
+			600,
+			1,
+		));
+		let m_id = get_marketplace_id("my marketplace", 500, 600, 1);
+		assert_noop!(
+			GatedMarketplace::apply(
+				RuntimeOrigin::signed(3),
+				m_id,
+				create_application_fields(2),
+				create_custodian_fields(4, 0)
+			),
+			Error::<Test>::InsufficientCustodianFields
+		);
+	});
+}
+
 #[test]
 fn apply_with_same_account_as_custodian_shouldnt_work() {
 	new_test_ext().execute_with(|| {
@@ -378,6 +459,7 @@ fn apply_twice_shouldnt_work() {
 			1,
 		));
 		let m_id = get_marketplace_id("my marketplace", 500, 600, 1);
+
 		assert_ok!(GatedMarketplace::apply(
 			RuntimeOrigin::signed(3),
 			m_id,