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

Missing totalShares Update in cancelFlow() Leads to Inflated Share Calculations

krisrenzo

Summary

When a deposit is cancelled using cancelFlow(), the function fails to update totalShares, leading to inflated share value and incorrect shares calculations for subsequent deposits.

Vulnerability Details

When users deposit into the vault when no position is open (positionIsClosed == true), the following sequence occurs:

  1. User deposits tokens via deposit()

  2. The deposit immediately calls _mint() which:

    • Calculates and assigns shares to the depositor

    • Updates totalShares

  3. If the deposit needs to be cancelled via cancelFlow():

    • The deposited tokens are refunded

    • depositInfo is cleared

    • However, totalShares is not decremented

This creates a discrepancy where the totalShares remains inflated even though the corresponding deposit was cancelled and refunded.

Impact Details

The inflated totalShares affects the share calculation formula used in _mint():

_shares = amount * totalShares / totalAmountBefore

Because totalShares remains artificially high after cancelled deposits, subsequent depositors will receive fewer shares than they should for their deposited amount. This creates an unfair distribution of shares and incorrect representation of users' ownership in the vault.

Tools Used

Manual Review

Recommendations

Update _cancelFlow() to properly decrement totalShares when cancelling a deposit:

function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
totalShares = totalShares - depositInfo[depositId].shares; // Add this line
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
delete depositInfo[depositId];
}
// ... rest of the function
}
Updates

Lead Judging Commences

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

finding_cancelFlow_can_be_called_after_order_execution_leading_to_disturb_shares_and_refund_too_many_fees

Likelihood: None/Very Low, when the keeper call cancelFlow after an order execution Impact: High, Inflation/deflation of total shares, and too many fees refunded.

Support

FAQs

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

Give us feedback!