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

`_mint()` function mints shares at wrong rate for SHORT positions and LONG GMX Positions

ccvascocc

Summary

Function _mint() will be called on a deposit flow to mint shares to users according to their deposits.
If the vault has a SHORT position (or LONG Position with leverage higher than 1x) ongoing, totalAmountBefore will be computed by subtracting variable amount to _totalAmount(prices).

function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
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 {// --> here: if position is closed // SHORT || LONG (2x or 3x) <--
@> totalAmountBefore = _totalAmount(prices) - amount; <@
}
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);
}

However, _totalAmount(prices) is handling indexToken and collateralToken balances as if they were realized. Since this is a GMX (SHORT or LONG) position, the vault is holding neither indexToken nor collateralToken at this point.

function _totalAmount(MarketPrices memory prices) internal view returns (uint256) {
if (positionIsClosed) {
return collateralToken.balanceOf(address(this));
} else { // --> here: if position is open <--
IVaultReader.PositionData memory positionData = vaultReader.getPositionInfo(curPositionKey, prices);
@> uint256 total = IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min / prices.shortTokenPrice.min //@audit
@> + collateralToken.balanceOf(address(this))//@audit
+ positionData.netValue / prices.shortTokenPrice.min;
​
return total;
}
}

Vulnerability Details

If the vault has a SHORT position ongoing (or LONG with leverage > 1x), then mint() will not be called via function deposit() but via afterOrderExecution() instead, which is a callback function. This means that an order has already been executed on GMX, to which the deposited collateral balance was sent.

Therefore, when _totalAmount(prices) is called within mint(), it will return variable total which is the sum of:

  1. IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min / prices.shortTokenPrice.min

  2. collateralToken.balanceOf(address(this))

  3. positionData.netValue / prices.shortTokenPrice.min;

Since the vault is holding neither indexToken nor collateralToken at this point, this will be:

  1. 0 * prices.indexTokenPrice.min / prices.shortTokenPrice.min

  2. 0

  3. positionData.netValue / prices.shortTokenPrice.min;

This being, _totalAmount(prices) returns positionData.netValue / prices.shortTokenPrice.min.

Impact

Since _shares is computed in function _mint() as:
_shares = amount * totalShares / totalAmountBefore

And totalAmountBefore = _totalAmount(prices) - amount
which is the same as totalAmountBefore = (positionData.netValue / prices.shortTokenPrice.min) - amount

Then totalAmountBefore will not depend on the collateral deposited.

function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
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;
}
if (totalAmountBefore == 0) totalAmountBefore = 1;
@> _shares = amount * totalShares / totalAmountBefore; <@ //here
}
​
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);
}

The impact is in totalAmountBefore, which is the denominator in _shares = amount * totalShares / totalAmountBefore.
So:

  1. shares minted will depend solely on position's net value in USDC, regardless of the position's collateral

  2. the higher is totalShares, the more shares will the depositor get for the same prices.

These tests show how for equal deposit amounts, at the same price (for simplicity), the _mint() function will mint a disproportionate amount of shares. In these specific examples, late depositors get more shares for the same deposit (at equal prices). This is totally contrary to the expected as new deposits increase the position value and late deposits should represent a smaller share of the total position.
This is happening because totalShares (numerator) is growing at a higher rate than totalAmountBefore (denominator).


SHORT GMX Position

Paste this on PerpetualVault.t.sol:

function test_DepositMintsSharesWrongForShortPosition() external {
address alice = makeAddr("alice");
payable(alice).transfer(1 ether);
address bob = makeAddr("bob");
payable(bob).transfer(1 ether);
// Alice deposits while position is still closed
depositFixture(alice, 1e10);
​
// Off-chain script calls run to open SHORT GMX position
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);
​
// Alice makes another deposit to the open SHORT position
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 amount = 1e10;
deal(address(collateralToken), alice, amount);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
vm.startPrank(alice);
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);
​
// indexToken and collateralToken balances after deposit
address indexToken = PerpetualVault(vault).indexToken();
uint256 indexBal = IERC20(indexToken).balanceOf(vault);
uint256 collBal = IERC20(collateralToken).balanceOf(vault);
console.log("indexBal: ", indexBal);// -> 0
console.log("collBal: ", collBal);// -> 1e10
​
// call runNextAction() to process increase in the position and minting of shares
data[0] = abi.encode(3380000000000000);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, data);
​
// indexToken and collateralToken balances after calling runNextAction()
uint256 collBal2 = IERC20(collateralToken).balanceOf(vault);
uint256 indexBal2 = IERC20(indexToken).balanceOf(vault);
console.log("indexBal2: ", indexBal2);// -> 0
console.log("collBal2: ", collBal2);// -> 0
​
GmxOrderExecuted(true);// position increased
​
// indexToken and collateralToken balances after executing GMX order
uint256 collBal3 = IERC20(collateralToken).balanceOf(vault);
uint256 indexBal3 = IERC20(indexToken).balanceOf(vault);
console.log("indexBal3: ", indexBal3);// -> 0
console.log("collBal3: ", collBal3);// -> 0
​
// Check for shares minted for each deposit
uint256[] memory depositIdsAlice = PerpetualVault(vault).getUserDeposits(alice);
assertEq(depositIdsAlice.length, 2);
(,uint256 sharesAlice1,,,,) = PerpetualVault(vault).depositInfo(depositIdsAlice[0]);
(,uint256 sharesAlice2,,,,) = PerpetualVault(vault).depositInfo(depositIdsAlice[1]);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, metadata);
​
​
// Bob's turn to deposit same amount as each of Alice's deposit
// In theory he should receive less shares than Alice for the same deposit amount
deal(address(collateralToken), bob, amount);
vm.startPrank(bob);
collateralToken.approve(vault, amount);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(amount);
vm.stopPrank();
​
uint8 flow2 = uint8(PerpetualVault(vault).flow());
assertEq(flow2, 1);
(PerpetualVault.NextActionSelector selector2, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector2), 1);
​
data[0] = abi.encode(3380000000000000);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, data);
​
GmxOrderExecuted(true);
​
uint256[] memory depositIdsBob = PerpetualVault(vault).getUserDeposits(bob);
assertEq(depositIdsBob.length, 1);
(,uint256 sharesBob,,,,) = PerpetualVault(vault).depositInfo(depositIdsBob[0]);
console.log("sharesAlice1: ", sharesAlice1); //->1000000000000000000
console.log("sharesAlice2: ", sharesAlice2); //->1003180350956768875
console.log("sharesBob: ", sharesBob); //->1003980619786060295
assert(sharesAlice2 > sharesAlice1);
assert(sharesBob > sharesAlice2);
}


The same applies to LONG GMX positions with leverage greater than 1x. The below example is for a 2x leverage LONG.

LONG 2x Position

Paste it on PerpetualVault.t.sol:

function test_DepositMintsSharesWrongForLong2xPosition() external {
address keeper = PerpetualVault(vault2x).keeper();
address alice = makeAddr("alice");
address bob = makeAddr("bob");
// Alice & Bob deposit into the vault while position hasn't been opened
depositFixtureInto2x(alice, 1e10);
depositFixtureInto2x(bob, 1e10);
// Open LONG 2x position
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3390000000000000);
vm.prank(keeper);
PerpetualVault(vault2x).run(true, true, prices, data);
​
GmxOrderExecuted2x(true);
bytes32 curPositionKey = PerpetualVault(vault2x).curPositionKey();
assertTrue(curPositionKey != bytes32(0));
assertEq(PerpetualVault(vault2x).beenLong(), true);
​
PerpetualVault.FLOW flow = PerpetualVault(vault2x).flow();
assertEq(uint8(flow), 2);
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault2x).nextAction();
assertEq(uint8(selector), 6);
​
MarketPrices memory prices2 = mockData.getMarketPrices();
bytes[] memory swapData = new bytes[](2);
bytes memory paraSwapData = mockData.getParaSwapData(vault2x);
swapData[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
address[] memory gmxPath = new address[](1);
gmxPath[0] = address(0x70d95587d40A2caf56bd97485aB3Eec10Bee6336);
uint256 minOutputAmount = 1e10 * prices2.shortTokenPrice.min / prices2.longTokenPrice.min * 95 / 100;
swapData[1] = abi.encode(PROTOCOL.GMX, abi.encode(gmxPath, 1e10, minOutputAmount));
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices2, swapData);
​
// Alice deposits into the existing LONG 2x position
IERC20 collateralToken = PerpetualVault(vault2x).collateralToken();
uint256 amount = 1e10;
deal(address(collateralToken), alice, amount);
deal(alice, 1e18);
uint256 executionFee = PerpetualVault(vault2x).getExecutionGasLimit(true);
vm.startPrank(alice);
collateralToken.approve(vault2x, amount);
PerpetualVault(vault2x).deposit{value: executionFee * tx.gasprice}(amount);
vm.stopPrank();
​
// indexToken and collateralToken balances after deposit
address indexToken = PerpetualVault(vault2x).indexToken();
uint256 indexBal = IERC20(indexToken).balanceOf(vault2x);
uint256 collBal = IERC20(collateralToken).balanceOf(vault2x);
console.log("indexBal: ", indexBal);// -> 0
console.log("collBal: ", collBal);// -> 1e10
​
// call runNextAction() to process increase in the position and minting of shares
data[0] = abi.encode(3390000000000000);
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices2, data);
​
// indexToken and collateralToken balances after calling runNextAction()
uint256 collBal2 = IERC20(collateralToken).balanceOf(vault2x);
uint256 indexBal2 = IERC20(indexToken).balanceOf(vault2x);
console.log("indexBal2: ", indexBal2);// -> 0
console.log("collBal2: ", collBal2);// -> 0
​
​
GmxOrderExecuted2x(true);// position increased
​
PerpetualVault.FLOW flow2 = PerpetualVault(vault2x).flow();
assertEq(uint8(flow), 2);
(PerpetualVault.NextActionSelector selector2, ) = PerpetualVault(vault2x).nextAction();
assertEq(uint8(selector2), 6);
​
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices2, data);
​
// indexToken and collateralToken balances after executing GMX order
uint256 collBal3 = IERC20(collateralToken).balanceOf(vault2x);
uint256 indexBal3 = IERC20(indexToken).balanceOf(vault2x);
console.log("indexBal3: ", indexBal3);// -> 0
console.log("collBal3: ", collBal3);// -> 0
​
// Bob makes another deposit into the existing LONG 2x position
deal(address(collateralToken), bob, amount);
deal(bob, 1e18);
executionFee = PerpetualVault(vault2x).getExecutionGasLimit(true);
vm.startPrank(bob);
collateralToken.approve(vault2x, amount);
PerpetualVault(vault2x).deposit{value: executionFee * tx.gasprice}(amount);
vm.stopPrank();
​
data[0] = abi.encode(3390000000000000);
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices2, data);
​
GmxOrderExecuted2x(true);// position increased
​
////////////////////
// Check shares minted for each deposit
uint256[] memory depositIdsAlice = PerpetualVault(vault2x).getUserDeposits(alice);
uint256[] memory depositIdsBob = PerpetualVault(vault2x).getUserDeposits(bob);
(,uint256 sharesAlice1,,,,) = PerpetualVault(vault2x).depositInfo(depositIdsAlice[0]);
(,uint256 sharesAlice2,,,,) = PerpetualVault(vault2x).depositInfo(depositIdsAlice[1]);
(,uint256 sharesBob1,,,,) = PerpetualVault(vault2x).depositInfo(depositIdsBob[0]);
(,uint256 sharesBob2,,,,) = PerpetualVault(vault2x).depositInfo(depositIdsBob[1]);
console.log("1st Deposit (Alice) -> sharesAlice: ", sharesAlice1); //->1000000000000000000
console.log("2nd Deposit (Bob) -> sharesBob: ", sharesBob1); //->1000000000000000000
console.log("3rd Deposit (Alice) -> sharesAlice2: ", sharesAlice2); //->1003367448192106286
console.log("4th Deposit (Bob) -> sharesBob2: ", sharesBob2); //->1004109975039629309
assert(sharesBob2 > sharesAlice2);
assert(sharesAlice2 > sharesBob1);
assert(sharesBob1 == sharesAlice1);
}

Tools Used

Manual Review
Foundry testing

Recommendations

Ensure that for the case of SHORT and LONG (>1x leverage) GMX positions, function _totalAmount(prices)
accounts for PositionData values rather than current balances in vault.

Updates

Lead Judging Commences

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

Appeal created

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

Support

FAQs

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