itsabinashb - FlatcoinVault::wrong accounting of net PnL

[sherlock-admin]

itsabinashb

medium

FlatcoinVault::wrong accounting of net PnL

Summary

There is a wrong accounting of net PnL in updateGlobalPositionData() in FlatcoinVault.sol contract.

Vulnerability Detail

In updateGlobalPositionData() there is a internal call to _updateStableCollateralTotal() where net PnL is passed as argument. However, the net PnL is passed as negetive value:

        _updateStableCollateralTotal(-profitLossTotal);

While calculating net PnL we are substracting the previous price, i.e the price when the position was opened, from current price of asset, here:

        int256 priceShift = int256(price) - int256(globalPosition.lastPrice);

So, if the price goes down then the priceShift will be negative, as a result the net PnL will be negative too. because net PnL is calculated like this:

        return (int256(globalPosition.sizeOpenedTotal) * (priceShift)) / int256(price);

So, if the net PnL is negative and as it is passed with -ve sign to _updateStableCollateral() it will increase the stable collateral total amount instead of decreasing it & if positive then it will decrease stable collateral total.

Impact

As net PnL is passed to _updateStableCollateral() with -ve sign it will increase the stable collateral total instead of decreasing and decrease instead of increasing it.

Code Snippet

1. https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/libraries/PerpMath.sol#L192-L199
2. https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/FlatcoinVault.sol#L205
3. https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/FlatcoinVault.sol#L429-L435

Tool used

Manual Review

Recommendation

Pass the profitLossTotal as +ve value to _updateStableCollateralTotal().

[nevillehuang]

Invalid, agree with sponsors comments:

The Watson seems to not have understood the protocol well. Stable collateral total is the collateral deposited by the LPs to mint UNIT. Leverage traders use this collateral for leverage trading. If the leverage traders are net losing due to price moving against them (price decrease) then they lose a portion/all of their margin to the LPs which is accounted by increasing the stableCollateralTotal. When the sign of adjustment in updateStableCollateral is negative then it rightly means we are adding the amount to the stableCollateralTotal. If we didn't want to use -ve values then we would have used uint256 for adjustments.