search

sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

Dliteofficial - Underflow in `FlatcoinVault::settleFundingFees()` if the absolute value of funding fees is higher than marginDepositedTotal. #146

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 7 months ago

Dliteofficial

high

Underflow in FlatcoinVault::settleFundingFees() if the absolute value of funding fees is higher than marginDepositedTotal.

Summary

When the fees owed to UNIT LPs by leverage traders is being settled or vice versa, if the absolute value of fundingFee is greater than the global marginDepositedTotal variable, an underflow might occur causing the marginDepositedTotal to be a huge value.

Vulnerability Detail

Funding fees or borrow rate is what the UNIT LPs charge the leverage traders for providing the leverage they use. This fee is settled in FlatcoinVault::settleFundingFees(). For proper accounting, if the marginDepositedTotal is higher than the fundingFee, the addition is calculated, recorded and effected for stableCollateralTotal as well and if otherwise, marginDepositedTotal is set to 0.

The issue here is because the addition of marginDepositedTotal and fundingFees returns an int value that when wrapped in uint returns a really high value. This is always going to be an issue if the absolute value of fundingFees is higher than that of marginDepositedTotal.

Impact

When this happens, the recorded marginDepositedTotal will be a really huge value.

Code Snippet

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

        cumulativeFundingRate = PerpMath._nextFundingEntry(unrecordedFunding, cumulativeFundingRate);

        lastRecomputedFundingRate += fundingChangeSinceRecomputed;
        lastRecomputedFundingTimestamp = (block.timestamp).toUint64();

        _fundingFees = PerpMath._accruedFundingTotalByLongs(_globalPositions, unrecordedFunding);

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

        _updateStableCollateralTotal(-_fundingFees);
    }

Tool used

Manual Review

Recommendation

Use Openzeppelin SafeMath and/or SafeCast to handle calculations and overflow/underflow

Duplicate of #195

sherlock-admin commented 6 months ago

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

takarez commented:

valid; high(2)