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

Unable to update DAO membership to lower tier quantity

vladzaev

Summary

When the admin tries to update the DAO membership, the function doesn't compare the number of current tiers with the number of new tiers. This makes impossible to to update DAO membership to lower tier quantity.

Vulnerability Details

First of all let's take a look on the update funtion:

function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external onlyRole(EXTERNAL_CALLER) returns (address) {
address daoAddress = getENSAddress[ensName];
require(tierConfigs.length <= TIER_MAX, "Invalid tier count.");
require(tierConfigs.length > 0, "Invalid tier count.");
require(daoAddress != address(0), "DAO does not exist.");
DAOConfig storage dao = daos[daoAddress];
if(dao.daoType == DAOType.SPONSORED){
require(tierConfigs.length == TIER_MAX, "Invalid tier count.");
}
​
uint256 maxMembers = 0;
​
// Preserve minted values and adjust the length of dao.tiers
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.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;
}

It first performs the necessary checks, then saves the values from the current tier, clears the array and finally adds new values to the empty array.

The problem is that the function does not compare the current number of tiers with the new one.

For example, we create or DAO with 7 tiers. Later we decided that we do not need Tier #7, and call the update function with new 6 tiers instead of 7.

Here the previous minted values will be copied and saved:

// Preserve minted values and adjust the length of dao.tiers
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
}

But current 7th Tiers will be missed from this loop as no 7 tier was added for update. So the array will ount of bound and the function reverts.

Impact

In the event of a requirement, it will not be possible to update the DAO to lower tier quantities, but it is possible to update to higher tier quantities.

Tools Used

Manual review

Recommendations

Consider fixing the number of tiers for each DAO from the moment it is created. Otherwise, it's better to refund or upgrade the user to the next tier in case the tier number shrinks.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

vladzaev Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!