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

NFT Tier Minting Status Manipulation Through Array Length Extension

otor

Summary

The [updateDAOMembership()](https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L100) contains a logical flaw in handling the minted values when the new tierConfigs array is longer than the existing dao.tiers array. While the function attempts to preserve existing minted values, it only does so for indices that exist in the original array. Any additional tiers beyond the original length retain their input values, allowing to inject arbitrary minted values which might cause a DOS attack and also allows minted values to start from a non 0.

Impact

  • This makes the tier permanently inaccessible as joinDAO() will always revert

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

  • False representation of tier availability

POC

// Step 1: Create DAO with 3 tiers (PUBLIC type)
DAOInputConfig memory daoConfig = DAOInputConfig({
ensname: "testdao.eth",
daoType: DAOType.PUBLIC,
currency: address(0x1234),
maxMembers: 1000,
noOfTiers: 3
});
​
TierConfig[] memory initialTiers = new TierConfig[]();
// Set up initial tiers with minted = 0 (passes validation)
initialTiers[0] = TierConfig(6400, 640, 0, 64);
initialTiers[1] = TierConfig(3200, 320, 0, 32);
initialTiers[2] = TierConfig(1600, 160, 0, 16);
​
createNewDAOMembership(daoConfig, initialTiers);
​
// Step 2: Update with 5 tiers, setting malicious minted values
TierConfig[] memory updateTiers = new TierConfig[]();
// First 3 will be overwritten with stored values
updateTiers[0] = TierConfig(6400, 640, 0, 64); // Will be reset by dao.tiers[0].minted
updateTiers[1] = TierConfig(3200, 320, 0, 32); // Will be reset by dao.tiers[1].minted
updateTiers[2] = TierConfig(1600, 160, 0, 16); // Will be reset by dao.tiers[2].minted
// These values will persist as they're beyond original array length
updateTiers[3] = TierConfig(800, 80, 400, 8); // Malicious minted value persists
updateTiers[4] = TierConfig(600, 60, 600, 4); // Malicious minted value persists
​
updateDAOMembership("test.eth", updateTiers);

Tools Used

Manual

Recommendations

for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
} else {
tierConfigs[i].minted = 0; //@audit Force new tiers to have minted = 0
}
}
Updates

Lead Judging Commences

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

Appeal created

0xbrivan2 Lead Judge
10 months ago
0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

minted value is not asserted to be zero when adding new tiers

Support

FAQs

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