Project

Summary

When updateDAOMembership is used to decrease the amount of a tier, we can have amount > minted, which generally should be avoided. This can lead to a situation where the maxMembers of a DAO are exceeded, which should never happen.

Vulnerability Details

joinDAO enforces that a full tier cannot be joined anymore, i.e. that the property minted <= amount holds for every tier (https://github.com/Cyfrin/2024-11-one-world/blob/02b59f43981d247caee9aa9ab68d286ce7844a77/contracts/dao/MembershipFactory.sol#L142). As mentioned in the Cyfrin report, there is one edge case where the property may not hold, namely after an upgrade (which is intended according to the report reply).

However, this can also happen in another case, which is not known so far: updateDAOMembership can be used to change the amount parameter of existing tiers, while keeping the minted parameter constant. If the amount is decreased beyond the minted amount, the property will also not hold anymore. This can cause maxMembers to be exceeded, as the new maxMembers in updateDAOMembership is calculated based on the new amount values (https://github.com/Cyfrin/2024-11-one-world/blob/02b59f43981d247caee9aa9ab68d286ce7844a77/contracts/dao/MembershipFactory.sol#L124-L130) and the minted amount for a tier can already be higher at this point.

Impact

It will be possible to have a higher minted amount than the max amount for one tier, which should be avoided (with one exception as mentioned below). This means that maxMembers can be exceeded, i.e. there can be more members in the whole membership, which should always be exceeded.

For instance, consider a membership with amount limits [10, 10, 10, 10, 10, 10]. The current minted for the tiers is [8, 8, 8, 8, 8, 8]. Now the, amount limits are updated and a new 7th tier is introduced, resulting in [5, 5, 5, 5, 5, 5, 50]. The new maxMembers is 80. But after 50 people have joined the 7th tier (which will succeed), the minted will be [8, 8, 8, 8, 8, 8, 50] and the number of members therefore 98, which is higher than 80.

Recommendations

Disallow decreasing the amount parameter if the new parameter will be greater than the current minted parameter.