Summary

The MembershipFactory::updateDAOMembership function allows updating all types of DAO tier configurations. However, when updating a private or a public DAO with fewer tiers than previously existed, the function fails to preserve tokens minted in the higher tier indexes, leading to token loss.

Link: https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L100-L134

Vulnerability Details

MembershipFactory::updateDAOMembership function allows updating all types of DAO tier configurations but it only processes tiers up to the length of tierConfigs. Any existing tiers beyond tierConfigs.length are not processed. After the loop, all existing tiers are deleted (delete dao.tiers) regardless of whether they were processed or not.
This means that, in the case of private or public DAO, if tierConfigs.length is less than dao.tiers.length, the higher tiers (those beyond tierConfigs.length) are effectively ignored in the loop and subsequently deleted when dao.tiers is reset.

Impact

As confirmed by the sponsor it is an allowed behaviour to update a public or private DAO with a lower number of tiers than the number of tiers already in the DAO but “it should be possible but should also be dependant upon there being no members in these tiers. A proposal should be issued to remove tiers in a DAO not subscribed". But in the system, there isn’t any guarantee that more members join the DAO in one ore more tiers during the voting period, the voting outcome and the voting execution because there isn’t in place a method to halt the members to join the DAO during voting and execution period.

In this scenario, updating a private or a public DAO with fewer tiers than previously existed, the users who hold tokens in the higher tier indexes (beyond tierConfigs.length) that are not included in the update configuration lose their tokens once the update is executed.

Tools Used

Manual review

Recommendations

The solution depends on the project requirements. Some possible scenario can be: considering to include a process that accounting the minted token in the higher tiers index and migrates them into one of the new tiers (for example: in the higher new tier index) or returns the token to the members in the excluded tiers. Or add a method that halt the member to join the joinDAO.