Steadefi

Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Submission Details
Severity: medium
Valid

`depositValue` is calculated with wrong argument

Summary

Wrong arguments are used in the gmxOracle function, which could lead to unexpected reverts.

Vulnerability Details

When a user wants to make a deposit with lpToken the depositeValue is calculated through the gmxOracle. However the arguments are indicating that the action performed will be a withdraw, this is problematic because depending on the PnL state of the market, the results could be significantly different for a withdraw than for a deposit.
This perhaps wrongfully calculated depositValue will be the foundation to calculate another vital element of the deposit process, the minMarketTokenAmt, which is the deposit value adjusted to the the possible slippage.
The default slippage tolerance is set to a relatively low amount, 0.03% which doesn't offer for much price deviation. The minMarketTokenAmt will be the minimum amount of tokens that the user settles for in this trade. We can see in the gmx contract that if the calculated amount of tokens that the protocol will deliver is lesser than minMarketTokenAmt the transaction will revert (https://github.com/gmx-io/gmx-synthetics/blob/228f2155a69a1be3e99614b4ade0f65e86b0209b/contracts/deposit/ExecuteDepositUtils.sol#L222-L226).

Unexpected revert scenario:

-A user wants to make a deposit with lpToken

-Because of the current market conditions, the returned value for deposits happens to be slightly lower than for
withdrawals (-0.04%).

-The minMarketTokens value is bigger than the amount of tokens the protocol is able to deliver to the user.

-Transaction reverts, user loses gas money

if the difference between the price fetched for deposit is slightly lower than the

Impact

The protocol might expose himself to having deposit transactions unexpectedly revert. This vulnerability is graded as medium because it will affect the normal functioning of the protocol and unexpectedly revert transactions.

Tools Used

Manual review, GMX v2 docs

Recommendations

When calculating the deposit value, it is important to set the isDeposit argument as true instead of false such as:

if (dp.token == address(self.lpToken)) {
// If LP token deposited
_dc.depositValue = self.gmxOracle.getLpTokenValue(
address(self.lpToken), //marketToken
address(self.tokenA), //indexToken
address(self.tokenA), //longToken
address(self.tokenB), //shortToken
true, //isDeposit
false //maximize
)

This will allow for more accurate calculations and justified reverts.

Updates

Lead Judging Commences

hans Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong PNL Factor in GMXDeposit for for lpToken

Impact: Medium Likelihood: High The impact of using a wrong price (conservative vs optimistic) is limited, especially given the users specify the slippage.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.