tvdung94 - Malicious pool owner can redirect funds of super pool, from other base pools, into their pool

[sherlock-admin3]

tvdung94

High

Malicious pool owner can redirect funds of super pool, from other base pools, into their pool

Summary

Depositing and withdrawing mechanics will cause low liquidity for some pools as malicious pool owners can redirect liquidity, provided from super pool, into their own pools.

Root Cause

• The choice to supply to and withdraw funds from the queues (deposit queue and withdrawal queue) sequentially might not be a good choice since it allows users with huge funds (whales or flashloans) to manipulate liquidity of base pools. When huge withdrawals happen, the frontmost pools in the withdrawal queu will have the highest risk of being empty/ low liquidity.
• Combining with the fact that users could deposit then withdraw right after makes this attack even more viable, for malicious attackers could use flashloans to manipulate the liquidity of base pools for free.

Internal pre-conditions

1. There is at least a pool with asset of superpool in it.
2. Use default queues (default deposit queue + default withdraw queue)

External pre-conditions

N/A

Attack Path

1. There is at least a pool (pool A) in the deposit pool with some funds of super pool depositing into it.
2. A new pool (pool B) is added. Default deposit queue is now [A B]. Default withdrawal queue is also [A B]
3. Pool B owner uses a flashloan to deposit a huge amount of asset into and then withdraw it from super pool.
4. Because withdrawal queue is [A B], all liquidity of super pool from A will be removed first before B. Thus, the remaining asset of super pool will end up in pool B. Pool B owner has successfully transfer liquidity from pool A to his pool.

Impact

Malicious base pool owners can redirect liquidity into their own pools, at the cost of low liquidity for other base pools. High liquidity will help these pools attract more borrowers, thus more profit in charging interest.

Code snippet

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/0b472f4bffdb2c7432a5d21f1636139cc01561a5/protocol-v2/src/SuperPool.sol#L258-L263

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/0b472f4bffdb2c7432a5d21f1636139cc01561a5/protocol-v2/src/SuperPool.sol#L281-L286

PoC

   function setUp() public override {
        super.setUp();

        pool = protocol.pool();
        registry = protocol.registry();
        riskEngine = protocol.riskEngine();
        superPoolFactory = protocol.superPoolFactory();

        FixedPriceOracle asset1Oracle = new FixedPriceOracle(1e18);
        vm.prank(protocolOwner);
        riskEngine.setOracle(address(asset1), address(asset1Oracle));

        vm.prank(protocolOwner);
        asset1.mint(address(this), initialDepositAmt);
        asset1.approve(address(superPoolFactory), initialDepositAmt);

        superPool = SuperPool(
            superPoolFactory.deploySuperPool(
                poolOwner, address(asset1), feeTo, 0.01 ether, 1_000_000 ether, initialDepositAmt, "test", "test"
            )
        );

        poolOwner1 = makeAddr("poolOwner1");
        poolOwner2 = makeAddr("poolOwner2");
        bytes32 FIXED_RATE_MODEL_KEY = 0xeba2c14de8b8ca05a15d7673453a0a3b315f122f56770b8bb643dc4bfbcf326b;
        vm.prank(poolOwner1);
        testPoolA = protocol.pool().initializePool(poolOwner1, address(asset1), type(uint128).max, FIXED_RATE_MODEL_KEY);
        vm.stopPrank();

        vm.prank(poolOwner2);
        testPoolB = protocol.pool().initializePool(poolOwner2, address(asset1), type(uint128).max, FIXED_RATE_MODEL_KEY);
        vm.stopPrank();
    }

    function testManipulateSuperPoolLiquidity() public {
        vm.startPrank(poolOwner);
        superPool.addPool(testPoolA, 50 ether);
        superPool.addPool(testPoolB, 50 ether);
        vm.stopPrank();

        vm.startPrank(user);
        asset1.mint(user, 25 ether);
        asset1.approve(address(superPool), 25 ether);
        superPool.deposit(25 ether, user);
        vm.stopPrank();

        vm.startPrank(poolOwner2);
        asset1.mint(poolOwner2, 100 ether);
        asset1.approve(address(superPool), 100 ether);
        superPool.deposit(100 ether, poolOwner2);
        superPool.withdraw(100 ether, poolOwner2, poolOwner2);
        uint256 poolAFinalAmount = superPool.POOL().getAssetsOf(testPoolA, address(superPool));
        uint256 poolBFinalAmount = superPool.POOL().getAssetsOf(testPoolB, address(superPool)); 
        assertEq(poolAFinalAmount, 0 ether);
        assertEq(asset1.balanceOf(poolOwner2), 100 ether);
        assertEq(poolBFinalAmount, 25 ether  + initialDepositAmt);
        vm.stopPrank();
    }

Mitigation

To mitigate this issue, we need to prevent users from withdrawing immediately after depositing.

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

z3s commented:

Admins are trusted

[sota1994]

Escalate Base pool owners are not admins. While they are trusted by the super pool, they don't necessarily trust each other. In my issue, one pool owner can increase liquidity of their pool at the cost of other pool owners.

[sherlock-admin3]

Escalate Base pool owners are not admins. While they are trusted by the super pool, they don't necessarily trust each other. In my issue, one pool owner can increase liquidity of their pool at the cost of other pool owners.

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.

[cvetanovv]

I don't understand what the impact is. What is the problem that pool owners can increase liquidity in their own pools?

[cvetanovv]

Because of the lack of impact and clarity on what the issue is, I plan to reject the escalation and leave the issue as is.

[sota1994]

Hello, Sorry for late reply.

Pool owners take fees on borrowing. More liquidity will attract more borrowing, especially when your pools have low liquidity due to vibrant borrowing activities.

Consider this scenario:

• Pool A has low liquidity because there are lots of borrowing activities lately
• Pool A owner can use the method described in this issue to increase liquidity for free, so he has higher chance to earn fees from borrowing
• This might make other pools, which are before pool A in the withdrawal queue, low liquidity, depending on how big the funds is from the super pool .For example, if the funds / position from super pool is 100% liquidity of the base pool B, withdrawing it can halt all borrowing activities on pool B. This will negatively affect pool B owner, because no borrowing means no fee for them.

[cvetanovv]

I still don't understand what the problem is for the owner to increase his liquidity. What is the vulnerability?

Each user decides where to invest. With a flash loan, you can't increase your liquidity because the loan has to be repaid with the same transaction. There is no way he can trick consumers into investing in his pool so he can charge them fees. But even if he did it is a normal part of the design and design decision.

Planning to reject the escalation and leave the issue as is.

[Evert0x]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• sota1994: rejected