Skip to content

[CRITICAL] DepositPool.migrate() sends ALL deposits to a single reward pool, permanently locking funds from other public pools #63

Description

@Apollyon13X

Summary

DepositPool.migrate() sends the entire totalDepositedInPublicPools balance to a single rewardPoolIndex_, ignoring that users may have deposits in multiple distinct public reward pools. Users whose deposits were tracked under a different reward pool index cannot withdraw their funds after migration because the Distributor's accounting (depositPools[rewardPoolIndex_][poolAddress].deposited) no longer reflects their share. This results in permanent fund lock or cross-pool theft.


Severity

  • CVSS 3.1 Score: 9.0 (Critical)
  • Vector: AV:N/AC:L/PR:N/UI:N/S:C/C:N/I:N/A:H
  • Impact: Direct loss of user deposits — complete loss of availability for funds in non-migrated-to pools.

Affected Component

  • Repository: MorpheusAIs/SmartContracts
  • File: contracts/capital-protocol/DepositPool.sol
  • Current version: v7 (commit 868db5537b8d63fa0eaa114f3c17f83efb1ca994)
  • Status: Previously flagged as M-04 in Code4rena audit — still unfixed in the current codebase

Root Cause

The migrate() function (lines ~189-210 in DepositPool.sol v7) sends the entire totalDepositedInPublicPools to a single reward pool index:

function migrate(uint256 rewardPoolIndex_) external onlyOwner {
    require(!isMigrationOver, "DS: the migration is over");
    if (totalDepositedInPublicPools == 0) {
        isMigrationOver = true;
        emit Migrated(rewardPoolIndex_);
        return;
    }

    IRewardPool rewardPool_ = IRewardPool(IDistributor(distributor).rewardPool());
    rewardPool_.onlyExistedRewardPool(rewardPoolIndex_);
    rewardPool_.onlyPublicRewardPool(rewardPoolIndex_);

    // Transfer yield
    uint256 remainder_ = IERC20(depositToken).balanceOf(address(this)) - totalDepositedInPublicPools;
    require(remainder_ > 0, "DS: yield for token is zero");
    IERC20(depositToken).transfer(distributor, remainder_);
    IERC20(depositToken).approve(distributor, totalDepositedInPublicPools);

    // @audit - sends ALL deposits to ONE rewardPoolIndex_
    IDistributor(distributor).supply(rewardPoolIndex_, address(this), totalDepositedInPublicPools);

    isMigrationOver = true;
    emit Migrated(rewardPoolIndex_);
}
However, user deposits are tracked per reward pool index via:

solidity



mapping(address => mapping(uint256 => UserData)) public usersData;
// usersData[userAddress][rewardPoolIndex_] = UserData
When a DepositPool serves multiple public reward pools (e.g., index 0 and index 1), each with its own user deposits, calling migrate(0) sends all tokens to reward pool 0. The Distributor's depositPools[0][poolAddress].deposited is credited with the full amount, while depositPools[1][poolAddress].deposited remains unchanged.

Users who deposited into pool 1 now have their physical tokens accounted under pool 0. Their withdraw() call — which queries the Distributor via rewardPoolIndex_ = 1 — finds insufficient or zero deposited balance and reverts, permanently locking their funds.

Attack Flow
Setup: A DepositPool is deployed with 2 public reward pools (index 0 and index 1).
Staking:
User A stakes 500 stETH in pool 0 → usersData[A][0].deposited = 500
User B stakes 500 stETH in pool 1 → usersData[B][1].deposited = 500
totalDepositedInPublicPools = 1000
Migration: Owner calls migrate(0).
Distributor's depositPools[0][poolAddr].deposited += 1000
isMigrationOver = true
Outcome for User B:
User B calls withdraw(1, 500 ether) → DepositPool queries IDistributor(distributor).withdraw(1, userB, 500)
Distributor checks depositPools[1][poolAddr] → deposited is 0 (or some incorrect value)
Transaction reverts — User B's 500 stETH is permanently locked in the protocol
Alternative outcome: If the withdraw() does not revert, User A (pool 0) can drain the migrated funds that belong to User B — cross-user theft.
Proof of Concept
solidity



// Hardhat test demonstrating the vulnerability
describe("DepositPool migrate() - fund lock", () => {
    it("should lock Pool 1 users' funds after migrate(0)", async () => {
        // Deploy DepositPool with 2 public reward pools
        const depositPool = await deployDepositPool(depositToken);
        
        // User A deposits 500 into pool 0
        await depositPool.connect(userA).stake(0, ethers.utils.parseEther("500"), ZERO_ADDRESS);
        // User B deposits 500 into pool 1
        await depositPool.connect(userB).stake(1, ethers.utils.parseEther("500"), ZERO_ADDRESS);
        
        // Owner migrates ALL funds to pool 0
        await depositPool.connect(owner).migrate(0);
        
        // User B tries to withdraw from pool 1 — REVERTS
        await expect(
            depositPool.connect(userB).withdraw(1, ethers.utils.parseEther("500"))
        ).to.be.reverted; // "DR: deposit pool amount exceeded" or similar
    });
});
Impact Analysis


Factor	Value
Direct financial risk	100% of deposits in non-migrated-to public pools (potentially millions in TVL)
Attack complexity	Low — single onlyOwner transaction
User action required	None — funds are locked without user error
Likelihood	Medium (if owner is malicious or if a cross-contract migration script targets the wrong index)
Who is affected	All users who deposited into public pools with indices ≠ the migration target
Recommended Remediation
The migrate() function must handle each public reward pool proportionally. There are two approaches:

Option 1: Track deposits per reward pool and migrate proportionally
Introduce a per-reward-pool deposit tracker and iterate:

solidity



// Add storage
mapping(uint256 => uint256) public depositedPerPublicPool;

// Update stake() to increment depositedPerPublicPool[rewardPoolIndex_]
// Update withdraw() to decrement depositedPerPublicPool[rewardPoolIndex_]

// Fix migrate():
function migrate(uint256 rewardPoolIndex_) external onlyOwner {
    require(!isMigrationOver, "DS: the migration is over");
    isMigrationOver = true;

    IRewardPool rewardPool_ = IRewardPool(IDistributor(distributor).rewardPool());
    
    // Iterate over ALL public reward pools known to this DepositPool
    uint256[] memory publicPoolIndices = getPublicPoolIndices(); // or hardcode known range
    for (uint256 i = 0; i < publicPoolIndices.length; i++) {
        uint256 poolIndex = publicPoolIndices[i];
        uint256 poolDeposited = depositedPerPublicPool[poolIndex];
        if (poolDeposited == 0) continue;
        
        rewardPool_.onlyExistedRewardPool(poolIndex);
        rewardPool_.onlyPublicRewardPool(poolIndex);
        
        // Transfer yield proportionally
        uint256 remainder_ = /* proportional yield */;
        if (remainder_ > 0) {
            IERC20(depositToken).transfer(distributor, remainder_);
        }
        
        IERC20(depositToken).approve(distributor, poolDeposited);
        IDistributor(distributor).supply(poolIndex, address(this), poolDeposited);
    }
    
    emit Migrated(rewardPoolIndex_);
}
Option 2: Require migration to be called once per pool (simpler)
solidity



mapping(uint256 => bool) public poolMigrationDone;

function migrate(uint256 rewardPoolIndex_) external onlyOwner {
    require(!poolMigrationDone[rewardPoolIndex_], "DS: pool already migrated");
    require(!isMigrationOver, "DS: the migration is over");
    
    IRewardPool rewardPool_ = IRewardPool(IDistributor(distributor).rewardPool());
    rewardPool_.onlyExistedRewardPool(rewardPoolIndex_);
    rewardPool_.onlyPublicRewardPool(rewardPoolIndex_);

    // Only migrate the deposits belonging to THIS pool
    uint256 poolDeposited = depositedPerPublicPool[rewardPoolIndex_];
    require(poolDeposited > 0, "DS: no deposits for this pool");
    
    uint256 remainder_ = IERC20(depositToken).balanceOf(address(this)) - totalDepositedInPublicPools;
    // ... proportional yield per pool ...
    
    IERC20(depositToken).approve(distributor, poolDeposited);
    IDistributor(distributor).supply(rewardPoolIndex_, address(this), poolDeposited);
    
    poolMigrationDone[rewardPoolIndex_] = true;
    
    // Check if all pools are done
    if (allPublicPoolsMigrated()) {
        isMigrationOver = true;
    }
    
    emit Migrated(rewardPoolIndex_);
}
References
Previous audit finding: Code4rena M-04 (same root cause, confirmed unfixed)
Current code: DepositPool.sol v7
Distributor: DistributorV2.sol


Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions