[High] Reachable panic via `index out of bounds` in `gated-marketplace` pallet

Summary

An index out of bounds panic can be triggered by a malicious actor calling an extrinsic in the gated-marketplace pallet.

Issue details

The set_up_application helper function can be called with input such that fields.len() > custodian_fields.as_ref().unwrap().1.len() -- for example, if there are elements in fields but not in custodian_fields.1.

In such cases, the function will try to access a non-existent index in custodian_fields.1, throwing an index out of bounds exception. This causes the runtime to panic and could potentially cause a deny-of-service of the chain.

The helper function is directly reachable from the following extrinsics:

For example here:

#[pallet::call_index(3)]
#[pallet::weight(Weight::from_parts(10_000,0) + T::DbWeight::get().writes(1))]
pub fn apply(
    origin: OriginFor<T>,
    marketplace_id: [u8; 32],
    // Getting encoding errors from polkadotjs if an object vector have optional fields
    fields: Fields<T>,
    custodian_fields: Option<CustodianFields<T>>,
) -> DispatchResult {
    let who = ensure_signed(origin)?;

    let (custodian, fields) = Self::set_up_application(fields, custodian_fields);

Risk

Since this can be triggered by a call to an extrinsic, an attacker could cause a validator node to crash by submitting a specific crafted extrinsic and cause them to miss their block authoring slot. This could lead to a denial-of-service of the whole chain with only very low cost requirements for the attacker.

Mitigation

Validate the arguments prior to calling set_up_application, e.g., ensure that custodian_fields.as_ref().unwrap().1.len() == fields.len() holds before calling the function. Note that this change needs to be applied everywhere set_up_application is called. Alternatively, modify set_up_application to gracefully handle errors.

hashed-io / hashed-pallets