Summary
When creating a new DAO with the createNewDAOMembership function, there are no validations done to make sure that higher tier memberships cost more than lower tier memberships. Allowing higher tiers to cost less than lower tiers goes against the functionality of the protocol that can be observed in updateTier and shareOf functions.
Vulnerability Details
Users can create a new DAO with the createNewDAOMembership function.
function createNewDAOMembership(DAOInputConfig calldata daoConfig, TierConfig[] calldata tierConfigs)
external returns (address) {
require(currencyManager.isCurrencyWhitelisted(daoConfig.currency), "Currency not accepted.");
require(daoConfig.noOfTiers == tierConfigs.length, "Invalid tier input.");
require(daoConfig.noOfTiers > 0 && daoConfig.noOfTiers <= TIER_MAX, "Invalid tier count.");
require(getENSAddress[daoConfig.ensname] == address(0), "DAO already exist.");
if (daoConfig.daoType == DAOType.SPONSORED) {
require(daoConfig.noOfTiers == TIER_MAX, "Invalid tier count for sponsored.");
}
uint256 totalMembers = 0;
for (uint256 i = 0; i < tierConfigs.length; i++) {
totalMembers += tierConfigs[i].amount;
}
require(totalMembers <= daoConfig.maxMembers, "Sum of tier amounts exceeds maxMembers.");
TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(
membershipImplementation,
address(proxyAdmin),
abi.encodeWithSignature("initialize(string,string,string,address,address)", daoConfig.ensname, "OWP", baseURI, _msgSender(), daoConfig.currency)
);
DAOConfig storage dao = daos[address(proxy)];
dao.ensname = daoConfig.ensname;
dao.daoType = daoConfig.daoType;
dao.currency = daoConfig.currency;
dao.maxMembers = daoConfig.maxMembers;
dao.noOfTiers = daoConfig.noOfTiers;
for (uint256 i = 0; i < tierConfigs.length; i++) {
require(tierConfigs[i].minted == 0, "Invalid tier config");
dao.tiers.push(tierConfigs[i]);
}
getENSAddress[daoConfig.ensname] = address(proxy);
userCreatedDAOs[_msgSender()][daoConfig.ensname] = address(proxy);
emit MembershipDAONFTCreated(daoConfig.ensname, address(proxy), dao);
return address(proxy);
}
As seen from this function there are no checks or validations done for the prices of tiers.
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.
In cases where higher tier memberships do not cost close to twice the amount of the lower tier membership, not only it will create a race for users to join the most profitable tier. Consider the following scenario where Tier prices are as below:
Tier 1 costs 10 USDC
Tier 2 costs 40 USDC
Tier 3 costs 80 USDC
This situation will create a gas race where users try to front-run each other to buy Tier 1 memberships, all users would avoid Tier 2 and Tier 3 memberships as they don't make sense for the cost. This DAO would only have members for the Tier 1 while rest of the tiers would not be joined as Tier 1 membership that costs 10 USDC would give twice the profits and voting power of a Tier 2 membership that costs 40 USDC, essentially breaking the purpose of the DAO.
Proof of Concept
The following test proves that Tier 7 price can be higher than Tier 1 price
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 and run the following test 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 testTier1LowerPriceThanTier7() public {
TierConfig[] memory tiersInput = new TierConfig[]();
tiersInput[0] = TierConfig({ amount: 1, price: 1, power: 64, minted: 0 });
tiersInput[1] = TierConfig({ amount: 2, price: 20, power: 32, minted: 0 });
tiersInput[2] = TierConfig({ amount: 4, price: 45, power: 16, minted: 0 });
tiersInput[3] = TierConfig({ amount: 8, price: 15, power: 8, minted: 0 });
tiersInput[4] = TierConfig({ amount: 16, price: 34, 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 });
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.startPrank(DAOMember2);
uint256 DAOMember2WETBalanceBefore = IERC20(weth).balanceOf(DAOMember2);
IERC20(weth).approve(address(membershipFactory), 1000e18);
membershipFactory.joinDAO(daoPublicProxy, 0);
uint256 DAOMember2WETBalanceAfter = IERC20(weth).balanceOf(DAOMember2);
uint256 DAOMember2PaidForTier1 = DAOMember2WETBalanceBefore - DAOMember2WETBalanceAfter;
vm.stopPrank();
assertGt(DAOMemberPaidForTier7, DAOMember2PaidForTier1);
}
}
Impact
There will be no reason for users to join tiers that do not make sense for the cost. This will create a gas race where users try to front-run each other to join the most profitable tiers while rest of the tiers are abandoned.
Impact: High
Likelihood: Low
Tools Used
Manual review
Recommendations
Implement a for loop in the createNewDAOMembership function to validate tier prices. An example of this is shown below.
function createNewDAOMembership(DAOInputConfig calldata daoConfig, TierConfig[] calldata tierConfigs)
external returns (address) {
require(currencyManager.isCurrencyWhitelisted(daoConfig.currency), "Currency not accepted.");
require(daoConfig.noOfTiers == tierConfigs.length, "Invalid tier input.");
require(daoConfig.noOfTiers > 0 && daoConfig.noOfTiers <= TIER_MAX, "Invalid tier count.");
require(getENSAddress[daoConfig.ensname] == address(0), "DAO already exists.");
if (daoConfig.daoType == DAOType.SPONSORED) {
require(daoConfig.noOfTiers == TIER_MAX, "Invalid tier count for sponsored.");
}
uint256 totalMembers = 0;
for (uint256 i = 0; i < tierConfigs.length; i++) {
totalMembers += tierConfigs[i].amount;
}
require(totalMembers <= daoConfig.maxMembers, "Sum of tier amounts exceeds maxMembers.");
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."
);
}
