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

Former members of a tier retain profit-claiming rights despite removal via `updateDAOMembership`

tejaswarambhe

Summary

Members which are ousted from a membership following the MembershipFactory::updateDAOMembership call, are still allowed to claim profits.

Vulnerability Details

The MembershipFactory::updateDAOMembership is used to update the tier configuration of the DAO along with retention of members for those tiers.

​
function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external onlyRole(EXTERNAL_CALLER) returns (address) {
// Rest of the code
​
// Preserve minted values and adjust the length of dao.tiers
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) { <@ // Neglects old tiers when tierConfigs.length >= dao.tiers.length
tierConfigs[i].minted = dao.tiers[i].minted; <@ // Saves old values for the rest
}
}
​
// 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; <@ // Saves old limits
}
​
// updating the ceiling limit acc to new data
if(maxMembers > dao.maxMembers){
dao.maxMembers = maxMembers;
}
​
dao.noOfTiers = tierConfigs.length;
return daoAddress;
}

If we assume that the tier size, which was 7 for a PUBLIC or PRIVATE DAO, gets reduced by 1 as we pass an shorter tierConfigs array, the tier which is being ousted from this membership still carries the ability to claim profits via MembershipERC1155::claimProfit because the shareOf checks for all the hardcoded values.

function shareOf(address account) public view returns (uint256) {
return (balanceOf(account, 0) * 64) +
(balanceOf(account, 1) * 32) +
(balanceOf(account, 2) * 16) +
(balanceOf(account, 3) * 8) +
(balanceOf(account, 4) * 4) +
(balanceOf(account, 5) * 2) +
balanceOf(account, 6); <@ // Assuming this was ousted when updateDAOMembership was called, `shareOf` still consider's it's balance, hence allows profit sharing.
}

Impact

Unfair for other members to share profits with members that are being ousted from the membership.

Tools Used

Manual Review

Recommendations

It is recommended to not hardcode values but pass in the tier length.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge
about 1 year ago
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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