sherlock-audit / 2023-12-flatmoney-judging

9 stars 7 forks source link

Bony - In correct calculation logic of `FlatcoinVault.checkSkewMax` function #247

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

Bony

high

In correct calculation logic of FlatcoinVault.checkSkewMax function

Summary

To calculate the longSkewFraction correctly, the _additionalSkew amount should be substracted from the stableCollateralTotal, not to be added to the sizeOpenedTotal. And since the FlatcoinVault.checkSkewMax is used to check whether the skew is disabled or not, the skew check will be bypassed and the system will be too skewed towards the long

Vulnerability Detail

   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);
        }
    }

To calculate the longSkewFraction correctly, the additonalSkew should be substracted from the stableCollateralTotal, not to be added to the sizeOpenedTotal

Impact

The system will 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

   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:

valid: the value should infact be subtracted during withdrawal; 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.