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

DAO Tier Counter Desynchronization in Membership Upgrades

echokly

Summary

In the MembershipFactory contract, when users upgrade their tier membership, the tiers[tierIndex].minted counter is not decremented when tokens are burned. This leads to a permanent desynchronization between the actual number of tokens in circulation and the tracked minted amount, eventually causing tiers to appear full when they still have available capacity.

Vulnerability Details

The issue exists in the upgradeTier function of the MembershipFactory contract. When users upgrade their tier, 2 tokens are burned from their current tier, but the minted counter for that tier is not decremented:

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.");
// Burns 2 tokens but doesn't decrement the minted counter
@> IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
@> IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}

The joinDAO function checks the minted counter against the tier's capacity:

function joinDAO(address daoMembershipAddress, uint256 tierIndex) external {
@> require(daos[daoMembershipAddress].tiers[tierIndex].amount >
daos[daoMembershipAddress].tiers[tierIndex].minted, "Tier full.");
// ... rest of the function
}

This check becomes inaccurate as the minted counter is never decremented during burns.

Impact

The impact is severe for several reasons:

  1. Users can be wrongly prevented from joining tiers that appear full but actually have available capacity

  2. The DAO's membership tracking becomes permanently inaccurate

  3. Tiers can become effectively bricked once enough upgrades have occurred, as the minted counter will show the tier as full

  4. This could lead to loss of revenue for the DAO as new members cannot join tiers that appear full

Proof of Concept

Add this helper functions to MembershipFactory.sol

// Helper function to get tier minted count (you'll need to add this to your contract)
function getTierMinted(address daoMembership, uint256 tierIndex)
public view returns (uint256) {
return daos[daoMembership].tiers[tierIndex].minted;
}
​
// Helper function to get tier amount (you'll need to add this to your contract)
function getTierAmount(address daoMembership, uint256 tierIndex)
public view returns (uint256) {
return daos[daoMembership].tiers[tierIndex].amount;
}
​

Paste the following code in a new File.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import {Test, console} from "forge-std/Test.sol";
import {MembershipFactory} from "../dao/MembershipFactory.sol";
import {IMembershipERC1155} from "../dao/interfaces/IERC1155Mintable.sol";
import {DAOConfig, DAOInputConfig, TierConfig, DAOType, TIER_MAX} from "../dao/libraries/MembershipDAOStructs.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
contract MockERC20 is IERC20 {
mapping(address => uint256) private _balances;
mapping(address => mapping(address => uint256)) private _allowances;
uint256 private _totalSupply;
​
function mint(address account, uint256 amount) public {
_totalSupply += amount;
_balances[account] += amount;
}
​
function totalSupply() external view returns (uint256) { return _totalSupply; }
function balanceOf(address account) external view returns (uint256) { return _balances[account]; }
function transfer(address to, uint256 amount) external returns (bool) {
_balances[msg.sender] -= amount;
_balances[to] += amount;
return true;
}
function allowance(address owner, address spender) external view returns (uint256) { return _allowances[owner][spender]; }
function approve(address spender, uint256 amount) external returns (bool) {
_allowances[msg.sender][spender] = amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
require(_allowances[from][msg.sender] >= amount, "ERC20: insufficient allowance");
_allowances[from][msg.sender] -= amount;
_balances[from] -= amount;
_balances[to] += amount;
return true;
}
}
​
contract MockMembershipERC1155 is IMembershipERC1155 {
mapping(address => mapping(uint256 => uint256)) public balances;
​
function initialize(
string memory _name,
string memory _symbol,
string memory _baseURI,
address _owner,
address _currency
) external {}
​
function mint(address to, uint256 id, uint256 amount) external {
balances[to][id] += amount;
}
​
function burn(address from, uint256 id, uint256 amount) external {
require(balances[from][id] >= amount, "Insufficient balance");
balances[from][id] -= amount;
}
}
​
contract MockCurrencyManager {
mapping(address => bool) public whitelistedCurrencies;
function whitelistCurrency(address currency) external {
whitelistedCurrencies[currency] = true;
}
function isCurrencyWhitelisted(address currency) external view returns (bool) {
return whitelistedCurrencies[currency];
}
}
​
contract MembershipFactoryTest is Test {
MembershipFactory factory;
MockERC20 currency;
MockCurrencyManager currencyManager;
MockMembershipERC1155 membershipNFT;
address owpWallet;
address user1;
address daoMembershipAddr;
​
function setUp() public {
// Setup addresses
owpWallet = makeAddr("owpWallet");
user1 = makeAddr("user1");
​
// Deploy mock contracts
currency = new MockERC20();
currencyManager = new MockCurrencyManager();
membershipNFT = new MockMembershipERC1155();
// Setup currency
currencyManager.whitelistCurrency(address(currency));
​
// Deploy factory
factory = new MembershipFactory(
address(currencyManager),
owpWallet,
"baseURI/",
address(membershipNFT)
);
​
// Setup test DAO
daoMembershipAddr = testCreateDAO();
​
// Setup user balances
currency.mint(user1, 1000 ether);
vm.startPrank(user1);
currency.approve(address(factory), type(uint256).max);
vm.stopPrank();
}
​
function testCreateDAO() internal returns (address) {
// Create tier configs
TierConfig[] memory tierConfigs = new TierConfig[]();
for (uint256 i = 0; i < TIER_MAX; i++) {
uint256 amount = (i == 4) ? 2 : 100;
tierConfigs[i] = TierConfig({
amount: 100,
price: 1 ether,
power: i + 1,
minted: 0
});
}
​
// Create DAO config
DAOInputConfig memory daoConfig = DAOInputConfig({
ensname: "test-dao",
daoType: DAOType.SPONSORED,
currency: address(currency),
maxMembers: 1000,
noOfTiers: TIER_MAX
});
​
// Create DAO
return factory.createNewDAOMembership(daoConfig, tierConfigs);
}
​
​
function testBurnDoesNotDecrementMinted() public {
uint256 fromTierIndex = 5; // Starting at tier 5
vm.startPrank(user1);
console.log("\n1. Initial state for tier 5:");
console.log("Initial tier 5 minted:", factory.getTierMinted(daoMembershipAddr, fromTierIndex));
// Get 2 tokens in tier 5
factory.joinDAO(daoMembershipAddr, fromTierIndex);
factory.joinDAO(daoMembershipAddr, fromTierIndex);
uint256 mintedBeforeBurn = factory.getTierMinted(daoMembershipAddr, fromTierIndex);
console.log("\n2. After minting 2 tokens:");
console.log("Tier 5 minted count:", mintedBeforeBurn); // Should be 2
// Burn the tokens through upgrade
console.log("\n3. Performing upgrade (burns 2 tokens)");
factory.upgradeTier(daoMembershipAddr, fromTierIndex);
uint256 mintedAfterBurn = factory.getTierMinted(daoMembershipAddr, fromTierIndex);
console.log("\n4. After burning 2 tokens:");
console.log("Tier 5 minted count:", mintedAfterBurn); // Still 2, should be 0!
// Prove that minted count wasn't decremented
assertEq(mintedAfterBurn, mintedBeforeBurn,
"Bug not demonstrated: minted count actually changed!");
// Show that this causes issues: the tier appears full even though tokens were burned
uint256 tierAmount = factory.getTierAmount(daoMembershipAddr, fromTierIndex);
console.log("\n5. Tier state:");
console.log("Tier 5 'minted' count:", mintedAfterBurn);
console.log("Tokens were burned but minted count wasn't decremented!");
vm.stopPrank();
}

Test output demonstrates the counter remaining at 2 even after tokens are burned:

Initial tier 5 minted: 0
After minting 2 tokens: 2
After burning 2 tokens: 2

Tools Used

  • Manual code review

Recommended Mitigation

Add minted counter decrementation in the 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);
+ // Decrement the minted counter when burning tokens
+ daos[daoMembershipAddress].tiers[fromTierIndex].minted -= 2;
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
+ // Increment the minted counter for the new tier
+ daos[daoMembershipAddress].tiers[fromTierIndex - 1].minted += 1;
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Appeal created

echokly Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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

Give us feedback!