sherlock-audit / 2023-04-gmx-judging

2 stars 1 forks source link

ten-on-ten - Logical error when computing `estimatedRemainingCollateralUsd` #277

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ten-on-ten

high

Logical error when computing estimatedRemainingCollateralUsd

Summary

                estimatedRemainingCollateralUsd += params.order.initialCollateralDeltaAmount().toInt256();
                params.order.setInitialCollateralDeltaAmount(0);

here, collateral amount is not multiplied with the price of collateral when incrementing estimatedRemainingCollateralUsd. It assumes price to be 1 USD (without precision) for any given collateral.

Vulnerability Detail

initialCollateralDelta is the user input token which may or may not be stablecoin, so the worth of it should be evaluated with respect to its price which is not currently happening.

Impact

It will lead to ambiguous processing of position's collateral and might give lot of profit to user or liquidate them depending on the actual price of collateral.

Code Snippet

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/position/DecreasePositionUtils.sol#L139-L143

Tool used

Manual Review

Recommendation

Multiply with collateral token price with proper precision and added test-cases where collateral token is non USDC

Duplicate of #249