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

During `_cancelFlow()` the depositor will still keep the shares for free

Summary

During specific scenarios, a user might get to keep all of his minted shares for free when the keeper calls the _cancelFlow() function.

Vulnerability Details

Imagine the following scenario.
1) The keeper calls the run() function to change a closed position. Suppose the position is closed (positionIsClosed).

2) The vault attempts to call _createIncreasePosition on GMX, which fails, and the afterOrderCancellation is called.

3) Right after the afterOrderCancellation is called, a new user deposits an amount with the deposit() function. Remember that the position is still closed here, so the new depositor will be minted shares in accordance with the amount deposited.

4) And now the bug occurs if the keeper decides to cancelFlow()(for example, because they want to try and re-run the previous action before the depositor stepped in), the following code will run:

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 {}

While the depositor gets back the entire amount deposited, he will also keep all of his shares for free.

Impact

Severity: High (since he gets all the shares for free). Likelihood: Low. (Since it is a specific scenario).

Tools Used

Manual Review.

Recommendations

Ensure that the minted shares from the deposit are also burnt.

Updates

Lead Judging Commences

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

finding_cancelFlow_can_be_called_after_order_execution_leading_to_disturb_shares_and_refund_too_many_fees

Likelihood: None/Very Low, when the keeper call cancelFlow after an order execution Impact: High, Inflation/deflation of total shares, and too many fees refunded.

Support

FAQs

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