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

The _cancelFlow function lacks of event emission when refundExecutionFee fails

Summary

The _cancelFlow function in the PerpetualVault contract is designed to handle the cancellation of ongoing flows, including refunding the ExecutionFee to the user. However, the current implementation uses a try-catch block to handle potential failures in the refundExecutionFee call without emitting an event. This lack of event emission makes it impossible for users to track whether the ExecutionFee was successfully refunded, leading to potential confusion and a poor user experience.

Vulnerability Details

The _cancelFlow function attempts to refund the ExecutionFee to the user using the refundExecutionFee function from the gmxProxy contract. The call is wrapped in a try-catch block to handle potential failures, but no event is emitted if the refund fails.

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

Impact

Users may assume their ExecutionFee was refunded when it was not, leading to dissatisfaction and disputes. The impact is Medium, the likelihood is Low, so the severity is Low.

Tools Used

Manual Review

Recommendations

To address this issue, the contract should emit an event when the ExecutionFee refund fails. This event should include relevant details such as the recipient address and the amount of the ExecutionFee.

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
) {
// Refund succeeded, no action needed
} catch {
// Refund failed, emit event
emit ExecutionFeeRefundFailed(depositInfo[counter].owner, depositInfo[counter].executionFee);
}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {
// Refund succeeded, no action needed
} catch {
// Refund failed, emit event
emit ExecutionFeeRefundFailed(depositInfo[counter].owner, depositInfo[counter].executionFee);
}
}
// Setting flow to liquidation has no meaning.
// The aim is to run FINAIZE action. (swap indexToken to collateralToken);
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}
Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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