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

Non-existing track of `minted`, leading to DOS `joinDAO()`

deltaxv

Summary

The way minted field for each dao.tiers struct is being handled within the code is creating many issues.

Vulnerability Details

Taking a closer look at how the mintedis being tracked, the state is not handled correctly at all, while some important checks/invariant are relying on this variable.

Notice the line under at MembershipFactory::L167/L168 is the only operation that handle the amount of mintedtoken per tier, when a token is initially minted through MembershipFactory::joinDAO() function.

daos[daoMembershipAddress].tiers[tierIndex].minted += 1;

Here after minting the value is incremented by 1 which is correct so far, but when burning and minting tokens for each tier at upgradeTier(), the minted value is never incremented nor decremented thus leading a discrepancy about the reality of the mint/burn operations that are being initiated for tokens, see MembershipFactory::L167/L168 line under.

IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
// @audit-issue Should safely decrement `Minted` field by 2, for the appropriate `fromTierIndex`
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
// @audit-issue Should increment `Minted` field by 1, for the appropriate `fromTierIndex - 1`

Due to this a lot of issues are being introduced, first of all looking at the check that lies atMembershipFactory::L150

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

It ensures that minted does not exceed the amount field of tokens that can be available for a specific tier. But due to the mentioned issue, the limit will surely be reached at the pace that users are joining the DAO. When the limit is reached the DAO would be permanently DOSed, because minted never decreases even when burning the token thus leaving the check in a wrong state. Even if the admin "burns" tokens minted amount would still stay bricked.

Impact

  • Incorrect state of minted that impact major check.

  • DOS on core functionality for DAOs, joinDAO function affected.

Tools Used

Manual Review

Recommendations

  • Keep consistent track of all operations (mint/burn) and the current circulation of token according to their tiers

/// @notice Allows users to upgrade their tier within a sponsored DAO
/// @param daoMembershipAddress The address of the DAO membership NFT
/// @param fromTierIndex The current tier index of the user
function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
require(daos[daoMembershipAddress].daoType == DAOType.SPONSORED, "Upgrade not allowed.");
require(daos[daoMembershipAddress].noOfTiers >= fromTierIndex + 1, "No higher tier available.");
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
+ daos[daoMembershipAddress].tiers[fromTierIndex].minted -= 2;
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
+ daos[daoMembershipAddress].tiers[fromTierIndex - 1].minted += 1;
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}

  • Highly recommended => the EXTERNAL_CALLER will likely have to burn/mint directly on the MembershipERC1155 contract.

    • Quoting protocol's answer on cyfrin finding 7.3.8:

    One World Project: There is intentionally no process in place for a member to exit the DAO as per business logic.
    They can be removed by burning their Membership NFTs through off-chain process by the EXTERNAL_CALLER

    => Record these action to avoid having an incorrect state.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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

Give us feedback!