Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

When a withdraw flow is cancelled, it should not always refund the entire executionFee.

pkqs90

Summary

When a withdraw flow is cancelled, it should not always refund the entire executionFee.

Vulnerability Details

The withdraw flow consists of two decreasePosition orders. The first one is for settling funding fees. The second one is for withdraw.

When users pay the executionGasLimit of the withdraw flow, it pays 2 times the MarketDecrease gas limit.

However, when the withdraw flow is cancelled, it doesn't take account of whether the first order is finished or not, and refunds the full executionFee. Ideally, if the first order is finished, and the second order is pending, only half of the executionFee should be refunded.

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/main/contracts/PerpetualVault.sol#L667

function getExecutionGasLimit(bool isDeposit) public view returns (uint256 minExecutionGasLimit) {
if (positionIsClosed == false) {
if (_isLongOneLeverage(beenLong)) {
minExecutionGasLimit = gmxProxy.getExecutionGasLimit(Order.OrderType.MarketSwap, callbackGasLimit);
} else {
if (isDeposit) {
minExecutionGasLimit = gmxProxy.getExecutionGasLimit(Order.OrderType.MarketIncrease, callbackGasLimit);
} else {
// withdraw action has 2 gmx call parts: settle + decrease position
@> minExecutionGasLimit = gmxProxy.getExecutionGasLimit(Order.OrderType.MarketDecrease, callbackGasLimit) * 2;
}
}
}
}
​
​
function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
@> try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
}
// Setting flow to liquidation has no meaning.
// The aim is to run FINAIZE action. (swap indexToken to collateralToken);
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}

Impact

User gets refunded extra gas fee.

Tools Used

N/A

Recommendations

Only refund the gas fee for orders that did not finish.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_cancelFlow_can_be_called_between_settle_and_withdraw

Likelihood: None/Very Low, when any flow is cancelled after a settle and before a withdraw. No real reason to do that. Impact: High, loss of funds.

Support

FAQs

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