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

Logical issues in upgradeTier function

biakia

Summary

  1. A user can upgrade his tier even when the target tier is at full capacity because there is no check to ensure that the new tier has available capacity for minting before executing the upgrade.

  2. When a user upgrades from one tier to another, the function does not reflect the change in both old and new tier's minted count

Vulnerability Details

In contract MembershipFactory , the joinDAO function allows a user to join a DAO by purchasing a membership NFT, it will check whether the tier is full:

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

However, there is no such check in upgradeTier function:

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);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}

This absence of a safety check permits a user to upgrade their tier even when the target tier is at full capacity (i.e., when the number of minted NFTs equals or exceeds the amount configured for that tier).

In joinDAO function, it will increase the minted count:

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

However, there is no such change in upgradeTier function. When a user upgrades from one tier to another, the function currently burns 2 tokens from the fromTierIndex tier but doesn't decrease the minted count for that old tier. It also mints 1 tokens to the fromTierIndex -1 tier but doesn't increase the minted count for that new tier. This inconsistency can lead to situations where the sum of minted NFTs across tiers exceeds the configured amount, violating the intended constraints and limits imposed on tier capacities.

Impact

It could lead to an erroneous state where a user holds a membership NFT in a tier that should not accept any more members, leading to a violation of the membership structure's intended constraints. It will also lead to situations where the sum of minted NFTs across tiers exceeds the configured amount

Tools Used

Manual Review

Recommendations

Consider following fix:

/// @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.");
require(daos[daoMembershipAddress].tiers[fromTierIndex-1].amount > daos[daoMembershipAddress].tiers[fromTierIndex-1].minted, "Tier full.");
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
daos[daoMembershipAddress].tiers[fromTierIndex].minted -= 2;
daos[daoMembershipAddress].tiers[fromTierIndex-1].minted += 1;
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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