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

Unfair Share Distribution During Position Open State vs Close State

mohitisimmortal

Summary

When the vault position is open(positionIsClosed == false), deposits are processed through a deferred mechanism that involves off-chain signals and token swaps. During this process, the keeper-provided metadata triggers a GMX swap and subsequent order execution. In the afterOrderExecution() callback for MarketIncrease orders, the vault calculates an “increased” amount based on the deposited amount, fees, and a price impact factor before calling _mint(). This formula can result in deposits minted with more (or less) shares compared to deposits when the position is closed(positionIsClosed == true), leading to unfair distribution.

Vulnerability Details

Users get different number of shares depending on position is open or closed(irrespective of same collateral amount).

  • if user deposits during position is closed then shares mints directly.

  • if user deposits during position is open then below flow is followed:

  1. User calls deposit(), payexecutionfee and set- NextActionSelector.INCREASE_ACTION

  2. Now keeper runs- runNextAction() with metadata.length==2, which makes a gmx swap.

  3. After swap, now flow comes to afterOrderExecution() in vault, here it mint shares with an increased amount:

481 if (orderResultData.orderType == Order.OrderType.MarketIncrease) {
482 curPositionKey = positionKey;
483 if (flow == FLOW.DEPOSIT) {
484 uint256 amount = depositInfo[counter].amount;
485 uint256 feeAmount = vaultReader.getPositionFeeUsd(market, orderResultData.sizeDeltaUsd, false) / prices.shortTokenPrice.min;
486 uint256 prevSizeInTokens = flowData;
487 int256 priceImpact = vaultReader.getPriceImpactInCollateral(curPositionKey, orderResultData.sizeDeltaUsd, prevSizeInTokens, prices);
488 uint256 increased;
489 if (priceImpact > 0) {
490@> increased = amount - feeAmount - uint256(priceImpact) - 1;
491 } else {
492@> increased = amount - feeAmount + uint256(-priceImpact) - 1;
493 }
494@> _mint(counter, increased, false, prices);
495 nextAction.selector = NextActionSelector.FINALIZE;
496 } else {

  • In test case, one depositor received 1,000,000 shares while another received 1,003,806 shares for the same collateral deposit amount.

Impact

High

  • Unfair shares distribution: This unfair distribution may lead to one user eventually withdrawing more collateral than their fair share while diluting other depositors’ stakes.

Likelihood

High

  • High, particularly under volatile market conditions where GMX swaps occur frequently and price impacts are significant.

POC

Most of test code is mimcked from protocol's test/PerpetualVault.t.sol::test_DepositIntoGmx().

  1. Add test function into test/PerpetualVault.t.sol:

function test_UnfairDistribution() external {
address alice = makeAddr("alice");
payable(alice).transfer(1 ether);
depositFixture(alice, 1e12);
uint256[] memory userDeposits = PerpetualVault(vault).getUserDeposits(alice);
(, uint256 userShares1, , , , ) = PerpetualVault(vault).depositInfo(userDeposits[0]);
​
console.log("Alice shares after deposits:", userShares1/1e14); // Dividing by 1e14- bcz for every 100 usdc- it mints 100*1e6*1e8 shares.
​
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](2);
data[0] = abi.encode(3380000000000000);
address keeper = PerpetualVault(vault).keeper();
vm.prank(keeper);
PerpetualVault(vault).run(true, false, prices, data);
GmxOrderExecuted(true);
bytes[] memory metadata = new bytes[](0);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, metadata);
​
// Another user(tom) comes after a position is open.
address tom = makeAddr("tom");
payable(tom).transfer(1 ether);
​
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 amount = 1e12;
deal(address(collateralToken), tom, amount);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
vm.startPrank(tom);
collateralToken.approve(vault, amount);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(amount);
vm.stopPrank();
​
uint8 flow = uint8(PerpetualVault(vault).flow());
assertEq(flow, 1);
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 1);
​
data[0] = abi.encode(3380000000000000);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, data);
GmxOrderExecuted(true);
(selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 6);
flow = uint8(PerpetualVault(vault).flow());
assertNotEq(flow, 0);
vm.prank(keeper);
delete data;
PerpetualVault(vault).runNextAction(prices, data);
(selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 0);
flow = uint8(PerpetualVault(vault).flow());
assertEq(flow, 0);
​
uint256[] memory userDeposits2 = PerpetualVault(vault).getUserDeposits(tom);
(, uint256 userShares2, , , , ) = PerpetualVault(vault).depositInfo(userDeposits2[0]);
console.log("Tom shares after deposits:", userShares2/1e14);
}

  1. Run by: forge test --match-test test_UnfairDistribution --rpc-url https://arb1.arbitrum.io/rpc -vvv --via-ir

  2. Output:

[PASS] test_UnfairDistribution() (gas: 7652456)
Logs:
Alice shares after deposits: 1000000
Tom shares after deposits: 1003806

  • it's clear that tom shares are 3806 more than alice (even both deposits same amount of collateral (1e12) ).

Tools Used

Manual Review

Recommendations

Ensure that deposits—regardless of whether the position is open or closed—are valued consistently. This might involve recalculating the vault’s total collateral in a way that smooths out transient price impacts before minting shares.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Other

Appeal created

mohitisimmortal Submitter
7 months ago
0xl33 Auditor
7 months ago
mohitisimmortal Submitter
7 months ago
n0kto Lead Judge
6 months ago
n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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