Summary
When updating a DAO membership with the updateDAOMembership function there is no validations made for the new tier prices. This can result in lower tiers having a higher price than higher tiers. Setting the lower tiers price higher than a higher tier breaks the logic of the contract that can be observed in updateTier and shareOf functions. There are also no funds paid back to existing tier members when that tier's price goes down with the update.
Vulnerability Details
The EXTERNAL_CALLER role can update a dao with the updateDAOMembership function.
function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external onlyRole(EXTERNAL_CALLER) 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.");
}
uint256 maxMembers = 0;
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
}
delete dao.tiers;
for (uint256 i = 0; i < tierConfigs.length; i++) {
dao.tiers.push(tierConfigs[i]);
maxMembers += tierConfigs[i].amount;
}
if(maxMembers > dao.maxMembers){
dao.maxMembers = maxMembers;
}
dao.noOfTiers = tierConfigs.length;
return daoAddress;
}
As observed in this function, there are no validations to make sure that the higher tier price is higher than the tiers below it.
Taking a look at 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);
}
It is observed that this function burns 2 tokens of a lower tier to mint 1 token from a higher tier while doing no price adjustments for the users according to these tiers such as refunding or requesting more funds. We can assume that a higher tier is supposed to be worth twice the amount of a lower tier (e.g. if Tier 1 price is 100 USDC, Tier 2 price would be 50 USDC).
Taking a look at the shareOf function, it supports this assumption that is made by observing the upgradeTier function.
function shareOf(address account) public view returns (uint256) {
return (balanceOf(account, 0) * 64) +
(balanceOf(account, 1) * 32) +
(balanceOf(account, 2) * 16) +
(balanceOf(account, 3) * 8) +
(balanceOf(account, 4) * 4) +
(balanceOf(account, 5) * 2) +
balanceOf(account, 6);
}
This shareOf function is used when calculating the users' profit as a DAO member.
function claimProfit() external returns (uint256 profit) {
profit = saveProfit(msg.sender);
require(profit > 0, "No profit available");
savedProfit[msg.sender] = 0;
IERC20(currency).safeTransfer(msg.sender, profit);
emit Claim(msg.sender, profit);
}
function profitOf(address account) external view returns (uint256) {
return savedProfit[account] + getUnsaved(account);
}
function getUnsaved(address account) internal view returns (uint256 profit) {
return ((totalProfit - lastProfit[account]) * shareOf(account)) / ACCURACY;
}
It is observed that, Tier 1 members earn twice the profit of Tier 2 members who earn twice the profit of Tier 3 members etc.
Considering all this, we can assume that Tier 1 membership that gives twice the profit and has twice the voting power should be costing twice the amount of a Tier 2 membership, if not twice, it should cost at least more as upgradeTier function makes it evident that upgrading to Tier 1 from Tier 2 would burn 2 Tier 2 tokens to mint 1 Tier 1 token.
Consider the scenario where the DAO prices are as follows:
Tier 1 = 100 USDC
Tier 2 = 50 USDC
Tier 3 = 25 USDC
These prices would make sense considering the calculations in the upgradeTier and shareOf functions. However, since there are no price validations done in the updateDAOMembership function, a DAO can be upgraded with the prices below:
Tier 1 = 25 USDC
Tier 2 = 100 USDC
Tier 3 = 50 USDC
This scenario would break the logic of the contract as observed with the calculations in the upgradeTier and shareOf functions. Tier 1, having twice the profits and voting power of Tier 2 can cost less than a Tier 2 membership.
This also creates an issue where updated prices will be unfair for users that have joined the DAO before the update as people can now join the same Tier paying less.
Proof of Concept
Following test proves that after the update, second member paid less for the same tier, first member did not get compensated and tier prices does not make sense.
1) 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);
}
}
2) 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;
address daoPublicProxy;
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 testDAOPricesUpdated() 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: 100, power: 1, minted: 0 });
TierConfig[] memory tiersInput2 = new TierConfig[]();
tiersInput2[0] = TierConfig({ amount: 1, price: 1, power: 64, minted: 0 });
tiersInput2[1] = TierConfig({ amount: 2, price: 20, power: 32, minted: 0 });
tiersInput2[2] = TierConfig({ amount: 4, price: 45, power: 16, minted: 0 });
tiersInput2[3] = TierConfig({ amount: 8, price: 15, power: 8, minted: 0 });
tiersInput2[4] = TierConfig({ amount: 16, price: 34, power: 4, minted: 0 });
tiersInput2[5] = TierConfig({ amount: 32, price: 2, power: 2, minted: 0 });
tiersInput2[6] = TierConfig({ amount: 64, price: 10, power: 1, minted: 1 });
DAOInputConfig memory daoPublicInputConfig = DAOInputConfig({
ensname: "daoPublic",
daoType: DAOType.PUBLIC,
currency: address(weth),
maxMembers: 300,
noOfTiers: 7
});
vm.prank(DAOCreator);
daoPublicProxy = membershipFactory.createNewDAOMembership(
daoPublicInputConfig,
tiersInput
);
vm.startPrank(DAOMember);
uint256 DAOMemberWETBalanceBefore = IERC20(weth).balanceOf(DAOMember);
IERC20(weth).approve(address(membershipFactory), 1000e18);
membershipFactory.joinDAO(daoPublicProxy, 6);
uint256 DAOMemberWETBalanceAfter = IERC20(weth).balanceOf(DAOMember);
uint256 DAOMemberPaidForTier7 = DAOMemberWETBalanceBefore - DAOMemberWETBalanceAfter;
vm.stopPrank();
vm.prank(OWPTeam);
membershipFactory.updateDAOMembership("daoPublic", tiersInput2);
vm.startPrank(DAOMember2);
uint256 DAOMember2WETBalanceBefore = IERC20(weth).balanceOf(DAOMember2);
IERC20(weth).approve(address(membershipFactory), 1000e18);
membershipFactory.joinDAO(daoPublicProxy, 6);
uint256 DAOMember2WETBalanceAfter = IERC20(weth).balanceOf(DAOMember2);
uint256 DAOMember2PaidForTier7 = DAOMember2WETBalanceBefore - DAOMember2WETBalanceAfter;
vm.stopPrank();
uint256 DAOMemberLastBalance = IERC20(weth).balanceOf(DAOMember);
assertGt(DAOMemberPaidForTier7, DAOMember2PaidForTier7);
assertEq(DAOMemberWETBalanceAfter, DAOMemberLastBalance);
}
}
Impact
Updating tier prices to amounts that do not match the calculations in the upgradeTier and shareOf functions will cause users to have a gas race where they front-run each other to join the most profitable tiers for its price.
Additionally users that have joined a tier before the update will not get compensated if the tier price goes down.
Impact: High
Likelihood: Low
Tools Used
Manual review
Recommendations
Implement a for loop in the updateDAOMembership function to validate tier prices. An example of this is shown below.
function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external onlyRole(EXTERNAL_CALLER) 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.");
}
uint256 maxMembers = 0;
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
}
delete dao.tiers;
for (uint256 i = 0; i < tierConfigs.length; i++) {
dao.tiers.push(tierConfigs[i]);
maxMembers += tierConfigs[i].amount;
}
for (uint256 i = 1; i < tierConfigs.length; i++) {
require(
tierConfigs[i - 1].price >= 2 * tierConfigs[i].price,
"Each tier must be at least double the price of the next tier."
);
}
Additionally, when a tier price goes down, compensate members that have joined this tier before the update.
