Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Disposition of execution fee refunds due to early data deletion

bladesec

Title

Disposition of execution fee refunds due to early data deletion

Summary

A critical issue in the _handleReturn() function is causing the loss of user funds. The function deletes deposit information before processing potential refunds, making it impossible to return excess execution fees.

Vulnerability Details

The problem occurs in the _handleReturn() function when it calls _burn(), which removes the depositInfo mapping. This mapping contains essential data like the execution fee provided by the user. Later, when the code checks if depositInfo[depositId].executionFee exceeds usedFee to process a refund, the data is no longer available because it was deleted. As a result, users cannot recover excess fees, leading to financial loss.

1129: function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
1130: (uint256 depositId) = flowData;
1131: uint256 shares = depositInfo[depositId].shares;
1132: uint256 amount;
1133: if (positionClosed) {
1134: amount = collateralToken.balanceOf(address(this)) * shares / totalShares;
1135: } else {
1136: uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
1137: amount = withdrawn + balanceBeforeWithdrawal * shares / totalShares;
1138: }
1139: if (amount > 0) {
1140: _transferToken(depositId, amount);
1141: }
1142: emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
1143: _burn(depositId);
1144:
1145: if (refundFee) {
1146: uint256 usedFee = callbackGasLimit * tx.gasprice;
1147: if (depositInfo[depositId].executionFee > usedFee) {//@audit-issue depositInfo[depositId] was already burnt above
1148: try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
1149: }
1150: }
1151:
1152: // update global state
1153: delete swapProgressData;
1154: delete flowData;
1155: delete flow;
1156: }
1157:
​

Impact

Users cannot recover excess execution fees because the necessary data is deleted before the refund check.

Tools Used

Manual Review

Recommendations

Modify the _handleReturn() function to retrieve and store the execution fee before calling _burn().

Updates

Lead Judging Commences

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

finding_burn_depositId_before_refund

Likelihood: High, every time a user withdraw on 1x vault with paraswap Impact: Medium, fees never claimed to GMX and refund to the owner.

Support

FAQs

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