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

Vulnerability in `MembershipFactory::createNewDAOMembership` Allows Creation of DAO Memberships with Zero Prices

farismaulana

Summary

The createNewDAOMembership function in the MembershipFactory contract does not validate the price parameter in the tierConfigs struct, allowing the creation of DAO memberships with zero prices. This vulnerability can lead to a loss of potential income for the protocol as users can join DAOs for free.

Vulnerability Details

MembershipFactory.sol#L55-L94

MembershipFactory::createNewDAOMembership does not validate tierConfigs parameters besides the minted value and length of the struct.
This flaw makes possible to create new DAO Membership with zero prices.

add this code to test folder:

ERC20Mock.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract ERC20Mock is ERC20 {
constructor(string memory name, string memory symbol, uint256 initialSupply) ERC20(name, symbol) {
_mint(msg.sender, initialSupply);
}
​
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
​
function burn(address from, uint256 amount) public {
_burn(from, amount);
}
}
​

Base.t.sol:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {CurrencyManager} from "../contracts/dao/CurrencyManager.sol";
import {DAOType, DAOConfig, DAOInputConfig, TierConfig} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import {Test} from "lib/forge-std/src/Test.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
import {console2} from "lib/forge-std/src/console2.sol";
​
contract Base is Test {
string baseURI = "0x";
address owpWallet = makeAddr("owpWallet");
address owner = makeAddr("owner");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address charlie = makeAddr("charlie");
​
MembershipERC1155 membershiImplementation;
MembershipFactory membershipFactory;
CurrencyManager currencyManager;
ERC20Mock currency;
​
event UserJoinedDAO(address indexed user, address indexed membershipNftAddress, uint256 tierIndex);
​
function setUp() public {
vm.startPrank(owner);
membershiImplementation = new MembershipERC1155();
currencyManager = new CurrencyManager();
currency = new ERC20Mock("Currency", "CUR", 100e18);
​
membershipFactory =
new MembershipFactory(address(currencyManager), owpWallet, baseURI, address(membershiImplementation));
​
currencyManager.addCurrency(address(currency));
​
currency.mint(alice, 100e18);
currency.mint(bob, 100e18);
currency.mint(charlie, 100e18);
vm.stopPrank();
}
​
function test_POC_createDAOWithZeroPrice() public {
vm.startPrank(alice);
DAOInputConfig memory daoConfig = DAOInputConfig("DAO", DAOType.PUBLIC, address(currency), 100, 3);
TierConfig[] memory tiersConfig = new TierConfig[]();
// amount 10, price 0, power 30, minted 0
tiersConfig[0] = TierConfig(10, 0, 30, 0);
// amount 10, price 0, power 20, minted 0
tiersConfig[1] = TierConfig(10, 0, 20, 0);
// amount 10, price 0, power 10, minted 0
tiersConfig[2] = TierConfig(10, 0, 10, 0);
​
address daoAddress = membershipFactory.createNewDAOMembership(daoConfig, tiersConfig);
vm.stopPrank();
​
vm.startPrank(bob);
uint256 balanceProtocolBefore = currency.balanceOf(owpWallet);
uint256 balanceBobBefore = currency.balanceOf(bob);
​
membershipFactory.joinDAO(daoAddress, 0);
membershipFactory.joinDAO(daoAddress, 1);
membershipFactory.joinDAO(daoAddress, 2);
​
uint256 balanceProtocolAfter = currency.balanceOf(owpWallet);
uint256 balanceBobAfter = currency.balanceOf(bob);
​
// no change in balance of either protocol or bob
assertEq(balanceProtocolAfter - balanceProtocolBefore, 0);
assertEq(balanceBobAfter - balanceBobBefore, 0);
}
}
​

run the test using forge test --mt test_POC_createDAOWithZeroPrice and the test would pass indicating user and protocol does not have any balance change because the fee of joining DAO is 0.

Impact

because the main resource of protocol fee is from user joining a DAO, this would create a loss of potential income, and user can use the feature for free.

Tools Used

foundry

Recommendations

add a check for price value in the createNewDAOMembership function.
we can use greater than zero (example below) or enforce a minimum price for each tier of DAO using another variables.

diff --git a/contracts/dao/MembershipFactory.sol b/contracts/dao/MembershipFactory.sol
index 02205e3..6bc618a 100644
--- a/contracts/dao/MembershipFactory.sol
+++ b/contracts/dao/MembershipFactory.sol
function createNewDAOMembership(DAOInputConfig calldata daoConfig, TierConfig[] calldata tierConfigs)
external returns (address) {
.
.
.
DAOConfig storage dao = daos[address(proxy)];
@@ -84,10 +100,13 @@ contract MembershipFactory is AccessControl, NativeMetaTransaction {
​
for (uint256 i = 0; i < tierConfigs.length; i++) {
require(tierConfigs[i].minted == 0, "Invalid tier config");
+ require(tierConfigs[i].price > 0, "Invalid tier config");
dao.tiers.push(tierConfigs[i]);
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Appeal created

farismaulana Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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

Give us feedback!