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

Inconsistent Weight Application in Profit Distribution Tracking Causes Incorrect Profit Calculations During Token Transfers, Leading to Potential Misallocations

cheatc0d33

Summary

The current implementation of the MembershipERC1155 contract contains a critical flaw in its profit distribution tracking system that could lead to incorrect profit calculations during token transfers and subsequent distributions. This vulnerability stems from inconsistent weight application in the contract's accounting system and could result in users receiving incorrect profit shares, particularly when tokens change ownership across different distribution periods.

The vulnerability lies in the contract's dual application of weights - once in totalSupply tracking and again in shareOf() calculations - which creates inconsistencies in historical profit tracking during token transfers. When tokens are transferred, the contract updates the profit records using saveProfit(), which relies on the current shareOf() value. However, since totalSupply already includes weighted values, this creates a misalignment in how historical profits are recorded and how new profits are calculated after transfers.

Vulnerable code snippets:

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L60
https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L169

function mint(address to, uint256 tokenId, uint256 amount) external override onlyRole(OWP_FACTORY_ROLE) {
totalSupply += amount * 2 ** (6 - tokenId); // First weight application
_mint(to, tokenId, amount, "");
}
​
function shareOf(address account) public view returns (uint256) {
return (balanceOf(account, 0) * 64) + // Second weight application
(balanceOf(account, 1) * 32) +
(balanceOf(account, 2) * 16) +
(balanceOf(account, 3) * 8) +
(balanceOf(account, 4) * 4) +
(balanceOf(account, 5) * 2) +
balanceOf(account, 6);
}
​
function _update(address from, address to, uint256[] memory ids, uint256[] memory amounts) internal virtual override(ERC1155Upgradeable) {
if (from != address(0)) saveProfit(from);
if (to != address(0)) saveProfit(to);
super._update(from, to, ids, amounts);
}

Consider this scenario:
Alice owns 1 token of ID 0 (weight 64)
Initial totalSupply = 64 (weighted)
A profit distribution of 1000 USDC occurs
totalProfit += (1000 * 1e30) / 64

Alice transfers her token to Bob, triggering _update():

  • saveProfit(Alice) calculates her unsaved profits using shareOf(Alice) = 64

  • Sets lastProfit[Alice] = totalProfit

  • saveProfit(Bob) sets his lastProfit = totalProfit

When a new profit distribution occurs:
Bob's profit calculation uses the lastProfit value set during the transfer, which was based on a different weighting scheme than what's used in totalSupply, potentially leading to incorrect profit calculations.

Recommended Fix

contract MembershipERC1155 {
mapping(uint256 => uint256) private _totalSupplyById;
function mint(address to, uint256 tokenId, uint256 amount) external onlyRole(OWP_FACTORY_ROLE) {
_totalSupplyById[tokenId] += amount;
_mint(to, tokenId, amount, "");
}
​
function getTotalWeightedSupply() public view returns (uint256) {
uint256 weighted = 0;
for(uint256 i = 0; i < 7; i++) {
weighted += _totalSupplyById[i] * (2 ** (6 - i));
}
return weighted;
}
​
function sendProfit(uint256 amount) external {
uint256 weightedSupply = getTotalWeightedSupply();
if (weightedSupply > 0) {
totalProfit += (amount * ACCURACY) / weightedSupply;
IERC20(currency).safeTransferFrom(msg.sender, address(this), amount);
emit Profit(amount);
} else {
IERC20(currency).safeTransferFrom(msg.sender, creator, amount);
}
}
​
function burn_(address from, uint256 tokenId, uint256 amount) internal {
_totalSupplyById[tokenId] -= amount;
_burn(from, tokenId, amount);
}
}

This fix ensures consistent weight tracking throughout the contract by maintaining unweighted token supplies per ID and calculating weights only when needed. The solution provides better encapsulation of the weighting logic, makes the code more maintainable, and eliminates the potential for profit calculation errors during token transfers. Most importantly, it maintains accurate historical profit tracking by using a consistent weighting scheme across all operations.

Updates

Lead Judging Commences

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

Support

FAQs

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