search

sherlock-audit / 2023-12-flatmoney-judging

9 stars 8 forks source link

ni8mare - In `settleFundingFees`, `_globalPositions.marginDepositedTotal` can be assigned a wrong value because of improper comparisons. #237

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

ni8mare

high

In settleFundingFees, _globalPositions.marginDepositedTotal can be assigned a wrong value because of improper comparisons.

Summary

_globalPositions.marginDepositedTotal can take a very different value than intended because of improper comparisons and break any calculations which are dependent on the marginDepositedTotal.

Vulnerability Detail

Look at the following section in settleFundingFees:

        // 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;

int256(_globalPositions.marginDepositedTotal) is compared with _fundingFees. If marginDepositedTotal then _globalPositions.marginDepositedTotal equals uint256(int256(_globalPositions.marginDepositedTotal) + _fundingFees). Otherwise, it's 0. This is alright in all cases, except when _fundingFees is negative but its magnitude is bigger than _globalPositions.marginDepositedTotal). In that case, the value of int256(_globalPositions.marginDepositedTotal) + _fundingFeeswill be negative. uint256(-ve value) is equal to a very big positive value. For instance, uint256(-1) is equal to 115792089237316195423570985008687907853269984665640564039457584007913129639935.

Hence, _globalPositions.marginDepositedTotal will be assigned the wrong value.

Impact

_globalPositions.marginDepositedTotal is assigned an incorrect value and as it is used throughout the protocol, it will lead to wrong calculations.

Code Snippet

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

Tool used

Manual Review

Recommendation

Compare the magnitudes of _globalPositions.marginDepositedTotal and _fundingFees in the above code snippet.

Duplicate of #195

sherlock-admin commented 5 months ago

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

takarez commented:

valid: high(2)