sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

0xrobsol - Addressing Potential Inequity in Margin Reset Logic Due to Funding Fee Calculations #108

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

0xrobsol

medium

Addressing Potential Inequity in Margin Reset Logic Due to Funding Fee Calculations

Summary

There is a potential flaw in the settleFundingFees() logic that is used to update the total deposited margin in the context of funding fees accrued to long positions.

Vulnerability Detail

The core of the vulnerability lies in how the protocol adjusts the marginDepositedTotal based on the funding fees accrued. The current logic sets the marginDepositedTotal to 0 in scenarios where the updated margin (after accounting for funding fees) would not be positive. However, the condition fails to account for scenarios where the margin exactly equals the funding fees, potentially leading to an unnecessary reset of the margin to 0.

Impact

The primary impact of this issue is the potential for an inaccurate representation of the total deposited margin. Specifically, in situations where the margin is equal to the funding fees, resetting the margin to 0 could unjustly penalize position holders by erasing their deposited margin when, logically, it should break even.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/FlatcoinVault.sol#L232

Tool used

Manual Review

Recommendation

The condition for resetting the marginDepositedTotal to 0 should be refined to more accurately reflect the intended logic. Specifically, it should account not only for scenarios where the margin is less than the funding fees but also for scenarios where the margin exactly equals the funding fees. A possible solution is to adjust the conditional check to ensure that the margin is set to 0 only when it is strictly less than the funding fees, not when it is equal. This could look something like:

if (int256(_globalPositions.marginDepositedTotal) > _fundingFees) {
    _globalPositions.marginDepositedTotal = uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees);
} else if (int256(_globalPositions.marginDepositedTotal) < _fundingFees) {
    _globalPositions.marginDepositedTotal = 0;
} // Optionally handle the == case explicitly if needed

This adjustment ensures that the margin is only reset to 0 in scenarios where it is indeed less than the accrued funding fees, thereby preserving the integrity of the margin accounting system and preventing undue penalization of users.

Duplicate of #195

sherlock-admin commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: high(2)