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

Give us feedback!