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

Wrong `maxMembers` update on `updateDAOMembership()` function

demorextess

Summary

Wrong maxMembers update on updateDAOMembership() function may cause wrong value for maxMembersin DAO

Vulnerability Details

External caller have an ability to update the configuration of the DAO by calling updateDAOMembership(). It resets the global state variable and overwrite the new configurations into it. While calculation of the maxMembersit's using summation for all the new tier configuration indexes.

for (uint256 i = 0; i < tierConfigs.length; i++) {
require(tierConfigs[i].minted <= tierConfigs[i].amount, "Invalid tier config");
dao.tiers.push(tierConfigs[i]);
maxMembers += tierConfigs[i].amount;
}

Now, maxMemberslocal variable is holding the exact correct max member attribute but while updating, it only updates the dao.maxMembersif the new variable is higher than old one.

// updating the ceiling limit acc to new data
if(maxMembers > dao.maxMembers){
dao.maxMembers = maxMembers;
}

In conclusion, if we reduce the total number of members, it will still hold the old value which is incorrect in this situation.

Scenario:

  1. Current configuration, there are 3 tiers in DAO and each of it has 100 members. So we can have 300 members in total

  2. We don't have any user on tier 1 and we want to remove tier 1.

  3. After removal it won't update the max members attribute

PoC

it("Should update DAO Membership with correct maxMembers", async function () {
​
// Initial state, we have 11 amount
await currencyManager.addCurrency(testERC20.address); // Assume addCurrency function exists in CurrencyManager
TierConfig = [{ price: 300, amount: 11, minted: 0, power: 12 }];
​
await membershipFactory.createNewDAOMembership(DAOConfig, TierConfig);
​
// Update DAO, it will reduce the amount to 10 from 11
const newTierConfig = [{ price: 150, amount: 10, minted: 0, power:4 }];
await membershipFactory.updateDAOMembership("testdao.eth", newTierConfig);
await expect(membershipFactory.daos(membershipERC1155.address).maxMembers).to.equal(10);
});

Impact

Low - Because it doesn't affect the protocol that much. While checking the limit of tiers, we use amountparameter in tiers. So, we can still check the correct max members but in state variables it will store the wrong value.

Tools Used

Manua Review

Recommendations

We can directly set the new maxMembersattribute. It will store accurate value.

- if(maxMembers > dao.maxMembers){
dao.maxMembers = maxMembers;
- }
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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