Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`MembershipFactory.updateDAOMembership()` can break existing membership structures when tierConfigs.length != dao.tiers.length.

kral01

Summary

There is a MembershipFactory.updateDAOMembership()that is used to upgrade the DAO configurations. It can only be called by the `EXTERNAL_CALLER` role. It updates the tier configs, the maxmembers and no of tiers.

Vulnerability Details

From the code we can see that the function knows that tierConfigs.length != dao.tiers.length is possible and it is handling it.\

function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external
onlyRole(EXTERNAL_CALLER)
returns (address)
{
......................................
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) { // handling the difference in length
tierConfigs[i].minted = dao.tiers[i].minted;
}
}
// Reset and update the tiers array
delete dao.tiers;
for (uint256 i = 0; i < tierConfigs.length; i++) {
dao.tiers.push(tierConfigs[i]);
maxMembers += tierConfigs[i].amount;
}
​
// updating the ceiling limit acc to new data
if (maxMembers > dao.maxMembers) {
dao.maxMembers = maxMembers;
}
​
dao.noOfTiers = tierConfigs.length;
return daoAddress;
...........................

But the thing is that it is not handling the difference in length very well.
Lets take an example where the dao.tiers. length == 7 and tierConfigs.length == 6

The dao.noOfTiers is updated to 6 from 7. It looks good at first but after the DAO update, if a user at tier 6 wants to upgrade to higher tier ,

function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
require(daos[daoMembershipAddress].daoType == DAOType.SPONSORED, "Upgrade not allowed.");
require(daos[daoMembershipAddress].noOfTiers >= fromTierIndex + 1, "No higher tier available.");
................................................
}

It will fail at require(daos[daoMembershipAddress].noOfTiers >= fromTierIndex + 1

cos noOfTier = 6 < fromTierIndex = 6 +1 , which will fail the above check, making users unable to upgrade their tier.


Impact

Users will be unable to upgrade their lower tiers to higher ones and will miss out on rewards. And from the perspective of DAO the users are holding invalid tier of tokens even after the update with no way to upgrade their tier.

Keeping it medium impact since action is performed by trusted role while performing intended operation. And in the live walkthrough, the sponsor explicity stated that upgrading dao should not break any existing configurations.

Tools Used

Manual review

Recommendations

The fix for this is kinda tricky or rather ugly.

Dont update the noOfTiers in updateDAOMembership, it is used for validation only in joinDAO and upgradeTier but the sweet thing is that if user tiers to joinDao with a tier that is invalid now, it will fail the check at joinDAO

require(
daos[daoMembershipAddress].tiers[tierIndex].amount > daos[daoMembershipAddress].tiers[tierIndex].minted,
"Tier full."
);

Since the amount field be 0 for invalid DAO. and it will work when user calls upgradeTier .

Updates

Lead Judging Commences

0xbrivan2 Lead Judge
7 months ago
0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

kral01 Submitter
7 months ago
0xbrivan2 Lead Judge
7 months ago
0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.