Project

One World
NFTDeFi
15,000 USDC
View results
Submission Details
Severity: medium
Invalid

Missing DAOConfig struct update during tier upgrade

Summary

Members of sponsored DAOs can upgrade their memberships by burning two tokens from their current tier to mint one token in a higher tier. While these changes are applied in the membership contract, they are not reflected in the DAOConfig struct in the factory contract, resulting in a desynchronization between the actual membership distribution and the DAO configuration.

Vulnerability Details

When the tier-specific member counts are not updated in the DAOConfig struct, fewer users may be able to mint tokens in lower tiers than intended, while more users than allowed can mint tokens in higher tiers. This leads to an imbalanced distribution of memberships, deviating from the intended structure set by the DAO. Additionally, the upgrade function does not verify if the new tier has already reached its maximum membership limit, allowing more tokens to be minted than configured.

Impact

The PoC below demonstrates this issue: Alice mints two tier 6 memberships, then upgrades to tier 5. Although the tier-specific token balances are updated in the membership contract, the DAOConfig struct in the factory does not reflect these changes. This inconsistency can disrupt DAO governance by skewing the tier allocations.

See PoC
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Vm, Test} from "forge-std/Test.sol";
import {console2} from "forge-std/console2.sol";
import {DAOConfig, DAOInputConfig, TierConfig, DAOType, TIER_MAX} from "../src/dao/libraries/MembershipDAOStructs.sol";
import {MembershipFactory} from "../src/dao/MembershipFactory.sol";
import {CurrencyManager} from "../src/dao/CurrencyManager.sol";
import {MembershipERC1155} from "../src/dao/tokens/MembershipERC1155.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MembershipFactoryTest is Test {
MembershipFactory membershipFactory;
CurrencyManager currencyManager;
MembershipERC1155 membershipImplementation;
MockToken mockToken;
address alice = makeAddr("alice");
address daoAddress;
function setUp() public {
vm.startPrank(alice);
mockToken = new MockToken("TOKEN", "TOKEN");
membershipImplementation = new MembershipERC1155();
currencyManager = new CurrencyManager();
membershipFactory = new MembershipFactory(
address(currencyManager),
address(this),
"baseURI",
address(membershipImplementation)
);
DAOInputConfig memory inputConfig = DAOInputConfig({
ensname: "TEST",
daoType: DAOType.SPONSORED,
currency: address(mockToken),
maxMembers: 700,
noOfTiers: 7
});
TierConfig[] memory tierConfig = new TierConfig[](); // This should be new TierConfig[](7)
for (uint256 i = 0; i < 7; i++) {
tierConfig[i] = TierConfig({
amount: 10,
price: (7 - i) * 10 ** 18,
power: (7 - i) * 10 ** 18,
minted: 0
});
}
currencyManager.addCurrency(address(mockToken));
daoAddress = membershipFactory.createNewDAOMembership(
inputConfig,
tierConfig
);
vm.stopPrank();
}
/**
* In this test, Alice buys two tier 6 tokens and then upgrade to tier 5
* The amounts are not updated in the factory dao config struct
*/
function testUpgradeTier() public {
deal(address(mockToken), alice, 2 ether);
vm.startPrank(alice);
mockToken.approve(address(membershipFactory), type(uint256).max);
membershipFactory.joinDAO(daoAddress, 6);
membershipFactory.joinDAO(daoAddress, 6);
membershipFactory.upgradeTier(daoAddress, 6);
TierConfig[] memory tierConfig = membershipFactory.tiers(daoAddress);
assertEq(tierConfig[6].minted, 2);
assertEq(tierConfig[5].minted, 0);
assertEq(MembershipERC1155(daoAddress).balanceOf(alice, 6), 0);
assertEq(MembershipERC1155(daoAddress).balanceOf(alice, 5), 1);
}
}
contract MockToken is ERC20 {
constructor(
string memory name_,
string memory symbol_
) ERC20(name_, symbol_) {
_mint(msg.sender, 100 ether);
}
}

Tools Used

Manual review.

Recommendations

Update the tier counts in the DAOConfig struct after each upgrade, and add a validation check to ensure the target tier does not exceed its maximum allowed membership limit.

Updates

Lead Judging Commences

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

Support

FAQs

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