sherlock-audit / 2023-12-flatmoney-judging

9 stars 7 forks source link

deepplus - `checkSkewMax` function of the `FlatcoinValut` contract calculate the `longSkewFraction` incorrectly. #271

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

deepplus

high

checkSkewMax function of the FlatcoinValut contract calculate the longSkewFraction incorrectly.

Summary

In order to accurately calculate the longSkewFraction, it is necessary to subtract the _additionalSkew amount from the stableCollateralTotal instead of adding it to the sizeOpenedTotal. And it may result in the bypassing of the skew check, leading to an excessive skew towards the long side within the system since the FlatcoinVault.checkSkewMax is utilized to determine whether the skew is disabled.

Vulnerability Detail

To calculate the longSkewFraction correctly, the additonalSkew should be subtracted from the stableCollateralTotal instead of being added to the sizeOpenedTotal

   function checkSkewMax(uint256 _additionalSkew) public view {
        // check that skew is not essentially disabled
        if (skewFractionMax < type(uint256).max) {
            uint256 sizeOpenedTotal = _globalPositions.sizeOpenedTotal;

            if (stableCollateralTotal == 0) revert FlatcoinErrors.ZeroValue("stableCollateralTotal");

@>      uint256 longSkewFraction = ((sizeOpenedTotal + _additionalSkew) * 1e18) / stableCollateralTotal;

            if (longSkewFraction > skewFractionMax) revert FlatcoinErrors.MaxSkewReached(longSkewFraction);
        }
    }

Impact

Due to wrong calculation in checkSkewMax function, the system would be too skewed towards the long.

Code Snippet

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

Tool used

Manual Review

Recommendation

checkSkewMax function should be updated as follow.

   function checkSkewMax(uint256 _additionalSkew) public view {
        // check that skew is not essentially disabled
        if (skewFractionMax < type(uint256).max) {
            uint256 sizeOpenedTotal = _globalPositions.sizeOpenedTotal;

            if (stableCollateralTotal == 0) revert FlatcoinErrors.ZeroValue("stableCollateralTotal");

-          uint256 longSkewFraction = ((sizeOpenedTotal + _additionalSkew) * 1e18) / stableCollateralTotal;
+          uint256 longSkewFraction = (sizeOpenedTotal * 1e18) / (stableCollateralTotal - _additionalSkew);

            if (longSkewFraction > skewFractionMax) revert FlatcoinErrors.MaxSkewReached(longSkewFraction);
        }
    }

Duplicate of #193

sherlock-admin commented 5 months ago

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

takarez commented:

medium(7)

sherlock-admin commented 4 months ago

The protocol team fixed this issue in PR/commit https://github.com/dhedge/flatcoin-v1/pull/273.