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

ADL can result in wrong withdrawn amount for users

vinica_boy

Summary

If ADL occurs right before finalization of a withdrawal, all funds from the ADL will be wrongly send to the withdrawer.

Vulnerability Details

In order for a user to withdraw from vault, there is decrease order which when executed will trigger GmxProxy::afterOrderExecution() which internally calls PerpetualVault::afterOrderExecution(). The following snippet from PerpetualVault::afterOrderExecution() handles what should happen next:

if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.FINALIZE;
uint256 prevCollateralBalance = collateralToken.balanceOf(address(this)) - orderResultData.outputAmount;
nextAction.data = abi.encode(prevCollateralBalance, sizeInUsd == 0, false);
}

We save the prevCollateralBalance based on the output of decrease order and Gamma keeper should call runNextAction() to finalize withdrawal and calculate how much collateral token should the user get.

In runNextAction() we have the following logic to handle withdrawal:

else if (_nextAction.selector == NextActionSelector.FINALIZE) {
if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
_finalize(_nextAction.data);
}

All index tokens would be swapped to collateral token and then in _finalize() we have

function _finalize(bytes memory data) internal {
if (flow == FLOW.WITHDRAW) {
(uint256 prevCollateralBalance, bool positionClosed, bool refundFee) = abi.decode(data, (uint256, bool, bool));
uint256 withdrawn = collateralToken.balanceOf(address(this)) - prevCollateralBalance;
_handleReturn(withdrawn, positionClosed, refundFee);
} else {
delete swapProgressData;
delete flowData;
delete flow;
}
}
function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
uint256 shares = depositInfo[depositId].shares;
uint256 amount;
if (positionClosed) {
// @audit after liquidation/adl problems if output is two tokens
amount = collateralToken.balanceOf(address(this)) * shares / totalShares;
} else {
uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
amount = withdrawn + balanceBeforeWithdrawal * shares / totalShares;
}
if (amount > 0) {
_transferToken(depositId, amount);
}
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId);
​
if (refundFee) {
// @audit different gasprice between order execution and runNextAction(finalize), incorrect calculation
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}
​
// update global state
delete swapProgressData;
delete flowData;
delete flow;
}

If ADL happens before finalizing withdrawal, all funds from the ADL wont be accounted in prevCollateralBalance and will be accounted as withdrawn from the decrease order. This is incorrect and withrawer will get much more collateral token than what he should.

Impact

Wrong withdraw amount for one user and loss of funds for all other users.

Tools Used

Manual review

Recommendations

Consider explicitly handling this scenario and update nextAction.data if needed when ADL occurs.

Updates

Lead Judging Commences

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

finding_ADL_before_a_finalize_withdraw_profit_to_one_user

Likelihood: Low, when ADL with profit happen just before a nextAction.FINALIZE and FLOW.WITHDRAW Impact: High, the withdrawing user receives all the delivery with the tokens.

Support

FAQs

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

Give us feedback!