sherlock-audit / 2023-02-gmx-judging

17 stars 11 forks source link

joestakey - Incorrect adjusted amount calculation in `getAdjustedLongAndShortTokenAmounts()` always reverts #203

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

joestakey

medium

Incorrect adjusted amount calculation in getAdjustedLongAndShortTokenAmounts() always reverts

Summary

The cases are handled incorrectly, leading to the call always reverting, which means markets where the short and long token are the same will never work.

Vulnerability Detail

The function calculates the adjusted token amounts that would minimize the price impact. It first handles the case poolLongTokenAmount < poolShortTokenAmount. The issue is that it performs the incorrect subtraction poolLongTokenAmount - poolShortTokenAmount, which will underflow.

File: contracts/deposit/ExecuteDepositUtils.sol
392: if (poolLongTokenAmount < poolShortTokenAmount) {
393:             uint256 diff = poolLongTokenAmount - poolShortTokenAmount;//@audit underflow

The same problem happens in the other block, which will always underflow (unless both amounts are 0, which is not a relevant case in a deposit context)

File: contracts/deposit/ExecuteDepositUtils.sol
401: } else {
402:             uint256 diff = poolShortTokenAmount - poolLongTokenAmount;

Impact

getAdjustedLongAndShortTokenAmounts() always reverts. This function is called internally in executeDeposit when market.longToken == market.shortToken. This means deposits do not work in this case: markets where the long and short token are the same will simply not work.

Code Snippet

https://github.com/gmx-io/gmx-synthetics/blob/7be3ef2d119d9e84473e1a49f346bcdc06fd57a3/contracts/deposit/ExecuteDepositUtils.sol#L392-L402

Tool used

Manual Review

Recommendation

File: contracts/deposit/ExecuteDepositUtils.sol
392: if (poolLongTokenAmount < poolShortTokenAmount) {
-393:             uint256 diff = poolLongTokenAmount - poolShortTokenAmount;
+393:            uint256 diff = poolShortTokenAmount - poolLongTokenAmount;
File: contracts/deposit/ExecuteDepositUtils.sol
401: } else {
-402:             uint256 diff = poolShortTokenAmount - poolLongTokenAmount;
+402:             uint256 diff = poolLongTokenAmount - poolShortTokenAmount;

Duplicate of #165