Project

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

Profit Distribution Issue Due to savedProfit[account] Being Set to Zero

Summary:

The core issue lies in the interaction between the claimProfit and saveProfit functions. In their current form, the profit distribution calculations are not accurate due to the way profits are calculated and reset. Specifically, the savedProfit[account] is reset to zero immediately after being updated with the user's calculated profit, breaking the continuity of the profit tracking system.

Vulnerability Details:

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);
}
function saveProfit(address account) internal returns (uint256 profit) {
uint256 unsaved = getUnsaved(account);
lastProfit[account] = totalProfit;
profit = savedProfit[account] + unsaved;
savedProfit[account] = profit;
}
function getUnsaved(address account) internal view returns (uint256 profit) {
return ((totalProfit - lastProfit[account]) * shareOf(account)) / ACCURACY;
}

The vulnerability revolves around how profits are calculated, stored, and claimed in the contract. Specifically, the issue arises from the line:

savedProfit[msg.sender] = 0;
  1. claimProfit():

    • Calls saveProfit(msg.sender), which updates the savedProfit[msg.sender] value by adding the unsaved profit to the stored profit.

    • Once the profit is calculated and saved, the savedProfit[msg.sender] value is set to 0, even though the value has just been updated to include unsaved profits.

    • The updated profit (before resetting savedProfit) is then transferred to the user.

Why Setting savedProfit[msg.sender] = 0 is Problematic:

The core issue lies in the double reset of the savedProfit[msg.sender] value:

  1. Loss of Intermediate State:

    • When you set savedProfit[msg.sender] to 0 immediately after the profit is transferred, you lose the intermediate state (the total profit accumulated for that user).

    • This is critical because the logic to calculate profits for a user relies on historical data (i.e., the amount of profit a user has accumulated over time). The value savedProfit[msg.sender] serves as the last known profit before the current profit calculation.

  2. Profit Recalculation Issues:

    • The saveProfit() function updates savedProfit[msg.sender] with the sum of the previous value (savedProfit[msg.sender]) and the unsaved profit for the user. However, immediately setting it to 0 in claimProfit() causes the savedProfit[msg.sender] to lose the accumulated profit, which will impact future profit calculations for that user.

    • This creates the problem where, on the next call to claimProfit(), the contract would "forget" the previously accumulated profit, and users will receive profits that don't reflect the true amount they're entitled to.

  3. Incorrect Profit Calculation:

    • The saveProfit() function relies on savedProfit[msg.sender] to track accumulated profits. When this value is reset to 0 after each claim, it means that on the next claim, the profit calculation for that user (getUnsaved(account)) will only include profits generated since the last claim.

    • As a result, users might not receive the correct amount of profit. Specifically, they might miss out on profits from previous periods because the contract resets the saved profit after each claim.

By resetting savedProfit[account] to zero, the contract loses track of the cumulative profit across multiple claims. The system will not accumulate profit values correctly, and users will not be able to track or claim all of their entitled profit.

  • Incorrect Calculations: Each time a user claims their profit, the system effectively resets their available profit to zero, leading to incorrect calculations for subsequent claims. For instance, if a user claims part of their profit and then calls saveProfit again, it will calculate the profit based on a stale or zeroed value, making it impossible to properly calculate the remaining balance. This causes wrong calculation of profits and wrong distributions. The users get lower amount of profit.

Example Scenario:

Consider the following scenario:

  1. A user has accumulated 1000 units of profit.

  2. They call claimProfit() and receive the full 1000 units.

  3. Immediately after receiving the profit, savedProfit[msg.sender] is set to 0.

  4. In the next period, more profit accumulates, and when the user calls claimProfit() again, they receive profit based only on the new accumulation, and not the 1000 units that were claimed in the previous round.

  5. This means the user is underpaid by the amount of the previous claim because the contract "forgot" about the previously accumulated profit.

Impact: Incorrect profit Distribution

Impact on Users:

  • Underclaiming: Users may not receive all the profit they are entitled to. This is especially problematic if the profit calculations span over a long period, as earlier profits will be lost after each claim.

  • Inaccurate Profit Distribution: The contract could end up distributing less profit to users than they should receive, leading to inconsistencies and potential disputes.

Tools Used:

Manual Code Review: Analyzed the interaction between claimProfit, saveProfit, and profitOf functions.

Recommendations:

Do Not Reset savedProfit[account] to Zero:

Instead of resetting savedProfit[account] to zero after each claim, allow it to persist across multiple claims. The savedProfit[account] value should accumulate over time as users continue to claim profits, ensuring that the calculation remains accurate and reflective of the total profit accumulated by the user.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
0xbrivan2 Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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