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

Tier minted values can be overwritten in `MembershipFactory::updateDAOMembership` allowing more memberships to be minted than intended

safdie

Summary

The updateDAOMembership function in the DAO Membership Factory contract has a flaw that could lead to unintended overwriting of the minted field within each tier during updates. This can lead to discrepancies between actual and recorded membership counts for specific tiers, allowing more memberships to be minted than intended, affecting the integrity of DAO membership management.

Vulnerability Details

In the updateDAOMembership function, the code attempts to preserve previously minted values for each tier by copying them from the old dao.tiers data into the tierConfigs array, before deleting and resetting dao.tiers. However, this logic can fail if the length of tierConfigs does not match the original dao.tiers array length, resulting in possible overwriting or loss of the minted data.

  1. Mismatch in Array Length: If the new tierConfigs array has fewer elements than the original dao.tiers, not all minted values are preserved, causing data loss.

  2. Inadvertent Overwriting: For new tiers, the minted field is left at 0, even if previously minted values exist for some of the updated tiers.

This behavior introduces potential discrepancies that could allow the DAO to mint more memberships than intended by resetting minted counts inadvertently.

The following code snippet from updateDAOMembership demonstrates where the minted data might be unintentionally overwritten:

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

// 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]);
}

  • The loop that copies minted values relies on tierConfigs having the same or greater length as dao.tiers.

  • If tierConfigs is shorter, some minted values are not copied and are lost.

  • This results in the final dao.tiers having unintended zero minted values, which allows memberships to be over-minted.

  1. Deploy a DAO with an initial minted count for each tier.

  2. Update the DAO’s tier configuration using a shorter tierConfigs array than the original dao.tiers.

  3. Verify that the minted values are reset or incorrectly updated, allowing memberships to be over-minted.

function testMintedValuesOverwrite() public {
// Arrange - Deploy a DAO with initial tiers and some minted values
TierConfig;
initialTiers[0] = TierConfig({amount: 10, minted: 5, price: 1 ether});
initialTiers[1] = TierConfig({amount: 10, minted: 3, price: 2 ether});
initialTiers[2] = TierConfig({amount: 10, minted: 7, price: 3 ether});
​
address daoAddress = membershipFactory.createNewDAOMembership(
DAOInputConfig({
ensname: "exampleDao",
daoType: DAOType.OPEN,
currency: address(token),
maxMembers: 25,
noOfTiers: 3
}),
initialTiers
);
​
// Act - Update the DAO with fewer tiers (shorter tierConfigs array)
TierConfig;
newTiers[0] = TierConfig({amount: 10, minted: 0, price: 1 ether}); // minted will be reset to 0
newTiers[1] = TierConfig({amount: 10, minted: 0, price: 2 ether}); // minted will be reset to 0
​
// Call update function without proper minted value preservation
membershipFactory.updateDAOMembership("exampleDao", newTiers);
​
// Assert - Attempt to mint more memberships than intended
bool canOverMint = false;
try membershipFactory.joinDAO(daoAddress, 0) {
canOverMint = true;
} catch {}
​
require(canOverMint, "Vulnerability confirmed: Minted values were reset allowing over-minting.");
}

Impact

Users can mint additional memberships beyond the intended minted limit.

A DAO’s membership data becomes inaccurate, leading to a misrepresentation of membership status and compromising DAO functionality and integrity.

Tools Used

Manual review.

Recommendations

To prevent overwriting of minted values, add validation to ensure that the updated tierConfigs array length matches the existing dao.tiers length. Additionally, validate each tierConfig to ensure the minted value is preserved accurately before overwriting.

// Ensure new tiers match old tiers length to preserve minted values
require(tierConfigs.length == dao.tiers.length, "Tier configuration length mismatch");
​
// Preserve minted values accurately
for (uint256 i = 0; i < tierConfigs.length; i++) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
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!