Submission Details
Severity:
MaxMembers not updated correctly will report wrong values to off chain service
Summary
The updateDAOMembership function only updates the maxMembers value in the DAO configuration if the new value exceeds the current one. This impacts off-chain services that rely on these values for token minting, potentially causing inconsistencies.
Vulnerability Details
For this lets have a look at updateDAOMembership
/home/aman/Desktop/audits/2024-11-one-world/contracts/dao/MembershipFactory.sol:100
100: function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
101: external onlyRole(EXTERNAL_CALLER) returns (address) {
102: address daoAddress = getENSAddress[ensName];
103: require(tierConfigs.length <= TIER_MAX, "Invalid tier count.");
104: require(tierConfigs.length > 0, "Invalid tier count.");
105: require(daoAddress != address(0), "DAO does not exist.");
106: DAOConfig storage dao = daos[daoAddress];
107: if(dao.daoType == DAOType.SPONSORED){
108: require(tierConfigs.length == TIER_MAX, "Invalid tier count.");
109: }
110:
111: uint256 maxMembers = 0;
112:
113: // Preserve minted values and adjust the length of dao.tiers
114: for (uint256 i = 0; i < tierConfigs.length; i++) {
115: if (i < dao.tiers.length) {
116: tierConfigs[i].minted = dao.tiers[i].minted; // @audit : why one resering the minted values here ? what about the amounts
117: }
118: }
119:
120: // Reset and update the tiers array
121: delete dao.tiers;
122: for (uint256 i = 0; i < tierConfigs.length; i++) {
123: dao.tiers.push(tierConfigs[i]);
124: maxMembers += tierConfigs[i].amount;
125: }
126:
127: // updating the ceiling limit acc to new data
128: if(maxMembers > dao.maxMembers){
129: dao.maxMembers = maxMembers;
130: }
131:
132: dao.noOfTiers = tierConfigs.length;
133: return daoAddress;
134: }
At Line 128 it will only update if the new MaxMember>dao.maxMember.
To test this case I have create a foundry based repo. clone https://github.com/amankakar/one-world-fuzz.
contract TestFactory is Test {
MembershipFactory factory;
CurrencyManager manager;
MembershipERC1155 implementation;
bool isSetup;
address newDao;
string ensName;
uint256 maxMemeber;
MockERC20 currency;
function setUp() public {
manager = new CurrencyManager();
implementation = new MembershipERC1155();
factory = new MembershipFactory(
address(manager),
address(this),
"url",
address(implementation)
);
currency = new MockERC20();
currency.approve(address(factory), type(uint256).max);
}
function _createNEwDaoMemeber(
DAOInputConfig memory daoConfig,
TierConfig[] memory tierConfigs
) internal {
// store name for testing
ensName = daoConfig.ensname;
daoConfig.currency = address(currency);
manager.addCurrency(daoConfig.currency);
newDao = factory.createNewDAOMembership(daoConfig, tierConfigs);
}
function _updateDAOMembership(TierConfig[] memory tierConfigs) internal {
factory.updateDAOMembership(ensName, tierConfigs);
}
function _prepareData()
internal
returns (DAOInputConfig memory, TierConfig[] memory)
{
DAOInputConfig memory daoConfig = DAOInputConfig({
ensname: "testdao.eth",
currency: address(this),
maxMembers: 100,
noOfTiers: 2,
daoType: DAOType.PUBLIC // Assuming 0 represents a specific DAO type for testing
});
maxMemeber = daoConfig.maxMembers;
// Assuming TierConfig is already defined somewhere in your contract
TierConfig[] memory tierConfigs = new TierConfig[]();
tierConfigs[0] = TierConfig({
amount: 50,
minted: 0,
price: 200000000,
power: 0
});
tierConfigs[1] = TierConfig({
amount: 50,
minted: 0,
price: 200000000,
power: 0
});
return (daoConfig, tierConfigs);
}
function _prepareTire() internal view returns (TierConfig[] memory) {
TierConfig[] memory tierConfigs = new TierConfig[]();
tierConfigs[0] = TierConfig({
amount: 50,
minted: 5,
price: 200000000,
power: 0
});
return tierConfigs;
}
function _newDaoStored() internal view {
address _newDao = factory.userCreatedDAOs(address(this), ensName);
assert(newDao == _newDao);
}
function test_deploy() public {
(
DAOInputConfig memory daoConfig,
TierConfig[] memory tierConfigs
) = _prepareData();
_createNEwDaoMemeber(daoConfig, tierConfigs);
_newDaoStored();
tierConfigs = _prepareTire();
_updateDAOMembership(tierConfigs);
}
function test_MaxMember() public {
test_deploy();
(
,
DAOType daoType,
address currency,
uint256 maxMembers,
uint256 noOfTiers
) = factory.daos(newDao);
console.log("maxMember", maxMembers);
}
}
and run the test with command : forge test --mt test_MaxMember -vvv
Impact
the off chain and on chain data will not be in sync.
Tools Used
Manual review
Recommendations
update the maxMember in every case and remove the if check.
Prize pool breakdown
Total prize
15,000 USDC
nSLOC
54128 USDC / LOC
14,000 USDC
1,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.