`committee_sizes` is not updated as it should be

[hats-bug-reporter[bot]]

Github username: @rodiontr Twitter username: -- Submission hash (on-chain): 0x7da0d041897fc9a62d2a521d150b4d3fde69b69e5d9d0a988e064577a9c40441 Severity: high

Description: Description\

committee_sizes variable is not updated when calling set_committee()

Attack Scenario\

In the lib.rs, there is a function called set_committee() that effectively can change the committee. However, the new length of the committee is not inserted into committee_sizes like it’s done in the constructor():

https://github.com/Cardinal-Cryptography/most/blob/70ab234cc3322fda82784413f5e0704907a0e1fe/azero/contracts/most/lib.rs#L238-239

  let mut committee_sizes = Mapping::new();
            committee_sizes.insert(committee_id, &(committee.len() as u128));

And how it's done in the set_committee():

self.ensure_owner()?;
            self.ensure_halted()?;
            Self::check_committee(&committee, signature_threshold)?;

            let mut data = self.data()?;

            let committee_id = data.committee_id + 1;
            let mut committee_set = Mapping::new();
            committee.into_iter().for_each(|account| {
                committee_set.insert((committee_id, account), &());
            });

            self.committees = committee_set;
            data.committee_id = committee_id;

            self.data.set(&data);

            self.signature_thresholds
                .insert(committee_id, &signature_threshold);

            Ok(())
        }

As this mapping is used for accounting purposes, all the places where it’s used will refer to outdated size:

            let total_amount = self
                .get_collected_committee_rewards(committee_id)
                .checked_div(
                    self.committee_sizes
                        .get(committee_id)
                        .ok_or(MostError::NoSuchCommittee)?,
                )
                .ok_or(MostError:

Attachments

Provided above.

Recommendation

Add the following line to the set_commitee() function:

  committee_sizes.insert(committee_id, &(committee.len() as u128));

[krzysztofziobro]

Invalid submission: A PoC is required for submission to be considered valid. You can create a new submission that contains a working PoC.

[rodiontr]

Invalid submission: A PoC is required for submission to be considered valid. You can create a new submission that contains a working PoC.

Sorry but do you even read the issue? It says it misses the line. So you can clearly tell if this is the issue or not. I don’t see how PoC can be written here

[krzysztofziobro]

Yes I have, it seems to put the contract in an unwanted state, and that's not enough for high. If you have a PoC that shows why this should be considered high, then provide it in a separate submission.

[rodiontr]

Yes I have, it seems to put the contract in an unwanted state, and that's not enough for high. If you have a PoC that shows why this should be considered high, then provide it in a separate submission.

Ok so you think it’s medium? I just don’t know the validity criteria

[krzysztofziobro]

I think the severity descriptions in the "Scope" tab on the competition site might be helpful: https://app.hats.finance/audit-competitions/most-aleph-zero-bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d/scope

[krzysztofziobro]

No, it's invalid because you marked it as high and did not provide a PoC