Portfolio Token Price Can be Manipulated When Minting Performance Fee

[hats-bug-reporter[bot]]

Github username: @MatinR1 Twitter username: MatinRezaii1 Submission hash (on-chain): 0xc0b578da9d84fc209e4ef97d3a426d2f8f6666833096b71402cd8210e52d942d Severity: medium

Description: Description\

The vulnerability allows malicious actors to manipulate the portfolio token price during the minting of performance fees. By exploiting the lack of slippage protection and price bands in the chargePerformanceFee() function, attackers can artificially inflate or deflate the token price. This can lead to inaccurate performance fee calculations, potentially resulting in significant financial losses for the protocol and its users.

The function chargePerformanceFee() fetches the token price using a direct ratio of the vault balance to the total supply:

  function chargePerformanceFee() external onlyAssetManager nonReentrant {
    uint256 totalSupply = portfolio.totalSupply();

    uint256 vaultBalance = portfolio.getVaultValueInUSD(
      IPriceOracle(protocolConfig.oracle()),
      portfolio.getTokens(),
      totalSupply,
      portfolio.vault()
    );
    uint256 currentPrice = _getCurrentPrice(vaultBalance, totalSupply);
  ...
}

And also,

  function _getCurrentPrice(
    uint256 _vaultBalance,
    uint256 _totalSupply
  ) internal pure returns (uint256 currentPrice) {
    currentPrice = _totalSupply == 0
      ? 0
      : (_vaultBalance * ONE_ETH_IN_WEI) / _totalSupply;
  }

However, without slippage protection and price bands, this calculation is susceptible to manipulation. An attacker can alter the token price by executing trades that temporarily change the vault balance or total supply right before the performance fee is charged. This manipulation can result in incorrect fee calculations, leading to either overcharging or undercharging of fees, and causing financial discrepancies within the protocol. This is more probable when the first user alters this ratio with depositing 1 wei of portfolio token.

Attack Scenario\ A possible proof of concept would be in such a way:

1. The portfolio and fee module contracts are deployed.
2. An attacker monitors the mempool and realizes there is a call to chargePerformanceFee() and there is not any deposits
3. He deposits 1 wei worth of portfolio token
4. He frontruns his transaction
5. The intended ratio now skyrockets inside the _getCurrentPrice()
6. The performance fee will be wrongly calculated and minted

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional) Implementing slippage protection and setting price tolerances would help mitigate this risk.

[langnavina97]

The manipulation would require manipulating the Chainlink price feed. How would you achieve that? @MatinR1

[MatinR1]

The manipulation would require manipulating the Chainlink price feed. How would you achieve that? @MatinR1

My intent with the price manipulation was not the Chainlink price, but the price defined as a pure function here.

I also made a simplified version of tests in Foundry, to show my purpose with this price manipulation: https://gist.github.com/MatinR1/0ea3cb58be744d52be086677e161ed57

I made a simple ecosystem consisting FeeModule and Portfolio with two sample tokens. I compared two situations, first a normal condition and supposing the portfolio has some deposits (~20 ether), which price is something reasonable. The second one is the price manipulation due to 1 wei deposit to the portfolio which the price skyrockets (~1e38). In both cases, I assumed the vault has 15 ether balance in both sample tokens.

This output can be generated by this POC:

Ran 2 tests for test/FeeModuleTest.t.sol:FeeModuleTest
[PASS] testNormalPerformanceFeeCharge() (gas: 446769)
Logs:
  Initial Price:  0
  Current Price:  6768450000000000000

[PASS] testOneWeiPerformanceFeeChargeAttack() (gas: 437516)
Logs:
  Price After 1 wei Attack:  135369000000000000000000000000000000000

[langnavina97]

A user can't deposit an amount which results in a mint amount less than the minPortfolioTokenHoldingAmount. Additionally, a deposit of one wei divided by any balance would result in a ratio of 0, which would cause the transaction to fail. @MatinR1

[MatinR1]

@langnavina97 I aimed to demonstrate the potential for such manipulation. Moreover, even with minimum deposits, price inflation with front-running remains a significant issue.