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 10 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.

Give us feedback!