sherlock-audit / 2023-12-flatmoney-judging

9 stars 7 forks source link

deepplus - In `settleFundingFees` function of `FlatcoinVault` contract, `_globalPositions.marginDepositedTotal` is updated incorrectly. #256

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

deepplus

high

In settleFundingFees function of FlatcoinVault contract, _globalPositions.marginDepositedTotal is updated incorrectly.

Summary

Since the signature of the _fundingFees is wrong, the _globalPositions.marginDepositedTotal is updated incorrectly.

Vulnerability Detail

In the protocol, they execute settleFundingFees function of the FlatcoinValut contract to settle the funding fees. However, the _globalPositions.marginDepositedTotal would be updated incorrectly due to _fundingFees has the wrong 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)
            ? uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees)
            : 0;

        _updateStableCollateralTotal(-_fundingFees);

To be correct, the _fundingFees of the signature should be -.

Impact

Due to wrong signature of the _fundingFees, _globalPositions.marginDepositedTotal is calculated incorrectly.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

The signature of the _fundingFees should be - in calculation.

    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 commented 5 months ago

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

takarez commented:

invalid