DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: medium
Valid

executionFee is refunded to the wrong user when keeper cancels a WITHDRAW Flow.

Summary

The executionfee is refunded when the keeper calls cancelFlow().
In case of deposit flow, the executionFee is correctly handled by transferring the fees to the depositInfo[counter].owner since counter denotes id of the last deposit.

But in case of withdraw , using counter as depositId is incorrect since the actual depositId is stored in the flowData.

Vulnerability Details

When a user calls withdraw(recipient,depositId) , the depositId is stored in the variable flowData. code

But when a keeper calls cancelFlow() later , the depositId of current processing deposit is taken from counter instead of flowData.

code

function _cancelFlow() internal {
....
else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
}

Similar bug can be seen in handleReturn() too.code . This get triggered when a keeper executes decreaseOrder for a 1x long swap maket by only using dexSwap.

Impact

When keeper cancels a withdraw flow , the executionFee is refunded to the wrong account(the owner of the last deposit).

Tools Used

Manual

Recommendations

use flowData instead of counter.

Updates

Lead Judging Commences

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

finding_counter_invalid_during_cancelFlow_after_withdrawing

Likelihood: Low, contract has to call cancelFlow after a withdraw, and the settle action is already executed by GMX. Impact: High, the fees will be distributed to the last depositor and not the withdrawer.

Support

FAQs

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