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

insufficient parameter validation in MembershipFactory.sol::updateDAOMembership() leads to data loss of dao tiers and user cannot join the dao tiers

aresaudits

Summary

Vulnerability Details

The `updateDAOMembership` function in the MembershipFactory contract is responsible for updating the tier configurations of a specific DAO.

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.");//@audit-this does not check length is same as previous tier length i.e it only checks wheather it is not zeroo and smaller than seven but it does not check the previous length
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.");
}
//@audit - does not check dao.tiers.length == tierConfigs.length
uint256 maxMembers = 0;
​
// Preserve minted values and adjust the length of dao.tiers
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
}
​
// Reset and update the tiers array
delete dao.tiers;
for (uint256 i = 0; i < tierConfigs.length; i++) {
dao.tiers.push(tierConfigs[i]);
maxMembers += tierConfigs[i].amount;
}
​
// updating the ceiling limit acc to new data
if(maxMembers > dao.maxMembers){
dao.maxMembers = maxMembers;
}
​
dao.noOfTiers = tierConfigs.length;
return daoAddress;
}
​

here this function correctly check all the parameters correctly However, the function lacks a critical check to ensure that the length of the new TierConfig[] array matches the length of the existing tiers i.e dao.noOfTiers. This oversight can lead to unintended data loss or misalignment of tier data.

i.e here

  • The function does not verify that tierConfigs.length matches dao.tiers.length or dao.noOfTiers before updating the tiers.

  • The existing dao.tiers array is deleted without ensuring that the new configuration aligns with the previous one, leading to potential data loss.

let's understand this with short example,

Existing DAO: A DAO has 3 tiers with specific configurations.

  • Update Attempt: update the DAO with a tierConfigs array of 2 tiers.

  • The function deletes the existing 3 tiers and replaces them with the new 2 tiers, resulting in the loss of data for the third tier.

Impact

data of the tier will be completely lost like no of mints and amount which leads to new users cannot join the dao

Tools Used

manual review

Recommendations

Before deleting dao.tiers, add a check to ensure that tierConfigs.length matches dao.tiers.length or dao.noOfTiers.

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];
+ require(dao.noOfTiers == tierConfigs.length)
if(dao.daoType == DAOType.SPONSORED){
require(tierConfigs.length == TIER_MAX, "Invalid tier count.");
}
Updates

Lead Judging Commences

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

Appeal created

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

Support

FAQs

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