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

In some places, counter is used as the ID value instead of the ID of the order that is being processed.

Summary

The protocol uses counter mechanism as an ID for new orders. In some cases, the PerpetualVault contract uses counter as the ID for deposit orders to retrieve the latest order. However, in certain places, this approach is incorrect, and only the specific depositId should be used. Otherwise, data from the latest order will be used instead of the one currently being processed.

Vulnerability Details

Let's take a look at the _handleReturn function:

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
uint256 shares = depositInfo[depositId].shares;
uint256 amount;
if (positionClosed) {
amount = (collateralToken.balanceOf(address(this)) * shares) / totalShares;
} else {
uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
amount = withdrawn + (balanceBeforeWithdrawal * shares) / totalShares;
}
if (amount > 0) {
_transferToken(depositId, amount);
}
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId);
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}
// Update global state
delete swapProgressData;
delete flowData;
delete flow;
}

As we can see, in the refunding logic, counter is used as the ID for depositInfo. However, withdrawals can be made for previously created deposits, while counter only refers to the latest one. This creates a problem: even if a user has provided sufficient gas fees and is eligible for a refund, they may not receive it. This is because the latest order might have used less gas, causing the refund calculation to underflow and the try/catch block to skip execution.

Impact

Users may be unable to receive gas refunds in some cases or may receive an incorrect refund amount.

Tools Used

Manual review.

Recommendations

Use depositId, as it is done in other parts of the code.

Updates

Lead Judging Commences

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

finding_counter_invalid_during_handleReturn

Likelihood: Medium/High, when withdraw on a 1x vault. 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.