settleFundingFees() might underflow and lead to incorrect huge value for marginDepositedTotal.

Summary

settleFundingFees() might underflow and lead to incorrect huge value for marginDepositedTotal, which will lead a great loss for the protocol.

Vulnerability Detail

settleFundingFees() will calculate the new marginDepositedTotal based on the calculated _fundingFees.

However, the following POC will underflow and lead to a huge number for marginDepositedTotal.. The intention to set it to zero in such case (_fundingFees is negative and will bring marginDepositedTotal to negative) fails.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";
import {Counter} from "../src/Counter.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";

contract FlatcoinVault {
    uint256  public marginDepositedTotal = 1_000_000;

    function settleFundingFees() public returns (int256 _fundingFees){
        _fundingFees = -2_000_2000;
         marginDepositedTotal = (int256(marginDepositedTotal) > _fundingFees)
            ? uint256(int256(marginDepositedTotal) + _fundingFees)
            : 0;
    }

}

contract FundingTest is Test {
    FlatcoinVault vault;

    function setUp() public {
        vault = new FlatcoinVault();
    }

    function testFunding() public{
        vault.settleFundingFees();
        assertTrue(vault.marginDepositedTotal() != 0);
        console2.log(vault.marginDepositedTotal());
    }
}

 _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

The main problem is that condition (int256(_globalPositions.marginDepositedTotal) > _fundingFees) will be true when _fundingFees is negative. The needed condition is "_globalPositions.marginDepositedTotal) + _fundingFees >=0) instead.

In the following, when marginDepositedTotal = 1_000_000 and _fundingFee = -2_000_000```, instead of settingmarginDepositedTotal = 0,marginDepositedTotal`` is set to a huge number: 115792089237316195423570985008687907853269984665640564039457584007913110637936.

This will lead to wrong value for marginDepositedTotal, and the loss of funding for the protocol and other clients.

Impact

settleFundingFees() might underflow and lead to incorrect huge value for marginDepositedTotal, which will lead a great loss for the protocol and other clients.

Code Snippet

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

Tool used

Foundry

Manual Review

Recommendation

Change the condition as follows:

 _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) + fundingFees >= 0)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

Duplicate of #195

sherlock-audit / 2023-12-flatmoney-judging

chaduke - settleFundingFees() might underflow and lead to incorrect huge value for marginDepositedTotal. #79