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

updateDAOMembership() may change dao.maxMembers, which is set by createNewDAOMembership()

zhanmingjing

Summary

updateDAOMembership() may change dao.maxMembers, which is set by createNewDAOMembership()

Vulnerability Details

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/MembershipFactory.sol#L129

when createNewDAOMembership, 'totalMembers <= daoConfig.maxMembers' is checked, and then set 'dao.maxMembers = daoConfig.maxMembers'.

function createNewDAOMembership(DAOInputConfig calldata daoConfig, TierConfig[] calldata tierConfigs)
external returns (address) {
.....
// enforce maxMembers
uint256 totalMembers = 0;
for (uint256 i = 0; i < tierConfigs.length; i++) {
totalMembers += tierConfigs[i].amount;
}
require(totalMembers <= daoConfig.maxMembers, "Sum of tier amounts exceeds maxMembers.");
​
...
dao.maxMembers = daoConfig.maxMembers;
...
}

However, dao.maxMembers could be updated directly when updateDAOMembership().

function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external onlyRole(EXTERNAL_CALLER) returns (address) {
...
// 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.maxMembers is set during createNewDAOMembership, and should keep the value.

Impact

updateDAOMembership() may change dao.maxMembers, which is set by createNewDAOMembership()

Tools Used

manually review

Recommendations

revert if 'maxMembers > dao.maxMembers' when updateDAOMembership.

function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external onlyRole(EXTERNAL_CALLER) returns (address) {
...
// 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;
}*/
require(maxMembers <= dao.maxMembers, "Sum of tier amounts exceeds maxMembers.");
​
...
}
Updates

Lead Judging Commences

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

Support

FAQs

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