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

Vault Liquidation Causes Unfair Share Allocation & Residual State Issues

Description
When a vault undergoes liquidation, its assets are set to zero, and it is considered "disposed of." However, there are two critical issues:

  1. Unfair Share Allocation to the First Depositor

  • If totalShares are not reset after liquidation, the next depositor could receive an excessive number of shares.

  • This happens due to the conditions:

if (totalAmountBefore == 0) totalAmountBefore = 1;
  • If totalAmountBefore is 0, it is artificially set to 1, leading to incorrect share calculations.

  • The first depositor post-liquidation ends up receiving a massively inflated share allocation.

  1. Residual State Issues in the Vault

    • If key state variables such as totalShares, totalAmount, or depositInfo are not reset, they may retain stale values from the previous vault state.

    • This can cause incorrect accounting for future deposits and withdrawals, making the vault behave unpredictably.

    • If any residual funds remain post-liquidation, they could be misallocated when new deposits occur.

Impact

  1. Unfair Advantage to Early Depositors

    • The first depositor post-liquidation can receive more shares than they should.

    • This can drastically affect reward distribution among users.

  2. Incorrect Accounting and Possible Exploit

    • If vault variables are not properly reset, users may manipulate deposits to benefit from the leftover vault state.

    • Deposits and withdrawals could be computed incorrectly, leading to unexpected losses for some users.

Poc
Issue 1: Unfair Share Allocation

  • The _mint() function contains the logic for computing share allocation:

function _mint(
uint256 depositId,
uint256 amount,
bool refundFee,
MarketPrices memory prices
) public {
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;
}
//@audit This condition creates an unfair advantage for the first depositor
@> 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);
}
  • How the Bug Occurs (Step-by-Step Reproduction)

  1. Vault undergoes liquidation, setting assets to zero.

  2. Key vault variables (totalShares, totalAmount) are NOT reset.

  3. A new depositor deposits funds after liquidation.

  4. totalAmountBefore is 0, triggering this condition:

if (totalAmountBefore == 0) totalAmountBefore = 1;
  1. Share calculation becomes incorrect because totalAmountBefore is artificially set to 1.

  • EXAMPLE Pre-Liquidation

amount = 1000; // Deposited amount
totalShares = 10000; // Existing shares
totalAmountBefore = 1000; // Correct vault balance
_shares = (1000 * 10000) / 1000 = 10000 // Fair distribution
  • EXAMPLE Post-Liquidation

amount = 1000; // First deposit after liquidation
totalShares = 10000; // Not reset after liquidation
totalAmountBefore = 0; // Due to liquidation
// Condition triggers:
totalAmountBefore = 1;
_shares = (1000 * 10000) / 1 = 10,000,000 // WAY TOO HIGH!
  • The first depositor gets unfair shares, impacting the entire system.

ISSUE 2 -> Residual problem

  • If other vault variables are not reset, we face additional issues:

    • Residual funds might remain unaccounted for, leading to incorrect share calculations.

    • Future withdrawals may be miscalculated, affecting users who try to exit.

Steps for Reproduction:

  1. Trigger liquidation to set assets to 0.

  2. Check vault variables like totalShares, totalAmount, and depositInfo to see if they retain old values.

  3. Deposit after liquidation and observe:

    • Are the calculations correct?

    • Is Vault still accounting for past deposits?

Recommended Mitigation

  • Explicitly Reset all the state variable

  • Ensure that after liquidation, all key variables (totalShares, totalAmount, etc.) are reset to zero to prevent incorrect calculations.

  • Ensure Proper Handling of Residual Funds

    • If any assets remain post-liquidation, properly distribute or account for them instead of ignoring them

Updates

Lead Judging Commences

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

finding_full_liquidation_do_not_reset_totalShares

Likelihood: Low/Medium, when fully liquidated. Liquidation often returns some tokens and shares are important to withdraw them. Moreover, shares are inflated, so only little part of tokens with huge value (WBTC) will be impacted. Impact: High, Previous depositor is able to withdraw token from the new depositors if the value of the token is huge like for WBTC.

Appeal created

wellbyt3 Auditor
5 months ago
parth Submitter
5 months ago
n0kto Lead Judge
4 months ago
n0kto Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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