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

Wrong PNL Factor for Deposits in `GMXdeposit` Contract Can Lead to Excessive Profits and Losses for Depositors

ro1sharkm

Summary

The vulnerability in the GMXDeposit contract exposes depositors to varying levels of maximum profits and losses during deposits, contrary to the intended risk parameters, as a result of the misapplied Profit and Loss (PNL) factor. This vulnerability jeopardizes the financial equilibrium between users, the protocol and the vault."

Vulnerability Details

The vulnerability arises from the incorrect usage of the PNL factor. In the deposit() function, the isDeposit parameter is consistently set to "false." Consequently, the getLpTokenValue() function employs the MAX_PNL_FACTOR_FOR_WITHDRAWALS parameter for deposit value calculations, rather than the intended MAX_PNL_FACTOR_FOR_DEPOSITS. This misalignment between deposits and withdrawals leads to unpredictably advantageous or detrimental outcomes for depositors.

_dc.depositValue = self.gmxOracle.getLpTokenValue(
address(self.lpToken),
address(self.tokenA),
address(self.tokenA),
address(self.tokenB),
false,
false
)
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"));
}

Impact

The vulnerability exposes depositors to variable maximum profits and losses during deposits, contrary to the intended risk parameters. This can benefit depositors seeking higher profits but potentially harms the protocol and vault. Deposit limits may be exceeded, compromising the system's security.

Tools Used

Manual analysis

Recommendations

Developers should adjust the isDeposit parameter in the deposit() function to "true," ensuring the getLpTokenValue() function correctly employs the MAX_PNL_FACTOR_FOR_DEPOSITS parameter for deposit value calculations.

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.