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

GMXDeposit - deposit() function passes a wrong flag to gmxOracle.getLpTokenValue()

0xasen

Summary

The deposit() function calls gmxOracle.getLpTokenValue with wrong flag for isDeposit.

Vulnerability Details

Let's take a look at this part of the code of the deposit() function:

_dc.depositValue = self.gmxOracle.getLpTokenValue(
address(self.lpToken),
address(self.tokenA),
address(self.tokenA),
address(self.tokenB),
false, <--
false
)

and take a look at the getLpTokenValue() in GMXOracle:

function getLpTokenValue(
address marketToken,
address indexToken,
address longToken,
address shortToken,
bool isDeposit, <--
bool maximize
) public view returns (uint256) {
bytes32 _pnlFactorType;
​
if (isDeposit) {
_pnlFactorType = keccak256(abi.encode("MAX_PNL_FACTOR_FOR_DEPOSITS"));
} else {
_pnlFactorType = keccak256(abi.encode("MAX_PNL_FACTOR_FOR_WITHDRAWALS")); <--
}
​
(int256 _marketTokenPrice,) = getMarketTokenInfo(
marketToken,
indexToken,
longToken,
shortToken,
_pnlFactorType,
maximize
);
​
// If LP token value is negative, return 0
if (_marketTokenPrice < 0) {
return 0;
} else {
// Price returned in 1e30, we normalize it to 1e18
return uint256(_marketTokenPrice) / 1e12;
}
}

As you can see, we're passing isDeposit == false so the _pnlFactorType will be for withdrawals.

Impact

I doubt this has much impact, depends on the way the GMX system utilizes it, but may cause some problems on the front-end for example, displaying a wrong pnlFactor. I still think this deserves a Low severity

Tools Used

Manual review

Recommendations

Pass it as true because we're depositing.

Updates

Lead Judging Commences

hans Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xasen Submitter
almost 2 years ago
hans Auditor
almost 2 years ago
0xasen Submitter
almost 2 years ago
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.