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

cancelFlow does not revert totalShares to its previous state.

Summary

The protocol is designed to create and execute a flow fully across multiple transactions before another flow can begin. While the keeper has the ability to cancel a flow, the current implementation does not fully revert all operations upon cancellation. Specifically, it fails to update totalShares after a cancellation, causing it to retain its previous value even after a deposit or withdrawal is canceled.

Vulnerability Details

The vulnerability exists in _cancelFlow:

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 the FINALIZE action (swap indexToken to collateralToken).
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}

As shown above, the function cancels a withdrawal or deposit, but it fails to update totalShares, even though it refunds funds to the user and updates totalDepositAmount.

Impact

The totalShares value will become inaccurate, leading to inflation. Since the actual shares and totalShares ratio will be corrupted, this could create unintended imbalances within the vault.

Tools Used

Manual review.

Recommendations

Update totalShares accordingly in the cancellation flow.

Updates

Lead Judging Commences

n0kto Lead Judge 8 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.