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

Over-Minting of Shares When the Vault Is 1x Long

cipherhawk

Root Cause & issue Loc.

In the _mint() function, there is special-case logic for when the vault is in a 1x long position (_isLongOneLeverage(beenLong) == true). Specifically:

if (positionIsClosed == false && _isLongOneLeverage(beenLong)) {
totalAmountBefore = IERC20(indexToken).balanceOf(address(this)) - amount;
} else {
totalAmountBefore = _totalAmount(prices) - amount;
}
_shares = amount * totalShares / totalAmountBefore;
​

When a user deposits collateral, that collateral eventually gets swapped into indexToken (since the vault is 1x long). >BUT, the code subtracts the deposit’s final amount from the old indexToken balance instead of adding it to the vault’s total. This incorrectly lowers the denominator in the share-minting formula and causes the depositor to receive more shares than they really deserve.

Attack Path

Assume the vault is already open in a 1x long position.

There is some existing balance of indexToken in the vault (e.g. 100 tokens) and some nonzero totalShares (e.g. 100 shares).
2. Deposit:

An attacker deposits X units of the collateral token, which are swapped 1:1 into X indexToken.

  • Now the actual total indexToken holdings in the vault should be 100 + X.

  1. Faulty Calculation:

    • Instead of recognizing that the vault’s “old” total was 100 and the “new” total is 100 + X, the code does:

      totalAmountBefore = IERC20(indexToken).balanceOf(address(this)) - amount;
      // which becomes: totalAmountBefore = 100 - X = 100 - X,
      // rather than something reflecting 100 as the pre-deposit total.
      ​

    • Then it mints shares as:

      _shares = amount * totalShares / totalAmountBefore;
      ​

    • Because totalAmountBefore is artificially reduced by X, the fraction (_shares / totalShares) ends up being larger than the correct fraction.

Proof by Example

  • Initial State:

    • Vault has 100 indexToken.

    • totalShares = 100.

    • So, each share should be worth 1 indexToken at this moment.

  • User Deposits 10 Collateral, which the vault swaps into 10 indexToken:

    • Real new total: 110 indexToken.

    • Fair share: The user should get shares equal to (10 / 110) * (old totalShares + ???), which is about 9.09 shares out of ~109.09 total. They end up with roughly 9.09% of the vault if done correctly.

  • Faulty Code Path:

    • It sets totalAmountBefore = 100 - 10 = 90 (subtracting the deposit from the old indexToken balance).

    • The minted shares = 10 * 100 / 90 = 11.111....

    • After minting, totalShares = 100 + 11.111... = 111.111..., so the user’s fractional ownership is 11.111 / 111.111 = 10%.

    • In terms of the vault’s actual 110 tokens, the user effectively controls 110 * 0.1 = 11 tokens’ worth—a free ~1 token.

Because the user is systematically over-minted shares, they effectively steal value from other participants. Moreover, an attacker can repeat this deposit process multiple times to compound the exploit.

PoC:

Below is a minimal Foundry proof-of-concept (PoC) demonstrating the “Over-Minting of Shares in 1x Long Scenario” issue in PerpetualVault. The key idea is:

  1. Deploy a simplified PerpetualVault with leverage = 1x (leverage = 10_000).

  2. Mock the vault so it believes it is in a “1x long, position open” state (beenLong = true; positionIsClosed = false).

  3. Have a user deposit a certain amount of collateral.

  4. Show that the minted shares exceed the correct, “expected” shares, proving an inflation in user share issuance.

Note: This is a minimal test. It does not compile against the entire protocol code, but focuses on the _mint() over-mint logic.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.17;
​
import "forge-std/Test.sol";
import "../src/PerpetualVault.sol"; // or wherever your PerpetualVault is located
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
/**
* @dev Minimal mock collateral token with 18 decimals (for simplicity).
*/
contract MockCollateral is ERC20 {
constructor() ERC20("Mock Collateral", "MCK") {
_mint(msg.sender, 1_000_000 ether); // Give test contract a big supply
}
}
​
/**
* @title OverMintingPoC
* @dev A Foundry test contract showing how a user can receive more shares than fair if vault is 1x long
*/
contract OverMintingPoC is Test {
PerpetualVault vault;
MockCollateral collateral;
​
address alice = address(0xAA); // test user
​
function setUp() public {
// Deploy mock collateral
collateral = new MockCollateral();
​
// Deploy vault (assuming it's upgradeable but we can just do normal construction for PoC)
vault = new PerpetualVault();
// Initialize the vault with some minimal arguments (in real code you'd pass everything needed)
// We'll cheat a bit and set storage vars directly for PoC:
// e.g. vault.initialize(...)
​
// For demonstration, we “manually” set essential state:
vm.label(address(vault), "PerpetualVault");
vm.label(address(collateral), "CollateralToken");
​
// Set vault's relevant public fields as if we are at 1x long
// In actual code, you'd do something like setVaultState(...) if the function is exposed
// Here we do a cheat: we handle it via vm.store or a specialized helper if your environment allows
// For demonstration, we assume we can set these storage variables directly:
vault.setVaultState(
PerpetualVault.FLOW.NONE,
0,
true, // beenLong = true
bytes32(0), // curPositionKey
false, // positionIsClosed = false => means there's an open position
false, // _gmxLock
PerpetualVault.Action({selector: PerpetualVault.NextActionSelector.NO_ACTION, data: hex""})
);
​
// We want a 1x leverage
// In the real code, you'd pass leverage = 10_000 in initialize or setLeverage
// We'll forcibly store it for demonstration:
// vault.leverage() = 10_000
​
// Let the vault use our mock collateral
// In real usage, you'd call some method or constructor param
// We'll do the simplest approach: vault.collateralToken = address(collateral);
​
// Give Alice some collateral
collateral.transfer(alice, 1_000 ether);
}
​
/**
* @notice Show that user gets more shares than the correct calculation
*/
function testOverMintingAtOneXLong() public {
// 1. Alice approves vault to pull her collateral
vm.startPrank(alice);
collateral.approve(address(vault), 1_000 ether);
​
// 2. Check the vault's totalShares is 0 initially (fresh vault)
assertEq(vault.totalShares(), 0, "Should start with zero shares");
​
// 3. Suppose Alice deposits 100 tokens
// In real code, you'd call vault.deposit(100)
// We'll replicate just enough of the deposit flow to trigger `_mint()`
// For demonstration, we can do a direct call if deposit() is public. Otherwise we replicate the logic.
​
// This call should cause the "1x long" path inside `_mint()`,
// subtracting `amount` from the vault's old indexToken balance incorrectly.
vault.deposit(100 ether);
​
// 4. Now check how many shares were minted
// The user would store depositInfo(...) but let's just read if the vault exposes anything.
// We'll do a simplified approach: compare vault.totalShares() to an expected correct total
​
uint256 mintedShares = vault.totalShares();
console.log("User minted shares:", mintedShares);
​
// 5. Compare to the "correct" share amount. If the vault had 0 prior shares and 0 prior assets,
// the correct share is simply amount * 1e8, or whatever the vault uses for first deposit.
// In PerpetualVault, if totalShares == 0, we do: _shares = depositInfo[depositId].amount * 1e8
// So we'd expect 100 * 1e8 = 10,000,000,000 if there's no special “1x long” path messing things up.
​
// Let's say the correct shares = 100 ether * 1e8 = 100 * 1e8 = 1e10
uint256 expectedShares = 100 ether * 1e8 / 1 ether;
// = 10,000,000,000
​
console.log("Expected shares for first deposit:", expectedShares);
​
// 6. If there's an over-mint bug, mintedShares > expectedShares
// For demonstration, we'll fail the test if mintedShares > expectedShares
require(
mintedShares <= expectedShares,
"Vault minted more shares than expected => Over-Minting vulnerability!"
);
​
vm.stopPrank();
}
}
​

Severity by likelyhood & impact

  • This is a high-severity issue because it allows one depositor to acquire a disproportionately large share of the vault with no special permissions. Over multiple deposits, the attacker can dilute and siphon value from honest depositors.

Recommended Remediation

  • In _mint(), remove the special subtraction of amount from the vault’s indexToken balance whenever the vault is in a 1x long position. Instead, the contract should compute the vault’s total value before adding the new deposit, then use that to calculate the correct fraction of shares to mint.

  • A safer approach is to always use a reliable “vault total value” call (e.g., _totalAmount(prices)) before the deposit is added. Then, once the deposit is fully swapped into indexToken, you mint shares relative to the vault’s pre-deposit state.

    In other words:

    // Pseudocode
    uint256 vaultValueBefore = _totalAmount(prices); // old value
    // ... user deposit is swapped ...
    // mintedShares = depositValue / vaultValueBefore * totalShares
    ​
Updates

Lead Judging Commences

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