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

close partial position leads to withdraw more tokens

Summary

function _finalize(bytes memory data) internal {
if (flow == FLOW.WITHDRAW) {
(uint256 prevCollateralBalance, bool positionClosed, bool refundFee) = abi.decode(data, (uint256, bool, bool));
uint256 withdrawn = collateralToken.balanceOf(address(this)) - prevCollateralBalance;
_handleReturn(withdrawn, positionClosed, refundFee);
} else {
delete swapProgressData;
delete flowData;
delete flow;
}
}

When the _finalize function is executed through the withdraw function, it calls the _handleReturn function with the actual withdrawal amount and the positionClosed status as parameters.

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 {}
}
}

The issue arises when positionClosed is false. The logic above allows the user to withdraw more tokens than intended.

Let’s illustrate with an example.

Assume the ratio of tokens to shares is 1:1, and the total amount of tokens is 120.

  1. The attacker deposits 20 tokens to open a position. (Total tokens: 140, Attackers’ shares: 20)

  2. The attacker closes 19 tokens of the position. (withdrawn = 19)

—> amount = 19 (withdrawn) + (140 - 19) (balanceBeforeWithdrawal) * 20 (shares) / 140 (totalShares)

—> amount = 19 + 17.28 ⇒ 36.28

—> The withdrawal amount is 36 tokens, which is more than the 20 tokens originally deposited.

Now, let's assume the attacker closes all positions.

  1. The attacker deposits 20 tokens to open a position. (Total tokens: 140, Attackers’ shares: 20)

  2. The attacker closes all positions. 140 (total tokens) * 20 (shares) / 140 (totalShares) ⇒ 20

The attacker receives the same 20 tokens they initially deposited.

Rerefences

Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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