Unexpected revert during announce and execute delayed orders

Summary

announce and execute delayed orders can revert due to an arithmetic overflow

Vulnerability Detail

User decides delayed deposit into the stable LP:

frist ,calls announceStableDeposit() method, https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/DelayedOrder.sol#L67

and then, executeDeposit() https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/StableModule.sol#L61

The problem is that the prepareAnnouncementOrder function must calls thesettleFundingFees()function each time.

 function announceStableDeposit(
        uint256 depositAmount,
        uint256 minAmountOut,
        uint256 keeperFee
    ) external whenNotPaused {
        uint64 executableAtTime = _prepareAnnouncementOrder(keeperFee);

https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/DelayedOrder.sol#L634

 function _prepareAnnouncementOrder(uint256 keeperFee) internal returns (uint64 executableAtTime) {
        // Settle funding fees to not encounter the `MaxSkewReached` error.
        // This error could happen if the funding fees are not settled for a long time and the market is skewed long
        // for a long time.
        vault.settleFundingFees();

settleFundingFees() method is used to settle the funding fees between longs and LPs. https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/FlatcoinVault.sol#L228

test poc

function testPoc() public{
                 setWethPrice(1000e8);

        uint256 tokenId = announceAndExecuteDepositAndLeverageOpen({
            traderAccount: alice,
            keeperAccount: keeper,
            depositAmount: 100e18,
            margin: 50e18,
            additionalSize: 120e18,
            oraclePrice: 1000e8,
            keeperFeeAmount: 0
        });
        skip(5 days);
          uint256 tokenId1 = announceAndExecuteDepositAndLeverageOpen({
            traderAccount: alice,
            keeperAccount: keeper,
            depositAmount: 100e18,
            margin: 50e18,
            additionalSize: 100e18,
            oraclePrice: 1000e8,
            keeperFeeAmount: 0
        });

        announceAndExecuteDeposit({  //_prepareAnnouncementOrder ---  vault.settleFundingFees();
            traderAccount: bob,
            keeperAccount: keeper,
            depositAmount: 100e18,
            oraclePrice: 1000e8,
            keeperFeeAmount: 0
        });
        skip(3 days);
        console2.log("marginDepositedTotal;",vaultProxy.getGlobalPositions().marginDepositedTotal);
        announceAndExecuteDeposit({  //_prepareAnnouncementOrder ---  vault.settleFundingFees();
            traderAccount: bob,
            keeperAccount: keeper,
            depositAmount: 100e18,
            oraclePrice: 1000e8,
            keeperFeeAmount: 0
        });

    }

Below is a partial screenshot of the results：

[7603] TransparentUpgradeableProxy::settleFundingFees() 
    │   │   │   ├─ [6967] FlatcoinVault::settleFundingFees() [delegatecall]
    │   │   │   │   └─ ← -69297708333333333480
    │   │   │   └─ ← -69297708333333333480

 console::log(marginDepositedTotal;, 54992013933095422080 [5.499e19]) [staticcall]

FlatcoinVault::getGlobalPositions() [delegatecall]
    │   │   │   │   └─ ← (115792089237316195423570985008687907853269984665640564039443278313512891728536 [1.157e77], 1000000000000000000000 [1e21], 220000000000000000000 [2.2e20])

[FAIL. Reason: Arithmetic over/underflow] testPoc() (gas: 4355421)

https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/FlatcoinVault.sol#L232 at this time , _globalPositions.marginDepositedTotal =5,4992013933095422080, _fundingFees= -69,297708333333333480

_globalPositions.marginDepositedTotal > _fundingFees

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

so, uint256(int256(5,4992013933095422080) + (-69,297708333333333480)) =uint256(-14305694400237911400) ,the value of marginDepositedTotal will be very huge(as we can see, 1.157e77)

Impact

The vulnerability can result in the contract reverting due to an overflow, disrupting the functionality of the contract.

Code Snippet

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

Tool used

Manual Review

Recommendation

need to take into account the case where marginDepositedTotal plus _fundingFees is negative

Duplicate of #195

sherlock-audit / 2023-12-flatmoney-judging

CL001 - Unexpected revert during announce and execute delayed orders #41