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

`_totalAmount` might be under-estimated as it uses `indexTokenPrice.min` rather than `indexTokenPrice.max` to estimate vault total position value, which might mint too much shares to the user

infect3d

Summary

Because Gamma create and manage GMX positions, it also uses GMX price functions to estimate assets values on GMX. GMX returns a min and max value when requesting a price.

The issue is that Gamma uses prices.indexTokenPrice.min to estimate the total position amount of the vault, rather than using prices.indexTokenPrice.max, which will underestimate the total value of the vault.

And when user gets minted new shares, the amount of minted shares is proportional to what they deposited compared to the total vault position, meaning users can get minted too much shares compared to what they should have received.

This difference is at all the other users expense, which is the opposite of what should happen in a vault.

Vulnerability details

When users deposits assets into a leveraged vault, the minted amount of shares is defined L774 as _shares = amount * totalShares / totalAmountBefore.

The user gets minted an amount proportional to the value he has deposited (amount) compared to the already deposited amount (totalAmountBefore).

totalAmountBefore is defined L771, which is equal to totalAmountBefore = _totalAmount(prices) - amount.

​
File: contracts/PerpetualVault.sol
762: function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
763: uint256 _shares;
764: if (totalShares == 0) {
765: _shares = depositInfo[depositId].amount * 1e8;
766: } else {
767: uint256 totalAmountBefore;
768: if (positionIsClosed == false && _isLongOneLeverage(beenLong)) {
769: totalAmountBefore = IERC20(indexToken).balanceOf(address(this)) - amount;
770: } else {
771: totalAmountBefore = _totalAmount(prices) - amount;
772: }
773: if (totalAmountBefore == 0) totalAmountBefore = 1;
774: _shares = amount * totalShares / totalAmountBefore;
775: }
776:
777: depositInfo[depositId].shares = _shares;
778: totalShares = totalShares + _shares;
...:
...: //* -------- some code --------- *//
...:
788: }

But as we can see L826, _totalAmount is estimated using the GMX price bounds (min/max), and in the current design, its prices.indexTokenPrice.min that is used to compute total, which will result in a lower total than if prices.indexTokenPrice.max was used:

File: contracts/PerpetualVault.sol
821: function _totalAmount(MarketPrices memory prices) internal view returns (uint256) {
822: if (positionIsClosed) {
823: return collateralToken.balanceOf(address(this));
824: } else {
825: IVaultReader.PositionData memory positionData = vaultReader.getPositionInfo(curPositionKey, prices);
826: uint256 total = IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min / prices.shortTokenPrice.min
827: + collateralToken.balanceOf(address(this))
828: + positionData.netValue / prices.shortTokenPrice.min;
829:
830: return total;
831: }
832: }

Going back to the _mint function, this under-estimated value of _totalAmount clearly shows that the users will have an amount of _shares minted that might be over-estimated compared to the real price, while it should be under-estimated, otherwise the vault (and all the other users) would be at loss compared to the current depositor.

This is similar to what vault does when they round down what goes out of the vault, and round up what goes in.

Impact

Depositor gets minted too much shares at the expense of the vault and its users.

  • Likelihood: high, as this happens on every deposits

  • Impact: medium, as this impact all users funds, but it is indirect

Recommended Mitigation Steps

function _totalAmount(MarketPrices memory prices) internal view returns (uint256) {
if (positionIsClosed) {
return collateralToken.balanceOf(address(this));
} else {
IVaultReader.PositionData memory positionData = vaultReader.getPositionInfo(curPositionKey, prices);
- uint256 total = IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min / prices.shortTokenPrice.min
+ uint256 total = IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.max / prices.shortTokenPrice.min
+ collateralToken.balanceOf(address(this))
+ positionData.netValue / prices.shortTokenPrice.min;
​
return total;
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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