Description
Each DAO can have between 1 to 7 membership tiers, except for sponsored DAOs, which must have exactly 7 tiers. In this project, the highest tier (tier 1) holds the greatest weight, which is 64, while the lowest tier, tier 7, holds the least weight, which is 1.
To maintain this tier structure, each higher tier should cost more than the tier immediately below it. In other words, the cost structure should ensure that:
Tier 1 costs more than Tier 2
Tier 2 costs more than Tier 3
Tier 3 costs more than Tier 4
Tier 4 costs more than Tier 5
Tier 5 costs more than Tier 6
Tier 6 costs more than Tier 7
Currently, there is no check in the MembershipFactory::createNewDAOMembership() and MembershipFactory::updateDaoMembership() functions to enforce this price hierarchy. As a result, it’s possible for a lower tier (e.g., Tier 3) to be priced higher than a higher tier (e.g., Tier 1), which contradicts the intended tier structure and can cause confusion or misalignment in tier-based privileges.
Impact
Without enforcing a hierarchical price structure across DAO tiers, there is a risk of mispricing, which can lead to confusion among users and inconsistency in the intended weight and privilege structure of each tier. Mispricing could also negatively impact the perceived fairness and logic of the DAO membership system.
Proof Of Concept
Add the following function inside MembershipFactory.sol for testing purposes:
function getDAOTiers(address daoAddress) external view returns (TierConfig[] memory) {
return daos[daoAddress].tiers;
}
and run the following POC:
import {Test, console} from "forge-std/Test.sol";
import {MembershipFactory} from "src/dao/MembershipFactory.sol";
import {MembershipERC1155} from "src/dao/tokens/MembershipERC1155.sol";
import { DAOType, DAOConfig, DAOInputConfig, TierConfig } from "src/dao/libraries/MembershipDAOStructs.sol";
import {CurrencyManager} from "src/dao/CurrencyManager.sol";
import {ERC20Mock} from "lib/openzeppelin-contracts/contracts/mocks/token/ERC20Mock.sol";
contract OWPTest is Test {
MembershipFactory DAOFactory;
MembershipERC1155 daoImplementationLogic;
MembershipERC1155 DAO;
CurrencyManager currencyManager;
address owpWallet = makeAddr("owpWallet");
address deployer = makeAddr("deployer");
address daoCreator = makeAddr("daoCreator");
address businessCEO = makeAddr("businessCEO");
address attacker = makeAddr("attacker");
ERC20Mock daoCurrency;
uint256 private constant DAO_MAX_MEMBER_LIMIT = 70;
uint256 private constant DAO_TIER_NUM = 5;
uint256 private daoTierPrice = 80e18;
function setUp() public {
vm.startPrank(deployer);
daoCurrency = new ERC20Mock();
currencyManager = new CurrencyManager();
currencyManager.addCurrency(address(daoCurrency));
daoImplementationLogic = new MembershipERC1155();
DAOFactory = new MembershipFactory(address(currencyManager), owpWallet, "SOME_RANDOM_URI", address(daoImplementationLogic));
vm.stopPrank();
}
function test_inconsistentTierPricing() public {
vm.startPrank(daoCreator);
DAOInputConfig memory daoInputConfig = DAOInputConfig({
ensname: "MY_DAO_NAME",
daoType: DAOType.PUBLIC,
currency: address(daoCurrency),
maxMembers: DAO_MAX_MEMBER_LIMIT,
noOfTiers: DAO_TIER_NUM
});
TierConfig[] memory tierConfigs = new TierConfig[]();
for (uint i = 0; i < tierConfigs.length; i++) {
tierConfigs[i] = TierConfig({
amount: 10,
price: vm.randomUint(1, 1e18),
power: 0,
minted:0
});
}
DAO = MembershipERC1155(DAOFactory.createNewDAOMembership(daoInputConfig, tierConfigs));
TierConfig[] memory tiers = DAOFactory.getDAOTiers(address(DAO));
uint256 tier1price = tiers[0].price;
uint256 tier2price = tiers[1].price;
uint256 tier3price = tiers[2].price;
uint256 tier4price = tiers[3].price;
uint256 tier5price = tiers[4].price;
console.log("");
console.log("inconsistent tier pricing caused by `createNewDAOMembership()` function:");
console.log("Tier 1 Price: ", tier1price);
console.log("Tier 2 Price: ", tier2price);
console.log("Tier 3 Price: ", tier3price);
console.log("Tier 4 Price: ", tier4price);
console.log("Tier 5 Price: ", tier5price);
console.log("");
vm.stopPrank();
vm.startPrank(deployer);
for (uint i = 0; i < tierConfigs.length; i++) {
tierConfigs[i] = TierConfig({
amount: 10,
price: vm.randomUint(2e18, 5e18),
power: 0,
minted:0
});
}
DAOFactory.updateDAOMembership("MY_DAO_NAME", tierConfigs);
tiers = DAOFactory.getDAOTiers(address(DAO));
tier1price = tiers[0].price;
tier2price = tiers[1].price;
tier3price = tiers[2].price;
tier4price = tiers[3].price;
tier5price = tiers[4].price;
console.log("inconsistent tier pricing caused by `updateDAOMembership()` function:");
console.log("Tier 1 Price: ", tier1price);
console.log("Tier 2 Price: ", tier2price);
console.log("Tier 3 Price: ", tier3price);
console.log("Tier 4 Price: ", tier4price);
console.log("Tier 5 Price: ", tier5price);
vm.stopPrank();
}
}
Recommended Mitigation
Implement a check in both the createNewDAOMembership() and updateDaoMembership() functions to ensure each higher tier costs more than the tier immediately below it. This can be achieved by iterating through the defined tiers and verifying that the price of each tier is strictly greater than the price of the next lower tier. This will preserve the intended hierarchy and prevent pricing inconsistencies.
contract MembershipFactory is AccessControl, NativeMetaTransaction {
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.");
}
// enforce maxMembers
uint256 totalMembers = 0;
+ uint256 tiersLength = tierConfigs.length;
- for (uint256 i = 0; i < tierConfigs.length; i++) {
+ for (uint256 i = 0; i < tiersLength; i++) {
totalMembers += tierConfigs[i].amount;
+ if (i != tiersLength - 1) require(tierConfigs[i].price > tierConfigs[i + 1].price);
}
// lets ignore rest of the code...
}
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;
// Preserve minted values and adjust the length of dao.tiers
+ uint256 tiersLength = tierConfigs.length;
- for (uint256 i = 0; i < tierConfigs.length; i++) {
+ for (uint256 i = 0; i < tiersLength; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
+ if (i != tiersLength - 1) require(tierConfigs[i].price > tierConfigs[i + 1].price);
}
// ignore rest of the code...
}
}
