Summary
The upgradeTier function fails to update the minted counter in tier TierConfig when users upgrade their membership tier. While the ERC1155 token transfers are handled correctly, the function doesn't accurately account for minted tokens per tier.
Vulnerability Details
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);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
The function burns 2 tokens from the source tier and mints 1 token in the destination tier, but it doesn't update minted counter when burning and minting. This creates a mismatch between the actual number of tokens in circulation per tier and the number of minted tokens counted in each configuration.
Impact
The missing counter update in upgradeTier allows users to mint more tokens than a tier's maximum capacity while the counter remains unchanged, making it appear as if the tier isn't full. The function doesn't decrement the minted in the source Tier when burning and doesn't increment in the destination Tier when minting. This creates a serious issue where the joinDAO function's check:
require(daos[daoMembershipAddress].tiers[tierIndex].amount > daos[daoMembershipAddress].tiers[tierIndex].minted, "Tier full.");
becomes useless since it depends on this inaccurate minted counter. As a result, there's a mismatch between the actual number of tokens in circulation and the recorded minted amount. This breaks the tier capacity logic which is crucial for maintaining the DAO's membership structure and tier limitations. For example, a tier with a maximum capacity of 10 tokens could end up having many more tokens in circulation while its minted counter still shows 0 (or whatever count caused by joinDAO which doesn't have the same issue), completely bypassing the intended tier capacity restrictions.
Scenario
1- Tier 7 has a max amount of 10 tokens.
2- 3 users join Tier 7 minting them 1 Token each. Tier 7 minted = 3
3- Users need 2 tokens to fulfill the upgrade so they join Tier 7 again. Tier 7 minted = 6
4- Users upgrade from Tier 7 to Tier 6 which successfully burns 2 tokens in the source Tier for 1 in the destination Tier.
5- Minted in Tier 7 is still 6 but it should be 0, and minted in Tier 6 is still 0 but it should be 3.
6- joinDAO still sees minted = 6 < amount = 10
7- 4 users join Tier 7 which updates minted to 10. but in reality it should be 4.
8- Any attempt from users trying to join Tier 7 will revert since joinDAO sees minted == amount causing a DOS.
9- Any upgrade from Tier 7 to Tier 6 won't fix the issue since minted is not updated and it requires Admin intervention which will be tedious especially if it affects multiple Tiers.
POC
The following test demonstrates the issue:
pragma solidity ^0.8.22;
import "lib/forge-std/src/Test.sol";
import "../../contracts/dao/MembershipFactory.sol";
import "../../contracts/dao/tokens/MembershipERC1155.sol";
import "../../contracts/dao/CurrencyManager.sol";
import "../../contracts/dao/libraries/MembershipDAOStructs.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC1155/IERC1155.sol";
contract MockERC20 is ERC20 {
constructor() ERC20("Mock", "MCK") {
_mint(msg.sender, 1000000 * 10**18);
}
}
contract MembershipDAOTest is Test {
MembershipFactory factory;
MembershipERC1155 implementation;
CurrencyManager currencyManager;
MockERC20 token;
address owner = makeAddr("owner");
function setUp() public {
vm.startPrank(owner);
token = new MockERC20();
currencyManager = new CurrencyManager();
currencyManager.addCurrency(address(token));
implementation = new MembershipERC1155();
factory = new MembershipFactory(
address(currencyManager),
owner,
"baseURI/",
address(implementation)
);
factory.grantRole(keccak256("EXTERNAL_CALLER"), owner);
vm.stopPrank();
}
function testUpgradeTierMintedCounter() public {
vm.startPrank(owner);
TierConfig[] memory tiers = new TierConfig[]();
for(uint i = 0; i < 7; i++) {
tiers[i] = TierConfig({
price: 0,
amount: 10,
minted: 0,
power: 1
});
}
DAOInputConfig memory daoConfig = DAOInputConfig({
ensname: "test",
daoType: DAOType.SPONSORED,
currency: address(token),
maxMembers: 700,
noOfTiers: 7
});
address daoAddress = factory.createNewDAOMembership(daoConfig, tiers);
factory.joinDAO(daoAddress, 6);
factory.joinDAO(daoAddress, 6);
TierConfig[] memory initialTiers = factory.tiers(daoAddress);
console.log("=== Initial State ===");
console.log("Tier 6 minted count:", initialTiers[6].minted);
console.log("Tier 5 minted count:", initialTiers[5].minted);
console.log("Tier 6 token balance:", IERC1155(daoAddress).balanceOf(owner, 6));
console.log("Tier 5 token balance:", IERC1155(daoAddress).balanceOf(owner, 5));
console.log("\n=== Performing Upgrade ===");
factory.upgradeTier(daoAddress, 6);
TierConfig[] memory finalTiers = factory.tiers(daoAddress);
console.log("\n=== Final State ===");
console.log("Tier 6 minted count:", finalTiers[6].minted);
console.log("Tier 5 minted count:", finalTiers[5].minted);
console.log("Tier 6 token balance:", IERC1155(daoAddress).balanceOf(owner, 6));
console.log("Tier 5 token balance:", IERC1155(daoAddress).balanceOf(owner, 5));
console.log("\n=== Issue Summary ===");
console.log("Token balances changed but minted counts remained unchanged!");
uint256 tier6BalanceAfter = IERC1155(daoAddress).balanceOf(owner, 6);
uint256 tier5BalanceAfter = IERC1155(daoAddress).balanceOf(owner, 5);
assertEq(tier6BalanceAfter, 0, "Should have 0 tokens in tier 6 after upgrade");
assertEq(tier5BalanceAfter, 1, "Should have 1 token in tier 5 after upgrade");
assertEq(finalTiers[6].minted, 2, "Tier 6 minted count unchanged despite burn");
assertEq(finalTiers[5].minted, 0, "Tier 5 minted count unchanged despite mint");
vm.stopPrank();
}
}
Run with forge test --match-test testUpgradeTierMintedCounter -vvv
Ran 1 test for test/foundry/testUpgradeTierMintedCounter.t.sol:MembershipDAOTest
[PASS] testUpgradeTierMintedCounter() (gas: 1591313)
Logs:
+=== Initial State ===
Tier 6 minted count: 2
Tier 5 minted count: 0
Tier 6 token balance: 2
Tier 5 token balance: 0
+=== Performing Upgrade ===
+=== Final State ===
Tier 6 minted count: 2
Tier 5 minted count: 0
Tier 6 token balance: 0
Tier 5 token balance: 1
+=== Issue Summary ===
Token balances changed but minted counts remained unchanged!
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.43ms (2.09ms CPU time)
Ran 1 test suite in 351.93ms (6.43ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Tools Used
Manual Review
Foundry
Remix IDE
Recommendations
The Admin can fix this issue when it happens by calling updateDAOMembership. but to prevent it from happening, Just like joinDAO, Update the tier minted counter when upgrades are performed:
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.");
+ // Check if there's enough capacity in the destination tier
+ require(daos[daoMembershipAddress].tiers[fromTierIndex - 1].amount >=
+ daos[daoMembershipAddress].tiers[fromTierIndex - 1].minted,
+ "Destination tier is full");
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
+ daos[daoMembershipAddress].tiers[fromTierIndex].minted -= 2;
+ daos[daoMembershipAddress].tiers[fromTierIndex - 1].minted += 1;
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
This mitigation adds a capacity check for the destination tier and updates the minted counter for the source tier after burning and for the destination Tier after minting.
