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

Using A PER LOWEST TIER TOKEN profit instead of the actual total profit in `MembershipERC1155::claimProfit` causes lower profit distribution than intended

x18a6

Summary

The MembershipERC1155 contract has a critical mismatch in how profits are recorded versus how they are calculated for claims, It uses A PER LOWEST TIER TOKEN profit instead of the actual total profit in MembershipERC1155::claimProfit causes lower profit distribution than intended.

Vulnerability Details

sendProfit is assumed to increment totalProfit by the amount of currency distributed. However, totalProfit includes deviding by _totalSupply which makes it A PER LOWEST TIER TOKEN profit not the actual total profit.

contracts/dao/tokens/MembershipERC1155.sol#L189-L200

/// @notice Distributes profits to token holders
/// @param amount The amount of currency to distribute
function sendProfit(uint256 amount) external {
uint256 _totalSupply = totalSupply;
if (_totalSupply > 0) {
totalProfit += (amount * ACCURACY) / _totalSupply; //@audit this is total profit PER LOWEST TIER TOKEN
IERC20(currency).safeTransferFrom(msg.sender, address(this), amount);
emit Profit(amount);
} else {
IERC20(currency).safeTransferFrom(msg.sender, creator, amount); // Redirect profit to creator if no supply
}
}

We say it's PER LOWEST TIER because total supply is the sum of all tokens weighted to the lowest tier token, references:
contracts/dao/tokens/MembershipERC1155.sol#L61
contracts/dao/tokens/MembershipERC1155.sol#L74

When claiming profit, totalProfit is not multiplied back by _totalSupply to get the actual total profit:

contracts/dao/tokens/MembershipERC1155.sol#L142-L150

/// @notice Claim profits accumulated from the profit pool
/// @return profit The amount of profit claimed
function claimProfit() external returns (uint256 profit) {
profit = saveProfit(msg.sender);
require(profit > 0, "No profit available");
savedProfit[msg.sender] = 0;
IERC20(currency).safeTransfer(msg.sender, profit);
emit Claim(msg.sender, profit);
}

Which should be done here:

contracts/dao/tokens/MembershipERC1155.sol#L159C1-L164C6

/// @notice Calculates unsaved profits for an account
/// @param account The account to query
/// @return profit The unsaved profit amount
function getUnsaved(address account) internal view returns (uint256 profit) {
return ((totalProfit - lastProfit[account]) * shareOf(account)) / ACCURACY;
}

Impact

This can lead to incorrect profit distributions which will be much lower than expected.

Tools Used

Manual review.

Recommendations

Consider removing division by _totalSupply from sendProfit or multiplying back by _totalSupply in MembershipERC1155::getUnsaved().

Updates

Lead Judging Commences

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.