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

the wrong owner gets the execution fee

Vulnerability details

In such functions as _mint and _handleReturn after burning a deposit, the execution fee is transferred to the owner of the deposit, but since the deposit by depositId is burned and the fee is transferred to the owner of the last deposit, because the counter displays the number of all deposits and respectively the last created one.

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
//code
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 {}
}
}
//code
function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
uint256 _shares;
if (totalShares == 0) {
_shares = depositInfo[depositId].amount * 1e8;
} else {
uint256 totalAmountBefore;
if (positionIsClosed == false && _isLongOneLeverage(beenLong)) {
totalAmountBefore = IERC20(indexToken).balanceOf(address(this)) - amount;
} else {
totalAmountBefore = _totalAmount(prices) - amount;
}
if (totalAmountBefore == 0) totalAmountBefore = 1;
_shares = amount * totalShares / totalAmountBefore;
}
depositInfo[depositId].shares = _shares;
totalShares = totalShares + _shares;
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[counter].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}
emit Minted(depositId, depositInfo[depositId].owner, _shares, amount);
}

Impact

Users who withdraw their tokens, in case the check is passed, the commission is received not by them but by other users.

Recommended Mitigation Steps

replace counter with depositId

Updates

Lead Judging Commences

n0kto Lead Judge 8 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.