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

Profit distribution flaw allowing imbalanced profit allocation in `MembershipERC1155::sendProfit`

safdie

Summary

The MembershipERC1155 contract contains a vulnerability within the sendProfit function that allows for inaccurate profit distribution among token holders due to improper handling of the weighted share calculation. The use of totalSupply without appropriate checks on token weighting can result in users receiving profits that do not accurately represent their holdings, leading to an unfair profit distribution.

Vulnerability Details

The sendProfit function calculates profit shares based on totalSupply but does not accurately account for the unique weighting of each token ID. Since each token ID has a different weight, profits calculated solely on totalSupply may not align with the actual weighted value of token holdings across all users. This mismatch can result in over- or under-allocation of profits to token holders, compromising fair distribution.

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/tokens/MembershipERC1155.sol#L191-L200

function sendProfit(uint256 amount) external {
uint256 _totalSupply = totalSupply;
if (_totalSupply > 0) {
totalProfit += (amount * ACCURACY) / _totalSupply;
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
}
}

The following test demonstrates the vulnerability by comparing the expected profit distribution using the token weights against the flawed distribution produced by the sendProfit function. In this example, a user holding a heavily weighted token (e.g., ID 0 with weight 64) should receive a larger share of the profit, but the current code does not account for this.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.22;
​
import "forge-std/Test.sol";
import "../contracts/MembershipERC1155.sol";
​
contract ProfitDistributionTest is Test {
MembershipERC1155 token;
address admin = address(0x1);
address user1 = address(0x2);
address user2 = address(0x3);
IERC20 currencyToken;
​
function setUp() public {
token = new MembershipERC1155();
token.initialize("Membership", "MBR", "uri/", admin, address(currencyToken));
// Minting weighted tokens to users
token.mint(user1, 0, 1); // weight 64
token.mint(user2, 1, 1); // weight 32
}
​
function testProfitDistribution() public {
uint256 profitAmount = 1000 * 1e18;
​
// Sending profit to the contract
vm.startPrank(admin);
currencyToken.transfer(address(token), profitAmount);
token.sendProfit(profitAmount);
vm.stopPrank();
​
// Users claim profits
vm.startPrank(user1);
uint256 user1Profit = token.claimProfit();
vm.stopPrank();
​
vm.startPrank(user2);
uint256 user2Profit = token.claimProfit();
vm.stopPrank();
​
// Expected profit shares based on weights (64 for user1 and 32 for user2)
uint256 expectedUser1Profit = (profitAmount * 64) / 96;
uint256 expectedUser2Profit = (profitAmount * 32) / 96;
​
// Assertion checks
assertEq(user1Profit, expectedUser1Profit, "User1 received incorrect profit amount");
assertEq(user2Profit, expectedUser2Profit, "User2 received incorrect profit amount");
}
}

Impact

  1. This flaw results in profit misallocation across token holders, causing discrepancies in the intended distribution based on the weight of each token ID.

  2. Affected users may lose trust in the system if they realize their profits are not proportional to their holdings.

  3. Malicious users may exploit this vulnerability by transferring low-weight tokens to maximize their profit share at the expense of legitimate holders.

Tools Used

Manual review.

Recommendations

Modify the sendProfit function to calculate each user’s profit share based on shareOf(account) instead of relying solely on totalSupply. This function already accounts for each token's weight in the distribution.

function sendProfit(uint256 amount) external {
if (totalSupply > 0) {
totalProfit += (amount * ACCURACY) / totalWeight();
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
}
}
​
function totalWeight() internal view returns (uint256) {
// Calculate the total weighted supply across all token holders
uint256 weightedSupply;
for (uint256 i = 0; i < 7; ++i) {
weightedSupply += balanceOf(account, i) * (2 ** (6 - i));
}
return weightedSupply;
}
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.

Give us feedback!