Submission Details
Severity:
H1: Upgrading Membership Tiers Configuration Should Not Be Allowed If Users Already Hold Tokens in Lower Tiers
Summary
If a user has already minted a token for Tier 2 while the DAO has a tier configuration of 0, 1, 2, 3, they will lose access to their token if the administrator upgrades the configuration to include fewer tiers, for example, only Tiers 0 and 1.
Vulnerability Details
To clarify:
The admin creates a DAO with 4 tiers: 0, 1, 2, 3.
Alice joins this DAO at Tier 2, so she has 1 Tier 2 token.
The DAO’s configuration is updated to include only 2 tiers: 0 and 1.
Alice loses access to her minted token.
It should not be possible to reduce the number of tiers if a user has already minted a token at a higher tier. However, it should still be possible to increase the number of tiers.
il nme devrait pas être possible de reduire le niveau de tiers lorsqu'un utilisateur a déja minter un token a un tiers plus élévé , par contre il doit être possible de mettre a jour le tiers a des niveau plus élévé
Impact
The user loses access to their tokens, even if the admin reverts to the original configuration with 4 tiers: 0, 1, 2, and 3.
Tools Used
Copy this code in to test repertory
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;
import {Test, console} from "forge-std/Test.sol";
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import {CurrencyManager} from "../contracts/dao/CurrencyManager.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {OWPERC20} from "../contracts/shared/testERC20.sol";
import {DAOConfig, DAOInputConfig, TierConfig, DAOType} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
contract Finding_01Test is Test {
MembershipFactory membershipFactory;
MembershipERC1155 membershipERC1155;
CurrencyManager currencyManager;
ProxyAdmin proxyAdmin;
IERC20 USDC = IERC20(0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359);
IERC20 WBTC = IERC20(0x1BFD67037B42Cf73acF2047067bd4F2C47D9BfD6);
IERC20 WETH = IERC20(0x7ceB23fD6bC0adD59E62ac25578270cFf1b9f619);
OWPERC20 testERC20;
address owner = address(this);
address alice = address(0x1);
address bob = address(0x2);
address attacker = address(0x3);
address[] addrs;
uint64 constant TIER_MAX = 7;
address private currency1;
address private currency2;
function setUp() public {
uint256 polygonChainid =
vm.createFork("https://polygon-mainnet.g.alchemy.com/v2/XXXXXXXXXXXXXXXXXXX");
vm.selectFork(polygonChainid);
console.log("active fork: ", polygonChainid);
// Deploy Currency Manager
currencyManager = new CurrencyManager();
// Deploy Membership ERC1155 implementation
membershipERC1155 = new MembershipERC1155();
// Deploy Membership Factory
membershipFactory = new MembershipFactory(
address(currencyManager), address(owner), "https://baseuri.com/", address(membershipERC1155)
);
// Deploy Test ERC20
testERC20 = new OWPERC20("OWPERC20", "OWP");
//array of TierConfig
// Initialize Tier configurations
TierConfig[] memory tierConfigs = new TierConfig[]();
tierConfigs[0] = TierConfig({price: 300, amount: 10, minted: 0, power: 12});
tierConfigs[1] = TierConfig({price: 200, amount: 10, minted: 0, power: 6});
tierConfigs[2] = TierConfig({price: 100, amount: 10, minted: 0, power: 3});
DAOConfig memory daoConfig = DAOConfig({
ensname: "testdao.eth",
daoType: DAOType.PUBLIC,
tiers: tierConfigs, // Pass the configured tierConfig array here
currency: address(testERC20),
maxMembers: 100,
noOfTiers: 3
});
DAOInputConfig memory daoInputConfig = DAOInputConfig({
ensname: "testdao.eth",
daoType: DAOType.PUBLIC,
currency: address(testERC20),
maxMembers: 100,
noOfTiers: 3
});
//Give money to alice and bob
testERC20.mint(alice, 1000 * 1e18);
testERC20.mint(bob, 1000 * 1e18);
deal(address(USDC), alice, 1000 * 1e6);
deal(address(USDC), bob, 1000 * 1e6);
deal(address(WBTC), alice, 1000 * 1e8);
deal(address(WBTC), bob, 1000 * 1e8);
deal(address(WETH), alice, 1000 * 1e18);
deal(address(WETH), bob, 1000 * 1e18);
currencyManager.grantRole(currencyManager.ADMIN_ROLE(), alice);
currency1 = address(USDC);
currency2 = address(WBTC);
}
function testUpdateDAOMembershipMinted() public {
//Initialize DAO configurations
DAOInputConfig memory daoInputConfig = DAOInputConfig({
ensname: "testdao.eth",
daoType: DAOType.PUBLIC,
currency: address(WBTC),
maxMembers: 100,
noOfTiers: 4
});
// Initialize Tier configurations with 4 tiers 0 , 1, 2 , 3
TierConfig[] memory tierConfigs = new TierConfig[]();
tierConfigs[0] = TierConfig({price: 700, amount: 10, minted: 0, power: 128});
tierConfigs[1] = TierConfig({price: 600, amount: 10, minted: 0, power: 64});
tierConfigs[2] = TierConfig({price: 500, amount: 10, minted: 0, power: 32});
tierConfigs[3] = TierConfig({price: 400, amount: 10, minted: 0, power: 16});
// Add currency to CurrencyManager
currencyManager.addCurrency(address(WBTC));
// Create new DAO Membership
address daoAddress = membershipFactory.createNewDAOMembership(daoInputConfig, tierConfigs);
//Alice and Bob approve WBTC
vm.prank(alice);
WBTC.approve(address(membershipFactory), 1000000);
vm.prank(bob);
WBTC.approve(address(membershipFactory), 1000000);
// Alice joins DAO 4 times to have 4 tokens tier 2
vm.prank(alice);
membershipFactory.joinDAO(daoAddress, 2);
vm.prank(alice);
membershipFactory.joinDAO(daoAddress, 2);
vm.prank(alice);
membershipFactory.joinDAO(daoAddress, 2);
vm.prank(alice);
membershipFactory.joinDAO(daoAddress, 2);
// Verify that Alice have 4 toekns tier 2
uint256 balance = MembershipERC1155(daoAddress).balanceOf(alice, 2);
assertEq(balance, 4);
// Check that we really have 4 token minted tier 2
TierConfig memory tiersConf = membershipFactory.tiers(daoAddress)[2];
assertEq(tiersConf.minted, 4);
assertEq(tiersConf.price, 500);
assertEq(tiersConf.amount, 10);
//We initate the update of the DAO with new configuration with 2 tiers 0 , 1
TierConfig[] memory tierConfigsUpdate = new TierConfig[]();
tierConfigsUpdate[0] = TierConfig({price: 70, amount: 5, minted: 0, power: 128});
tierConfigsUpdate[1] = TierConfig({price: 60, amount: 5, minted: 0, power: 64});
// Update DAO Membership
membershipFactory.updateDAOMembership("testdao.eth", tierConfigsUpdate);
// Verify that we have new updated configuration for tiers 2
//we also check that Alice still have his token mint , but we get an exception
TierConfig memory tiersConUpdate = membershipFactory.tiers(daoAddress)[2];
}
}
run this code
forge test --mt testUpdateDAOMembershipMinted
We got this result
Encountered 1 failing test in test/Finding_02Test.t.sol:Finding_01Test
[FAIL: panic: array out-of-bounds access (0x32)] testUpdateDAOMembershipMinted() (gas: 1889184)
Recommendations
Add a check to verify if a token has already been minted for a tier; if so, revert.
function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external
onlyRole(EXTERNAL_CALLER) //@audit it should be DEFAULT_ADMIN_ROLE
returns (address)
{
address daoAddress = getENSAddress[ensName];
require(tierConfigs.length <= TIER_MAX, "Invalid tier count.");
require(tierConfigs.length > 0, "Invalid tier count.");
require(daoAddress != address(0), "DAO does not exist.");
DAOConfig storage dao = daos[daoAddress];
if (dao.daoType == DAOType.SPONSORED) {
require(tierConfigs.length == TIER_MAX, "Invalid tier count.");
}
+ for (uint256 i = 0; i < daos[daoAddress].noOfTiers; i++) {
+ if (i >= tierConfigs.length) {
+ require(dao.tiers[i].minted == 0, "Not possible redude tier");
+ }
+ }
uint256 maxMembers = 0;
// Preserve minted values and adjust the length of dao.tiers
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
}
// Reset and update the tiers array
delete dao.tiers;
for (uint256 i = 0; i < tierConfigs.length; i++) {
dao.tiers.push(tierConfigs[i]);
maxMembers += tierConfigs[i].amount;
}
// updating the ceiling limit acc to new data
if (maxMembers > dao.maxMembers) {
dao.maxMembers = maxMembers;
}
dao.noOfTiers = tierConfigs.length;
return daoAddress;
}
Prize pool breakdown
Total prize
15,000 USDC
nSLOC
54128 USDC / LOC
14,000 USDC
1,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.