DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

No slippage protection on deposits during an active possition

Summary

During an active possition a user will be minted shares based on the current collateral they supply. However due to the lack of any slippage protection a user might receive unpredicatble shares in the protocol.

Vulnerability Details

When a user creates a deposit during an active possition they only specify the amount:

function deposit(uint256 amount) external nonReentrant payable {

However when they deposit during an active position, a keeper would be required to call runNextAction. The keeper will then create an order to the gmx protocol which will be based on the prices provided by the keeper:
https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/PerpetualVault.sol#L369
The order will then be executed by a gmx keeper and trigger afterOrderExecution at the time of execution.
The user's deposit will also be affected by any slippage which occurred between the time when user initiated a deposit and the increase order was finally executed by the gmx keeper:

function afterOrderExecution(
bytes32 requestKey,
bytes32 positionKey,
IGmxProxy.OrderResultData memory orderResultData,
MarketPrices memory prices
) external nonReentrant {
...
if (flow == FLOW.DEPOSIT) {
uint256 amount = depositInfo[counter].amount;
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, orderResultData.sizeDeltaUsd, false) / prices.shortTokenPrice.min;
uint256 prevSizeInTokens = flowData;
>>> int256 priceImpact = vaultReader.getPriceImpactInCollateral(curPositionKey, orderResultData.sizeDeltaUsd, prevSizeInTokens, prices);
uint256 increased;
if (priceImpact > 0) {
increased = amount - feeAmount - uint256(priceImpact) - 1;
} else {
increased = amount - feeAmount + uint256(-priceImpact) - 1;

However the time between the order submission and the order execution can vary and the user will not have any control over the slippage.

Impact

Users will be distributed shares they have no control over.

Tools Used

Manual Review

Recommendations

Add slippage protection

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_shares_slippage

Shares represent a part of the vault. Even if someone performs a frontrun or sandwich attack, you will still have the corresponding amount of shares representing your deposit. A user could add liquidity two days later, and you would still have the same amount of shares.

Support

FAQs

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

Give us feedback!