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

Calling `MembershipFactory::upgradeTier` Causes Incorrect Tier Member Count

farismaulana

Summary

The upgradeTier function in the MembershipFactory contract does not update the minted count for the original tier when a user upgrades to a higher tier. This results in the original tier being exhausted even though the user has been upgraded. Consequently, new members cannot join the original tier, leading to a loss of potential users and fees for the DAO.

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, the current tier NFT would be burned but the state variable of daos[membershipNftAddress].tiers is not updated reflecting the amount burned.
the amount burned should be used to reduce the amount of minted of corresponding tiers so new member can claim their places in the original tier but this is not the case, the user upgraded still count toward the original tier instead.

scenario:

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

  2. bob joins DAO on tier 6 (2 nft)

  3. bob upgradeTier

  4. alice wants to joins DAO on tier 6

  5. revert

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_userWhoCallUpgradeTier_wouldStillCountTowardOriginalTierMember() public {
vm.startPrank(alice);
DAOInputConfig memory daoConfig = DAOInputConfig("DAO", DAOType.SPONSORED, address(currency), 1000, 7);
TierConfig[] memory tiersConfig = new TierConfig[]();
// for test purpose, we set amount to 2 for each tiers
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);
// get to tierIndex 5, mint 2 nfts
uint256 tierIndex = 5;
currency.approve(address(membershipFactory), 100e18);
membershipFactory.joinDAO(daoAddress, tierIndex);
membershipFactory.joinDAO(daoAddress, tierIndex);
​
// upgrade tier
membershipFactory.upgradeTier(daoAddress, tierIndex);
​
TierConfig[] memory tiers = membershipFactory.tiers(daoAddress);
// bob mints still count toward tierIndex 5
assertEq(tiers[tierIndex].minted, 2);
vm.stopPrank();
​
vm.startPrank(alice);
// alice wants to get tierIndex 5 nft
tierIndex = 5;
currency.approve(address(membershipFactory), 100e18);
// this would revert, because there is no more tierIndex 5 nft that can be minted
// although bob no longer on tierIndex 5
vm.expectRevert();
membershipFactory.joinDAO(daoAddress, tierIndex);
}
}
​

run the command forge test --mt test_POC_userWhoCallUpgradeTier_wouldStillCountTowardOriginalTierMember the test would pass because the call is reverted.

Impact

the original tier where user upgraded would be exhausted even though the original user is already upgraded. this would cause the DAO cannot have more member other than the originally minted user.
Potentially would cause the protocol lose their opportunity to gain new user and fees.

Tools Used

foundry

Recommendations

add logic to reduce the minted amount of the original tier, also update the next tier amount of 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.");
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);
}
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.