First deposit after rebalancing might receive shares worth of less value

Summary

If the totalDepositAssets of a pool becomes zero because of rebalancing bad debt then the first depositor into the pool will receive shares worth significantly less value than the deposited.

The Pool.rebalanceBadDebt function distributes the bad debt across all the lenders and deducts the borrowed assets from the deposited assets

    function rebalanceBadDebt(uint256 poolId, address position) external {
        PoolData storage pool = poolDataFor[poolId];
        accrue(pool, poolId);

        // revert if the caller is not the position manager
        if (msg.sender != positionManager) revert Pool_OnlyPositionManager(poolId, msg.sender);

        // compute pool and position debt in shares and assets
        uint256 totalBorrowShares = pool.totalBorrowShares;
        uint256 totalBorrowAssets = pool.totalBorrowAssets;
        uint256 borrowShares = borrowSharesOf[poolId][position];
        // [ROUND] round up against lenders
        uint256 borrowAssets = _convertToAssets(borrowShares, totalBorrowAssets, totalBorrowShares, Math.Rounding.Up);

        // rebalance bad debt across lenders
        pool.totalBorrowShares = totalBorrowShares - borrowShares;
        // handle borrowAssets being rounded up to be greater than totalBorrowAssets
        pool.totalBorrowAssets = (totalBorrowAssets > borrowAssets) ? totalBorrowAssets - borrowAssets : 0;
        uint256 totalDepositAssets = pool.totalDepositAssets;
@> pool.totalDepositAssets = (totalDepositAssets > borrowAssets) ? totalDepositAssets - borrowAssets : 0;
        borrowSharesOf[poolId][position] = 0;
    }

The totalDepositAssets can become 0 if the rebalanced borrow is the only borrow in the pool and is of full amount.

For such pools, after execution of the rebalanceBadDebt function the totalDepositAssets will be zero and totalDepositShares will be non-zero.

If a user attempts to deposit into the pool when depositAssets are zero and depositShares are non-zero, the user will receive shares worth of less value than deposited

The Pool._convertToShares function is used to calculate the shares for the deposits

    function _convertToShares(
        uint256 assets,
        uint256 totalAssets,
        uint256 totalShares,
        Math.Rounding rounding
    ) internal pure returns (uint256 shares) {
        if (totalAssets == 0) return assets;
        shares = assets.mulDiv(totalShares, totalAssets, rounding);
    }

The _convertToShares function assumes that the totalShares is also zero if totalAssets is zero and returns 1:1 shares. However, in the above mentioned case it is possible that totalAssets is zero and totalShares is non-zero. After the deposit, the totalAssets become depositedAssets and totalShares become depositedAssets + previousTotalShares. The share exchange rate is not 1:1 and the user shares are worth less than the deposit amount.

As a result, when the first user deposits into the pool after rebalancing the shares minted will not have the same value.

Root Cause

The _convertToShares function assumes totalShares is zero if totalAssets is zero.

Internal pre-conditions

A position borrows full amount of the pool and becomes insolvent. The position debt is rebalanced using Pool.rebalanceBadDebt

External pre-conditions

No response

Attack Path

Pool A has deposits of 50k USDC. totalDepositAssets = 50k and totalDepositShares = 40,000
User Alice borrows 50k from Pool A
The price of alice's collateral declines and the Alice's position becomes insolvent.
The debt from Alice's position is rebalanced using rebalanceBadDebt by the protocol owner
The totalDepositAssets = 0 and totalDepositShares = 40,000.
User Bob deposits 30k USDC into the pool. Shares minted = 30,000
- totalDepositAssets = 30k, totalDepositShares = 70,000, Bob shares = 30, 000
Bob withdraws his shares, receive amout = 30,000/70,000 * 30k = 12857. Loss = 17200

Impact

A depositor receives shares worth significantly less than the deposited amount.

PoC

No response

Mitigation

Use the implementation of the _convertToShares and _convertToAssets in the SuperPool contract for the Pool contract.

Duplicate of #319

sherlock-audit / 2024-08-sentiment-v2-judging

S3v3ru5 - First deposit after rebalancing might receive shares worth of less value #598