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

Premature Deletion of `depositInfo` Before refundFee Processing

bhorprakash

Summary

In the _handleReturn function the depositInfo structure is deleted before executing the refund fee logic. This results in an issue where the executionFee field is always 0, causing incorrect or failed refund calculations.

Code Analysis
Deletion of depositInfo

_handleReturn calls _burn(depositId), which deletes depositInfo[depositId].
The deletion occurs before the refund logic in _handleReturn.

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;
//@audit depositInfo[depositId].executionFee will always 0
@=> 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;
}
​

_burn function

function _burn(uint256 depositId) internal {
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
totalShares = totalShares - depositInfo[depositId].shares;
@=> delete depositInfo[depositId];
}

As a result, any subsequent access to depositInfo[depositId] returns 0 or causes unintended behavior.

Impact

The refund logic attempts to access depositInfo[depositId].executionFee.
Since depositInfo has been deleted, executionFee defaults to 0.
This means the refundExecutionFee function will always receive incorrect values.

It will lead to users not receiving their due refunds.

Recommendation

Move Refund Logic Before _burn
Rearrange the function so that the refund logic executes before deleting depositInfo:

​
- 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[depositId].owner, depositInfo[depositId].executionFee - usedFee) {} catch {}
}
}
+ emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
+_burn(depositId); // Now deletion happens after refund is processed
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.