Project

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

Missing ownership verification of the current NFT in the tier upgrade function.

Summary

In the upgradeTier function, there is no check to ensure that the caller (msg.sender) actually owns the NFT at the current tier (fromTierIndex) before performing the burn and mint actions to upgrade to a higher tier. This creates a risk of failure if the user tries to upgrade without owning the NFT in that tier, leading to errors when burning the NFT and potentially other issues.

Vulnerability Details

The upgradeTier function upgrades between membership tiers by calling burn to destroy the NFT at the current tier (fromTierIndex) and then minting a new NFT at a higher tier. However, if the caller does not actually own the NFT at fromTierIndex, the burn action will fail and cause the transaction to error, impacting system stability. This issue could be exploited to create denial of service (DoS) attacks if the transaction is repeatedly reverted.

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/MembershipFactory.sol#L155-L161

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

Impact

If the user does not own the NFT at fromTierIndex and attempts to upgrade, the transaction will revert when the burn fails.

Tools Used

Manual

Recommendations

Check to ensure that the user owns at least one NFT at fromTierIndex.

function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
require(daos[daoMembershipAddress].daoType == DAOType.SPONSORED, "Upgrade not allowed.");
require(daos[daoMembershipAddress].noOfTiers > fromTierIndex, "No higher tier available.");
+ require(IMembershipERC1155(daoMembershipAddress).balanceOf(_msgSender(), fromTierIndex) > 0, "Insufficient NFT balance");
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 1);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
Updates

Lead Judging Commences

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

Support

FAQs

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