sherlock-audit / 2024-02-perpetual-judging

2 stars 2 forks source link

jasonxiale - tiny positions might not be liquidated entirely #135

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

jasonxiale

medium

tiny positions might not be liquidated entirely

Summary

According to LibLiquidation.sol#L101-L102, if a position's value is less than 100USD, and it's liquidatable, it will be liquidated entirely. Because of incorrectly decimal calculation, those position will not be liquidated entirely.

Vulnerability Detail

Assume we have one(1e18 wei) ETH, and according to pyth, its value should be 354567075439 with -8 as expo. By applying PythOracleAdapter._convertToUint256, the ETH's price should be 354567075439*10**(18 + (-8)) = 3545670754390000000000. Then according to LibLiquidation.sol#L103: positionValueAbs = self.positionSize.abs().mulWad(self.price); So the positionValueAbs for 1 ETH should be 1 * 3545670754390000000000 / 10**18 = 3545.67075439 But _MIN_PARTIAL_LIQUIDATE_POSITION_VALUE is defined as *100 WAD**.

which is not correct

Impact

tiny positions might not be liquidated entirely

Code Snippet

https://github.com/sherlock-audit/2024-02-perpetual/blob/02f17e70a23da5d71364268ccf7ed9ee7cedf428/perp-contract-v3/src/clearingHouse/LibLiquidation.sol#L26C1-L28C1

Tool used

Manual Review

Recommendation

diff --git a/perp-contract-v3/src/clearingHouse/LibLiquidation.sol b/perp-contract-v3/src/clearingHouse/LibLiquidation.sol
index a5bec76..cd2f780 100644
--- a/perp-contract-v3/src/clearingHouse/LibLiquidation.sol
+++ b/perp-contract-v3/src/clearingHouse/LibLiquidation.sol
@@ -25,7 +25,7 @@ struct LiquidationResult {
 }

 // 100 USD in WAD
-uint256 constant _MIN_PARTIAL_LIQUIDATE_POSITION_VALUE = 100 * WAD;
+uint256 constant _MIN_PARTIAL_LIQUIDATE_POSITION_VALUE = 100;

 library LibLiquidation {
     using SafeCast for uint256;
sherlock-admin4 commented 7 months ago

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

santipu_ commented:

Invalid - self.positionSize.abs is a value with 18 decimals (WAD). Therefore, positionValueAbs will have also 18 decimals (WAD).

takarez commented:

this does not ground to medium severity according to sherlock rules; there's no loss of funds nor breaking of a core function.

nevillehuang commented 6 months ago

Invalid, ETH is not supported as an collateral. Additionally, agree with santipu comments