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

Missing Access Control in upgradeTier::MembershipFactory Function

syedrabeet2002

Summary

The upgradeTier function in the MembershipFactory contract lacks proper access control mechanisms. While it checks for the DAO type (SPONSORED), it fails to verify that the caller is the actual owner of the NFTs being upgraded. This allows any address to potentially initiate upgrades for NFTs they don't own.

Vulnerability Details

The mentioned SPONSORED check-in upgradeTier function indeed restricts upgrades to DAOs with a SPONSORED type. However, this check alone does not control who can perform the upgrade; it only restricts the type of DAO that is eligible for an upgrade.

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

Why This Check Doesn't Control Unauthorized Users

This requirement confirms only that the DAO type is SPONSORED, meaning it's a valid target for upgrades. However, it does not ensure that:

  • The caller (_msgSender()) owns the NFT of the tier being upgraded.

  • The caller has specific permission to perform this upgrade action.

What’s Needed for Proper Access Control

To ensure only authorized users can perform the upgrade, you need additional checks that confirm ownership or permission for the caller:

  • Ownership Verification: Confirm that _msgSender() it holds the necessary tier of the NFT within the DAO.

    require(
    IMembershipERC1155(daoMembershipAddress).balanceOf(_msgSender(), fromTierIndex) >= 2,
    "Must own sufficient tokens to upgrade"
    );



This ensures that only the holder of at least two tokens fromTierIndex is eligible to call the upgrade.

  • Caller Verification: In addition to ownership, you may want to ensure that _msgSender() is the only one who can perform this action for their own NFT, preventing others from upgrading on their behalf.

Impact

  • Unauthorized addresses can attempt to upgrade NFTs they don't own

  • Potential confusion and gas waste from failed unauthorized attempts

  • No proper audit trail of who initiated the upgrade

  • Lack of flexibility for authorized operators or approved addresses

Tools Used

Manual Review

Recommendations

Here's an example that includes both the SPONSORED type check and ownership verification:

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.");
// Explicit balance check
require(
IMembershipERC1155(daoMembershipAddress).balanceOf(_msgSender(), fromTierIndex) >= 2,
"Must own sufficient tokens to upgrade"
);
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
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: Design choice

Support

FAQs

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