sherlock-audit / 2023-12-flatmoney-judging

9 stars 8 forks source link

bareli - Division by Zero: #229

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

bareli

medium

Division by Zero:

Summary

Division by Zero: The code should ensure that division by zero cannot occur. For example, in _calcLiquidationPrice, if position.additionalSize is zero, it could lead to a division by zero error.

Vulnerability Detail

function _calcLiquidationPrice( FlatcoinStructs.Position memory position, FlatcoinStructs.PositionSummary memory positionSummary, uint256 liquidationMargin ) private pure returns (int256 liqPrice) { // A position can be liquidated whenever:- remainingMargin <= liquidationMargin // // Hence, expanding the definition of remainingMargin the exact price at which a position can be liquidated is: // // liquidationMargin = margin + profitLoss + funding // liquidationMargin = margin + [(price - entryPrice) postionSize / price] + funding // liquidationMargin - (margin + funding) = [(price - entryPrice) postionSize / price] // liquidationMargin - (margin + funding) = postionSize - (entryPrice postionSize / price) // positionSize - [liquidationMargin - (margin + funding)] = entryPrice postionSize / price // positionSize entryPrice / {positionSize - [liquidationMargin - (margin + funding)]} = price // // In our case, positionSize = position.additionalSize. // Note: If there are bounds on liquidationFee and/or keeperFee then this formula doesn't yield an accurate liquidation price. // This is because, when the position size is too large such that liquidation fee for that position has to be bounded we are essentially // solving the following equation: // LiquidationBuffer + (LiquidationUpperBound / Price) + KeeperFee = Margin + (Price - EntryPrice)PositionSize + AccruedFunding // And according to Wolfram Alpha, this equation cannot be solved for Price (at least trivially): // https://www.wolframalpha.com/input?i=A+++(B+/+X)+%3D+C+++(X+-+D)+*+E+,+X+%3E+0,+Solution+for+variable+X return @> int256((position.additionalSize)._multiplyDecimal(position.lastPrice))._divideDecimal( int256(position.additionalSize + position.marginDeposited) + positionSummary.accruedFunding - int256(liquidationMargin) ); }

Impact

revert may happen.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/libraries/PerpMath.sol#L491

Tool used

Manual Review

Recommendation

use a require statement for zero check.

sherlock-admin commented 5 months ago

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

takarez commented:

invalid

nevillehuang commented 5 months ago

Invalid, position.additionalSize cannot be zero, as seen by this check here