Bony - Wrong calculation of `_globalPositions.marginDepositedTotal` in `FlatcoinVault.settleFundingFees`

[sherlock-admin2]

Bony

medium

Wrong calculation of _globalPositions.marginDepositedTotal in FlatcoinVault.settleFundingFees

Summary

globalPositions.marginDepositedTotal is updated is the FlatcoinVault.settleFundingFees. However, the globalPositions.marginDepositedTotal is not updated correctly due to the wrong signature of the _fundingFees in L232

Vulnerability Detail

FlatcoinVault.settleFundingFees is called multiple times in the protocol to settle the funding fees between longs and LPs. However in the settleFundingFees function, at L232, the _fundingFees has the wrong signature and this will affect the update of globalPositions.marginDepositedTotal.

    function settleFundingFees() public returns (int256 _fundingFees) {
        (int256 fundingChangeSinceRecomputed, int256 unrecordedFunding) = _getUnrecordedFunding();

        // Record the funding rate change and update the cumulative funding rate.
        cumulativeFundingRate = PerpMath._nextFundingEntry(unrecordedFunding, cumulativeFundingRate);

        // Update the latest funding rate and the latest funding recomputation timestamp.
        lastRecomputedFundingRate += fundingChangeSinceRecomputed;
        lastRecomputedFundingTimestamp = (block.timestamp).toUint64();

        // Calculate the funding fees accrued to the longs.
        // This will be used to adjust the global margin and collateral amounts.
        _fundingFees = PerpMath._accruedFundingTotalByLongs(_globalPositions, unrecordedFunding);

        // In the worst case scenario that the last position which remained open is underwater,
        // we set the margin deposited total to 0. We don't want to have a negative margin deposited total.
@>      _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

        _updateStableCollateralTotal(-_fundingFees);
    }

To update _globalPositions.marginDepositedTotal correctly signature of _fundingFees should be -.

Impact

_globalPositions.marginDepositedTotal is not updated correctly and this will affect the workflow of overall protocol.

Code Snippet

    function settleFundingFees() public returns (int256 _fundingFees) {
        (int256 fundingChangeSinceRecomputed, int256 unrecordedFunding) = _getUnrecordedFunding();

        // Record the funding rate change and update the cumulative funding rate.
        cumulativeFundingRate = PerpMath._nextFundingEntry(unrecordedFunding, cumulativeFundingRate);

        // Update the latest funding rate and the latest funding recomputation timestamp.
        lastRecomputedFundingRate += fundingChangeSinceRecomputed;
        lastRecomputedFundingTimestamp = (block.timestamp).toUint64();

        // Calculate the funding fees accrued to the longs.
        // This will be used to adjust the global margin and collateral amounts.
        _fundingFees = PerpMath._accruedFundingTotalByLongs(_globalPositions, unrecordedFunding);

        // In the worst case scenario that the last position which remained open is underwater,
        // we set the margin deposited total to 0. We don't want to have a negative margin deposited total.
@>      _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

        _updateStableCollateralTotal(-_fundingFees);
    }

Tool used

Manual Review

Recommendation

_fundingFees should have - signature.

    function settleFundingFees() public returns (int256 _fundingFees) {
        (int256 fundingChangeSinceRecomputed, int256 unrecordedFunding) = _getUnrecordedFunding();

        // Record the funding rate change and update the cumulative funding rate.
        cumulativeFundingRate = PerpMath._nextFundingEntry(unrecordedFunding, cumulativeFundingRate);

        // Update the latest funding rate and the latest funding recomputation timestamp.
        lastRecomputedFundingRate += fundingChangeSinceRecomputed;
        lastRecomputedFundingTimestamp = (block.timestamp).toUint64();

        // Calculate the funding fees accrued to the longs.
        // This will be used to adjust the global margin and collateral amounts.
        _fundingFees = PerpMath._accruedFundingTotalByLongs(_globalPositions, unrecordedFunding);

        // In the worst case scenario that the last position which remained open is underwater,
        // we set the margin deposited total to 0. We don't want to have a negative margin deposited total.
-       _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > _fundingFees)
+      _globalPositions.marginDepositedTotal = (int256(_globalPositions.marginDepositedTotal) > -_fundingFees)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

        _updateStableCollateralTotal(-_fundingFees);
    }

Duplicate of #195

[sherlock-admin]

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

takarez commented:

invalid: no impact mentioned