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

Lack of total minted member limit verification in `MembershipFactory::updateDAOMembership` allows a scenario where the DAO could issue more memberships than the intended limit

safdie

Summary

The updateDAOMembership function in the DAO Membership Factory contract lacks proper validation to ensure that the total number of already minted memberships across all tiers does not exceed the maxMembers limit after an update. This missing check allows a scenario where the DAO could issue more memberships than the intended limit, potentially diluting membership and impacting DAO integrity.

Vulnerability Details

In the updateDAOMembership function, there is no validation to ensure that the sum of minted memberships across all tiers stays within the maxMembers limit of the DAO. Although maxMembers is recalculated based on the updated tier configurations, there’s no check to ensure that this recalculated limit is compatible with the total of existing minted memberships across all tiers.

The vulnerable code is shown below, where there’s no check to ensure the total of minted memberships does not exceed the maxMembers limit:

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

uint256 maxMembers = 0;
​
// 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 according to new data
if (maxMembers > dao.maxMembers) {
dao.maxMembers = maxMembers;
}

Here, maxMembers is recalculated based on the updated tiers.

No check exists to ensure the sum of minted memberships across tiers does not exceed the recalculated maxMembers.

PoC:

  1. Deploy a DAO with initial tiers and mint some memberships.

  2. Update the DAO to new tier configurations without verifying total minted memberships.

  3. Check if the DAO allows minting memberships beyond maxMembers.

function testExceedMaxMembersOnUpdate() public {
// Arrange - Set up initial DAO with a maxMembers limit
TierConfig;
initialTiers[0] = TierConfig({amount: 10, minted: 8, price: 1 ether});
initialTiers[1] = TierConfig({amount: 15, minted: 12, price: 2 ether});
​
// Deploy DAO with initial tiers
address daoAddress = membershipFactory.createNewDAOMembership(
DAOInputConfig({
ensname: "exampleDao",
daoType: DAOType.OPEN,
currency: address(token),
maxMembers: 25, // Initial limit
noOfTiers: 2
}),
initialTiers
);
​
// Act - Update the DAO with new tier configurations
TierConfig;
newTiers[0] = TierConfig({amount: 5, minted: 8, price: 1 ether}); // Minted exceeds amount
newTiers[1] = TierConfig({amount: 5, minted: 12, price: 2 ether}); // Minted exceeds amount
​
// Attempt to update without verifying total minted vs maxMembers
membershipFactory.updateDAOMembership("exampleDao", newTiers);
​
// Assert - Check if new memberships can still be minted beyond the limit
bool canMintBeyondLimit = false;
try membershipFactory.joinDAO(daoAddress, 0) {
canMintBeyondLimit = true;
} catch {}
​
require(canMintBeyondLimit, "Vulnerability confirmed: Over-minting possible after update.");
}

Impact

Allows more memberships than the maxMembers limit, diluting DAO value and exclusivity.

Functions reliant on maxMembers will behave unexpectedly, affecting DAO management.

More memberships than expected could lead to unintended operational costs and gas inefficiencies.

Tools Used

Manual review.

Recommendations

To mitigate this vulnerability, add a check to verify that the sum of minted memberships across all tiers remains within the recalculated maxMembers value. If the total exceeds maxMembers, reject the update to prevent over-minting.

// Check that existing minted does not exceed new maxMembers
uint256 totalMinted = 0;
for (uint256 i = 0; i < dao.tiers.length; i++) {
totalMinted += dao.tiers[i].minted;
}
require(totalMinted <= maxMembers, "Total minted exceeds new maxMembers limit.");
Updates

Lead Judging Commences

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!