sherlock-audit / 2023-12-flatmoney-judging

9 stars 7 forks source link

deepplus - `_profitLoss` function of the `PerpMath` calculate the `PnL` incorrectly. #286

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 5 months ago

deepplus

medium

_profitLoss function of the PerpMath calculate the PnL incorrectly.

Summary

The calculation of PnL in _profitLoss function of PerpMath is wrong.

Vulnerability Detail

The _profitLoss function calculates and returns pnl based on passed params of position and price.

    function _profitLoss(FlatcoinStructs.Position memory position, uint256 price) internal pure returns (int256 pnl) {
        int256 priceShift = int256(price) - int256(position.lastPrice);
        int256 profitLossTimesTen = (int256(position.additionalSize) * (priceShift) * 10) / int256(price);

        if (profitLossTimesTen % 10 != 0) {
            return profitLossTimesTen / 10 - 1;
        } else {
            return profitLossTimesTen / 10;
        }
    }

First, following params may be passed to this function.

Next, think of the following params.

Impact

Above wrong calculation may leads to loss of user's fund.

Code Snippet

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

Tool used

Manual Review

Recommendation

The _profitLoss function should be updated as follow.

    function _profitLoss(FlatcoinStructs.Position memory position, uint256 price) internal pure returns (int256 pnl) {
        int256 priceShift = int256(price) - int256(position.lastPrice);
        int256 profitLossTimesTen = (int256(position.additionalSize) * (priceShift) * 10) / int256(price);

-       if (profitLossTimesTen % 10 != 0) {
-           return profitLossTimesTen / 10 - 1;
-       } else {
-           return profitLossTimesTen / 10;
-       }
+       return profitLossTimesTen / 10;
    }
sherlock-admin commented 5 months ago

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

takarez commented:

valid: seem like same report as issue 257; medium(8)

nevillehuang commented 4 months ago

I believe the rounding errors from the example provided are too small to justify this issue and its duplicates as medium severity