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

`_cancelFlow` will revert if depositor becomes blacklisted

Summary

Unlike in other parts of the contract where Try&Catch is used to ensure that failed USDC transfers don't halt the protocol, this is not guaranteed in _cancelFlow.

Vulnerability Details

The vulnerability lies in the transferring of the deposited amount back to the depositor:

function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
@> collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);//@audit
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 {}
}

Impact

The protocol may not be able to conclude this flow and get stuck.

Tools Used

Manual Review

Recommendations

Consider either:

  1. using Try&Catch to send these funds to treasury, as in other parts of the code

  2. or storing these balances in a mapping for the users to collect later.

Updates

Lead Judging Commences

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

finding_cancelFlow_blacklisted

Likelihood: Extremely Low, when user is blacklisted between the deposit/withdraw and cancelFlow is called by the Keeper. Impact: Medium/High, cancelFlow DoS.

Support

FAQs

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