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

Front-Running Vulnerability in saveProfit Allows Profit Manipulation Through Strategic Token Transfers, Leading to Disproportionate Claims and Financial Drain

cheatc0d33

Summary

The saveProfit mechanism in the profit distribution system contains a critical front-running vulnerability that allows malicious actors to manipulate their profit claims through strategic token transfers. This vulnerability directly impacts the protocol's financial integrity by enabling attackers to extract a disproportionate amount of profits from the system, effectively draining more funds than they are legitimately entitled to receive. The economic impact scales with the size of profit distributions and could potentially make the entire profit-sharing mechanism economically unviable.

The issue stems from the profit calculation logic being intrinsically tied to token transfers and the current global profit state. The contract maintains a global totalProfit variable and individual lastProfit[account] mappings that are updated during token transfers. When profits are distributed via sendProfit(), the contract increases totalProfit, and users can claim their share based on the difference between their lastProfit and the current totalProfit. However, the critical flaw lies in how saveProfit is called during token transfers, allowing users to reset their profit checkpoints multiple times during a single profit distribution event.

Vulnerable Code Snippets

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

function saveProfit(address account) internal returns (uint256 profit) {
uint256 unsaved = getUnsaved(account);
lastProfit[account] = totalProfit;
profit = savedProfit[account] + unsaved;
savedProfit[account] = profit;
}
​
​
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 the following scenario: An attacker monitors the mempool for incoming sendProfit transactions. Upon detecting a profit distribution of 1000 tokens, they execute a series of transfers between their controlled addresses before the profit transaction is mined. When the profit distribution occurs, they can claim profits for each address they transferred to, as each transfer created a new profit checkpoint through saveProfit. After the profit distribution, they consolidate the tokens back to their main address. Through this manipulation, they could claim significantly more profits than their fair share based on their actual token holding duration.

Recommended Fix

The profit distribution system should be redesigned to use a snapshot-based mechanism that prevents profit manipulation through transfers. Here's a proposed fix:

contract MembershipERC1155 {
struct Snapshot {
uint256 timestamp;
uint256 totalProfit;
}
Snapshot[] public profitSnapshots;
mapping(address => uint256) public lastClaimedSnapshot;
function sendProfit(uint256 amount) external {
uint256 _totalSupply = totalSupply;
if (_totalSupply > 0) {
profitSnapshots.push(Snapshot({
timestamp: block.timestamp,
totalProfit: amount * ACCURACY / _totalSupply
}));
IERC20(currency).safeTransferFrom(msg.sender, address(this), amount);
emit Profit(amount);
}
}
function claimProfit() external returns (uint256 profit) {
uint256 lastClaimed = lastClaimedSnapshot[msg.sender];
uint256 currentIndex = profitSnapshots.length;
require(lastClaimed < currentIndex, "No new profits to claim");
uint256 accumulatedProfit = 0;
for (uint256 i = lastClaimed; i < currentIndex; i++) {
accumulatedProfit += (profitSnapshots[i].totalProfit * shareOf(msg.sender)) / ACCURACY;
}
lastClaimedSnapshot[msg.sender] = currentIndex;
IERC20(currency).safeTransfer(msg.sender, accumulatedProfit);
emit Claim(msg.sender, accumulatedProfit);
return accumulatedProfit;
}
}

This solution creates immutable profit distribution snapshots that cannot be manipulated through transfers. Each profit distribution creates a new snapshot, and users can only claim profits they were eligible for at the time of each snapshot. This eliminates the front-running vulnerability while maintaining fair profit distribution based on actual token ownership at distribution time.

Updates

Lead Judging Commences

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.