Incorrect accounting of marginDepositedTotal

Summary

The _globalPositions.marginDepositedTotal could incorrectly be set to 0.

Vulnerability Detail

Let's take a look at a settleFundingFees function. This function is used to settle the currently unrecorded funding fees and update the _globalPositions.marginDepositedTotal.

function settleFundingFees() public returns (int256 _fundingFees) {
    ...
    _fundingFees = PerpMath._accruedFundingTotalByLongs(_globalPositions, unrecordedFunding);
    ...
    _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
        ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
        : 0;
    ...
}

In an edge case where funding fees haven't been settled for long, this can produce an incorrect state. First, we need to look at the _accruedFundingTotalByLongs function. By looking at a function we can see that the result can be either positive or negative based on the unrecodedFunding.

If the unrecordedFunding < 0 this means that shorts have to pay to the longs and if unrecordedFunding > 0 the longs have to pay to the shorts.

When unrecordedFunding < 0 this means profit for the longs and the return value of the function is positive.

function _accruedFundingTotalByLongs(
    FlatcoinStructs.GlobalPositions memory globalPosition,
    int256 unrecordedFunding
) internal pure returns (int256 accruedFundingLongs) {
    return -int256(globalPosition.sizeOpenedTotal)._multiplyDecimal(unrecordedFunding);
}

Consider the following scenario:

_globalPositions.marginDepositedTotal = 1 ether
_fundingFees = 1.1 ether (return value of _accruedFundingTotalByLongs)

Since _fundingFees are positive it means they have to be added to the marginDepositedTotal as the fee for the longs. Since fundingFees > marginDepositedTotal the marginDepositedTotal will be incorrectly set to 0 instead of 2.1 ether. (refer to the first piece of code).

Impact

marginDepositedTotal could be accounted incorrectly.

Code Snippet

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

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/libraries/PerpMath.sol#L219-L224

Tool used

Manual Review

Recommendation

Change the logic of the settleFundingFees as follows

function settleFundingFees() public returns (int256 _fundingFees) {
    ...
    _fundingFees = PerpMath._accruedFundingTotalByLongs(_globalPositions, unrecordedFunding);
    ...
    _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
        ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
-        : 0;
+        : _fundingFees > 0 ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees) : 0;
    ...
}

Duplicate of #195

sherlock-audit / 2023-12-flatmoney-judging

evmboi32 - Incorrect accounting of marginDepositedTotal #153