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

Withdrawal Race Condition Leads to Unfair Fund Distribution in PerpetualVault

chainsentryaudits

Summary and Impact

The PerpetualVault contract has a significant flaw in its withdrawal mechanism where users withdrawing in parallel can receive incorrect amounts of funds due to stale position data being used in calculations. This directly violates the protocol's core invariant of "Fair Share of Funding Fees" and "Depositor Share Value Preservation" as stated in the documentation.

According to the documentation: "Depositor Share Value Preservation: The value of a depositor's shares should never decrease due to the actions of other depositors. Any losses or gains should be distributed proportionally based on share ownership."

However, the current implementation fails to maintain this invariant during parallel withdrawals, leading to disproportionate fund distribution.

Vulnerability Details

The vulnerability lies in the _withdraw function, where position data used for calculating withdrawal amounts isn't properly synchronized with the actual state of the vault:

function _withdraw(uint256 depositId, bytes memory metadata, MarketPrices memory prices) internal {
uint256 shares = depositInfo[depositId].shares;
if (shares == 0) {
revert Error.ZeroValue();
}
if (positionIsClosed) {
_handleReturn(0, true, false);
} else if (_isLongOneLeverage(beenLong)) { // beenLong && leverage == BASIS_POINTS_DIVISOR
uint256 swapAmount = IERC20(indexToken).balanceOf(address(this)) * shares / totalShares;
nextAction.selector = NextActionSelector.SWAP_ACTION;
// abi.encode(swapAmount, swapDirection): if swap direction is true, swap collateralToken to indexToken
nextAction.data = abi.encode(swapAmount, false);
} else if (curPositionKey == bytes32(0)) { // vault liquidated
_handleReturn(0, true, false);
} else {
IVaultReader.PositionData memory positionData = vaultReader.getPositionInfo(curPositionKey, prices);
uint256 collateralDeltaAmount = positionData.collateralAmount * shares / totalShares;
uint256 sizeDeltaInUsd = positionData.sizeInUsd * shares / totalShares;
// we always charge the position fee of negative price impact case.
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, sizeDeltaInUsd, false) / prices.shortTokenPrice.max;
int256 pnl = vaultReader.getPnl(curPositionKey, prices, sizeDeltaInUsd);
if (pnl < 0) {
collateralDeltaAmount = collateralDeltaAmount - feeAmount - uint256(-pnl) / prices.shortTokenPrice.max;
} else {
collateralDeltaAmount = collateralDeltaAmount - feeAmount;
}
uint256 acceptablePrice = abi.decode(metadata, (uint256));
_createDecreasePosition(collateralDeltaAmount, sizeDeltaInUsd, beenLong, acceptablePrice, prices);
}
}

Consider this scenario:

  1. The vault has 5000 USDC total collateral

  2. Alice, Bob, Charlie, David, and Eve each deposit 1000 USDC (20% shares each)

  3. Alice initiates withdrawal

  4. Before Alice's withdrawal processes:

    • Bob withdraws successfully (800 USDC received after fees)

    • Charlie withdraws successfully (800 USDC received after fees)

    • David withdraws successfully (800 USDC received after fees)

  5. When Alice's withdrawal finally processes, it uses the original position data (5000 USDC), calculating her share as 1000 USDC instead of 320 USDC (20% of remaining 1600 USDC)

  6. Eve, the last user, is left with significantly less than their fair share

This is particularly concerning because the documentation explicitly states: "There could be delays in claiming some funding fees. If the user withdraws prior to the ability to claim, then it would be ok not to receive his fair share." However, this vulnerability causes early withdrawers to receive MORE than their fair share, leaving later withdrawers with less than they deserve.

function testMultipleParallelWithdrawalsIncorrectAmount() public {
// Setup: 5 users each deposit 1000 USDC
address[] memory users = new address[](5);
users[0] = alice;
users[1] = bob;
users[2] = charlie;
users[3] = david;
users[4] = eve;
​
for(uint i = 0; i < users.length; i++) {
vm.startPrank(users[i]);
deal(address(vault.collateralToken()), users[i], 1000e6);
vault.deposit{value: executionFee}(1000e6);
vm.stopPrank();
}
​
// Create position and wait for lock period
createPosition();
vm.warp(block.timestamp + vault.lockTime() + 1);
​
// Store initial position data
uint256 initialCollateral = getVaultCollateral();
​
// Users initiate withdrawals in sequence
for(uint i = 0; i < users.length; i++) {
uint256[] memory userDeposits = vault.getUserDeposits(users[i]);
vm.prank(users[i]);
vault.withdraw{value: executionFee}(users[i], userDeposits[0]);
}
​
// Process withdrawals in specific order
processWithdrawal(bob); // Gets ~800 USDC
processWithdrawal(charlie); // Gets ~800 USDC
processWithdrawal(david); // Gets ~800 USDC
processWithdrawal(alice); // Gets ~1000 USDC (incorrect amount)
processWithdrawal(eve); // Gets significantly less
​
// Verify the unfair distribution
uint256 aliceBalance = vault.collateralToken().balanceOf(alice);
uint256 eveBalance = vault.collateralToken().balanceOf(eve);
console.log("Alice final balance:", aliceBalance);
console.log("Eve final balance:", eveBalance);
// Alice should get more than her fair share
assertGt(aliceBalance, initialCollateral / 5);
// Eve should get less than her fair share
assertLt(eveBalance, initialCollateral / 5);
}

This scenario better demonstrates the real-world impact because:

  1. It shows how multiple withdrawals can compound the issue

  2. It demonstrates how later users can be significantly impacted

  3. It's more realistic with 5 users instead of just 2

  4. It accounts for protocol fees in the calculations

  5. It shows how the total withdrawn amount can exceed what should be available

Tools Used

  • Manual Review

  • Foundry

Recommendations

Implement a withdrawal queue system that captures position state at the time of withdrawal request:

struct WithdrawalRequest {
uint256 snapshotCollateral;
uint256 snapshotSize;
uint256 shares;
address recipient;
}
​
mapping(uint256 => WithdrawalRequest) public withdrawalQueue;
​
function _withdraw(uint256 depositId, bytes memory metadata, MarketPrices memory prices) internal {
WithdrawalRequest storage request = withdrawalQueue[depositId];
uint256 collateralDeltaAmount = request.snapshotCollateral * request.shares / totalShares;
// ... continue with withdrawal using snapshot values ...
}

This ensures that withdrawal amounts are calculated based on the position state at the time of request, maintaining fair distribution of funds among users.

Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Too generic
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.