sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

KingNFT - Users can not increase or decrease margin while ````screw >= 120```` #162

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 7 months ago

KingNFT

medium

Users can not increase or decrease margin while screw >= 120

Summary

In current implementation, the max allowed screw is 120%, increasing new position size would be rejected above this threshold. But actually, pure increasing or decreasing collateral would be reverted too. As this two operations are not related to net position changes, so they should be allowed.

Vulnerability Detail

The issue arise on L163 of LeverageModule.sol, the = 0 case which represent increasing or decreasing collateral would be rejected while current screw is bigger than 120% (L166 & L305).

File: src/LeverageModule.sol
163:         if (announcedAdjust.additionalSizeAdjustment >= 0) { // @audit the 0 case ?
164:             // Given that the size of a position is being increased, it's necessary to check that
165:             // it doesn't exceed the max skew limit.
166:             vault.checkSkewMax(uint256(announcedAdjust.additionalSizeAdjustment));
167: 
168:             if (adjustPrice > announcedAdjust.fillPrice)
169:                 revert FlatcoinErrors.HighSlippage(adjustPrice, announcedAdjust.fillPrice);
170:         } else {
171:             if (adjustPrice < announcedAdjust.fillPrice)
172:                 revert FlatcoinErrors.HighSlippage(adjustPrice, announcedAdjust.fillPrice);
173:         }

File: src/FlatcoinVault.sol
296:     function checkSkewMax(uint256 _additionalSkew) public view {
297:         // check that skew is not essentially disabled
298:         if (skewFractionMax < type(uint256).max) {
299:             uint256 sizeOpenedTotal = _globalPositions.sizeOpenedTotal;
300: 
301:             if (stableCollateralTotal == 0) revert FlatcoinErrors.ZeroValue("stableCollateralTotal");
302: 
303:             uint256 longSkewFraction = ((sizeOpenedTotal + _additionalSkew) * 1e18) / stableCollateralTotal;
304: 
305:             if (longSkewFraction > skewFractionMax) revert FlatcoinErrors.MaxSkewReached(longSkewFraction);
306:         }
307:     }

Impact

1) break part of core functionality 2) users might be liquidated out as they can't increase margin in time.

Code Snippet

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

Tool used

Manual Review

Recommendation

see PoC

sherlock-admin commented 6 months ago

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

takarez commented:

invalid

nevillehuang commented 6 months ago

Invalid and by design as noted in contest details:

When long max skew (skewFractionMax) is reached, flatcoin holders cannot withdraw, and no new leverage positions can be opened. This is to prevent the flatcoin holders being increasingly short. This is temporary because the funding rate will bring the skew back to 0 and create more room for flatcoin holders to withdraw and leverage traders to open positions.