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

Potential Exploitation of TierConfig Power and Price in updateDAOMembership by DAO Creator

lamsy

Summary:

The updateDAOMembership function in the MembershipFactory contract allows an administrator to update the tiers of a DAO. However, the function lacks validation for the power and price fields in the TierConfig structure. This oversight can lead to potential miss configuration of the `TierConfig` structure where the administrator can set the power to zero while increasing the price, forcing members to pay more without receiving any benefits in terms of power.

Vulnerability Details:

  • Function: updateDAOMembership

  • Location: MembershipFactory contract

  • Issue: The function does not validate the power and price fields of the TierConfig array. This allows the administrator to set arbitrary values, including zero power and high price, which can be detrimental to DAO members.

Impact:

  • Financial Exploitation: Members may be required to pay a high price for joining within the DAO without receiving any corresponding increase in power or benefits.

  • Misconfiguration of the `TierConfig` structure.

Tools Used: Manual review

POC:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {IMembershipERC1155} from "../contracts/dao/interfaces/IERC1155Mintable.sol";
import {ICurrencyManager} from "../contracts/dao/interfaces/ICurrencyManager.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {TestERC20} from "./mock/MockERC20.sol";
import {
DAOConfig,
DAOInputConfig,
TierConfig,
DAOType,
TIER_MAX
} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import "forge-std/StdCheats.sol";
import {Test, console} from "forge-std/Test.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "lib/forge-std/src/interfaces/IERC1155.sol";
​
​
​
contract MembershipFactoryTest is Test {
MembershipFactory factory;
MockCurrencyManager public currencyManager;
MockMembershipERC1155 public membershipToken;
address admin;
IERC20 public currency;
IERC1155 public erc1155;
address user;
address owpWallet;
// TierConfig[] public tierConfigs;
// TierConfig[] public newTierConfigs;
MembershipERC1155 public membershipTokennew;
​
function setUp() public {
admin = address(this);
user = address(0x123);
owpWallet = address(0x456);
membershipTokennew = new MembershipERC1155();
currency = new MockERC20("TestCurrency", "TC");
currency.approve(address(this), type(uint256).max);
currencyManager = new MockCurrencyManager();
membershipToken = new MockMembershipERC1155();
factory = new MembershipFactory(
address(currencyManager), owpWallet, "https://api.example.com/metadata/", address(membershipToken)
);
}
function testUpdateDAOMembership() public {
currencyManager.whitelistCurrency(address(currency));
vm.startPrank(user);
vm.deal(user, 1 ether);
DAOInputConfig memory daoConfig = DAOInputConfig({
ensname: "testdao.eth",
daoType: DAOType.SPONSORED,
currency: address(currency),
maxMembers: 100,
noOfTiers: 7
});
TierConfig[] memory tierConfigsJoinDao = new TierConfig[]();
tierConfigsJoinDao[0] = TierConfig({amount: 4, price: 1 ether, power: 1, minted: 0});
tierConfigsJoinDao[1] = TierConfig({amount: 4, price: 1 ether, power: 1, minted: 0});
tierConfigsJoinDao[2] = TierConfig({amount: 5, price: 1 ether, power: 1, minted: 0});
tierConfigsJoinDao[3] = TierConfig({amount: 6, price: 1 ether, power: 1, minted: 0});
tierConfigsJoinDao[4] = TierConfig({amount: 7, price: 1 ether, power: 1, minted: 0});
tierConfigsJoinDao[5] = TierConfig({amount: 8, price: 1 ether, power: 1, minted: 0});
tierConfigsJoinDao[6] = TierConfig({amount: 9, price: 1 ether, power: 1, minted: 0});
address daoAddress = factory.createNewDAOMembership(daoConfig, tierConfigsJoinDao);
assertEq(factory.getENSAddress(daoConfig.ensname), daoAddress);
MockERC20(address(currency)).mint(user, 1000 ether);
IERC20(address(currency)).approve(address(factory), 10 ether);
factory.joinDAO(daoAddress, 3);
factory.joinDAO(daoAddress, 3);
uint256 balAfter = IERC1155(address(daoAddress)).balanceOf(user, 3);
console.log("balafter : ", balAfter);
vm.stopPrank();
TierConfig[] memory newTierConfigs = new TierConfig[]();
+ newTierConfigs[0] = TierConfig({amount: 100, price: 1.5 ether, power: 0, minted: 0});
+ newTierConfigs[1] = TierConfig({amount: 40, price: 1.5 ether, power: 0, minted: 0});
+ newTierConfigs[2] = TierConfig({amount: 30, price: 1.5 ether, power: 0, minted: 0});
+ newTierConfigs[3] = TierConfig({amount: 35, price: 1.5 ether, power: 0, minted: 0});
+ newTierConfigs[4] = TierConfig({amount: 50, price: 1.5 ether, power: 0, minted: 0});
+ newTierConfigs[5] = TierConfig({amount: 70, price: 1.5 ether, power: 0, minted: 0});
+ newTierConfigs[6] = TierConfig({amount: 40, price: 1.5 ether, power: 0, minted: 0});
// Admin updates the DAO
address daoAddressUpdate = factory.updateDAOMembership("testdao.eth", newTierConfigs);
vm.startPrank(user);
factory.joinDAO(daoAddressUpdate, 4);
vm.stopPrank();
factory.tiers(daoAddressUpdate);
​
​
// Verify that the tiers were updated correctly
TierConfig memory updatedTier = factory.tiers(daoAddressUpdate)[0];
assertEq(updatedTier.amount, 100);
assertEq(updatedTier.price, 1.5 ether);
assertEq(updatedTier.power, 0);
vm.stopPrank();
}
}
​
contract MockCurrencyManager is ICurrencyManager {
mapping(address => bool) private whitelistedCurrencies;
​
function isCurrencyWhitelisted(address currency) external view override returns (bool) {
return whitelistedCurrencies[currency];
}
​
function whitelistCurrency(address currency) external {
whitelistedCurrencies[currency] = true;
}
​
function addCurrency(address currency) external override {}
​
function removeCurrency(address currency) external override {}
​
function viewWhitelistedCurrencies(uint256 cursor, uint256 size)
external
view
override
returns (address[] memory, uint256)
{}
​
function viewCountWhitelistedCurrencies() external view override returns (uint256) {}
}
​
contract MockMembershipERC1155 is IMembershipERC1155 {
mapping(address => mapping(uint256 => uint256)) private balances;
mapping(address => mapping(address => bool)) private operatorApprovals;
​
function mint(address to, uint256 id, uint256 amount) external override {
balances[to][id] += amount;
}
​
function burn(address from, uint256 id, uint256 amount) external override {
require(balances[from][id] >= amount, "Insufficient balance to burn");
balances[from][id] -= amount;
}
​
function initialize(
string memory _name,
string memory _symbol,
string memory _baseURI,
address _owner,
address _currency
) external override {}
​
function setApprovalForAll(address operator, bool approved) external {
operatorApprovals[msg.sender][operator] = approved;
}
​
function isApprovedForAll(address account, address operator) external view returns (bool) {
return operatorApprovals[account][operator];
}
​
function safeTransferFrom(address from, address to, uint256 id, uint256 amount, bytes calldata data) external {
require(from == msg.sender || operatorApprovals[from][msg.sender], "Not authorized to transfer");
require(balances[from][id] >= amount, "Insufficient balance to transfer");
​
balances[from][id] -= amount;
balances[to][id] += amount;
​
}
​
function transferFrom(address from, address to, uint256 amount) external {
require(from == msg.sender || operatorApprovals[from][msg.sender], "Not authorized to transfer");
require(balances[from][amount] >= amount, "Insufficient balance to transfer");
​
balances[from][amount] -= amount;
balances[to][amount] += amount;
}
​
function balanceOf(address account, uint256 id) external view returns (uint256) {
return balances[account][id];
}
​
function isContract(address account) internal view returns (bool) {
uint256 size;
assembly {
size := extcodesize(account)
}
return size > 0;
}
}
​
​

Recommendations

  1. Validation Checks: Implement validation checks within the updateDAOMembership function to ensure that power and price are set to reasonable and non-zero values. For example:

    for (uint256 i = 0; i < tierConfigs.length; i++) {
    require(tierConfigs[i].power > 0 && tierConfigs[i].power < MAX_POWER , "Invaild Power input" );
    require(tierConfigs[i].price > MIN_PRICE && tierConfigs[i].price < MAX_PRICE , "Invalid Price input.");
    totalMembers += tierConfigs[i].amount;
    }
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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