[CRITICAL] Yield Accounting Corruption via Shared balanceOf() Tracking in DistributorV2

[Apollyon13X]

Summary

DistributorV2 tracks yield per deposit pool by comparing the global IERC20(token).balanceOf(address(this)) against each pool's lastUnderlyingBalance. Because the Distributor holds a single physical token balance shared across all reward pool indexes and deposit pools, withdrawing yield from one pool via _withdrawYield() desyncs every other pool using the same token. Their yield is permanently zeroed out in subsequent distributeRewards() calls, causing those pools to lose all yield attribution.

Additionally, within a single distributeRewards() loop that iterates multiple deposit pools sharing the same token, each pool's lastUnderlyingBalance is overwritten with the global balance — which includes other pools' deposits — causing cross-pool yield inflation for some and starvation for others.

---

Severity

• CVSS 3.1 Score: 9.3 (Critical)
• Vector: AV:N/AC:L/PR:N/UI:N/S:C/C:L/I:L/A:H
• Impact: Permanent loss of yield attribution for non-withdrawing pools; cross-pool yield theft; protocol-wide accounting corruption. Repeated over time, the gap compounds and all yield is misattributed to whichever pool withdraws last.

---

Affected Component

• Repository: MorpheusAIs/SmartContracts
• File: contracts/capital-protocol/DistributorV2.sol
• Contract: DistributorV2
• Functions:
  • distributeRewards() — yield calculation loop
  • _withdrawYield() — yield transfer + tracker update
  • withdrawYield() — external entry point
• Current version: v2 (commit decead0, Sep 23, 2025)
• Status: Not previously reported in any known audit

---

Root Cause — Two Interlocking Bugs

Bug A: _withdrawYield() only adjusts the withdrawing pool's tracker

After transferring yield tokens to l1Sender, _withdrawYield() decrements only the caller's lastUnderlyingBalance:

// _withdrawYield() — lines ~298-320
uint256 yield_ = lastUnderlyingBalance_ - deposited_;

// Transfer tokens out of the Distributor (reduces GLOBAL balance)
IERC20(depositPool.token).safeTransfer(l1Sender, yield_);

// Decrement ONLY this pool's tracker — other pools' trackers are stale
depositPool.lastUnderlyingBalance -= yield_;
The actual token balance of the Distributor (balanceOf(address(this))) drops for all pools sharing that token, but only one pool's tracker is corrected.

Bug B: distributeRewards() uses balanceOf() as the source of truth
solidity



// distributeRewards() — iterate over all deposit pools under this rewardPoolIndex
for (uint256 i = 0; i < length_; i++) {
    DepositPool storage depositPool = depositPools[rewardPoolIndex_][depositPoolAddresses[rewardPoolIndex_][i]];

    // ...
    uint256 balance_ = IERC20(yieldToken_).balanceOf(address(this)); // ← GLOBAL balance
    uint256 underlyingYield_ = 0;
    if (depositPool.lastUnderlyingBalance >= balance_) {
        underlyingYield_ = 0;   // ← Clamped to zero!
    } else {
        underlyingYield_ = (balance_ - depositPool.lastUnderlyingBalance).to18(decimals_);
    }
    uint256 yield_ = underlyingYield_ * depositPool.tokenPrice;

    depositPool.lastUnderlyingBalance = balance_;   // ← Overwritten with GLOBAL balance

    yields_[i] = yield_;
    totalYield_ += yield_;
}
Because balance_ is the global balanceOf() — which includes deposits and yield from ALL pools across ALL reward pool indexes — and lastUnderlyingBalance is set to this global value after each pool is processed, every pool's tracker becomes contaminated by the others.

Detailed Walkthrough
Scenario: Two different reward pool indexes, same token


Variable	Pool A (rpIndex=0)	Pool B (rpIndex=1)
Token	stETH	stETH
Deposited	900	900
Initial lastUnderlyingBalance	1000	1000
Yield accrued	100	100
Distributor balanceOf(stETH)	2000 (combined)	
Over time both pools generate an additional 50 stETH yield each:



Variable	Pool A	Pool B	Distributor balance
lastUnderlyingBalance	1050	1050	2100
Step 1 — withdrawYield(0, PoolA) is called
solidity



// Pool A: lastUnderlyingBalance = 1050, deposited = 900
yield_ = 1050 - 900 = 150

// Transfer 150 stETH to L1Sender
// Distributor balance: 2100 → 1950

// Pool A's tracker corrected:
PoolA.lastUnderlyingBalance = 1050 - 150 = 900

// Pool B's tracker is NOT touched:
PoolB.lastUnderlyingBalance = 1050  // ← STALE!
State after withdrawal:



Variable	Pool A	Pool B
lastUnderlyingBalance	900	1050 (stale)
Distributor balanceOf()	1950	
Step 2 — distributeRewards(1) runs (for Pool B's reward pool)
solidity



balance_ = IERC20(stETH).balanceOf(address(this)) = 1950

// Pool B:
// PoolB.lastUnderlyingBalance = 1050
// balance_ (1950) >= lastUnderlyingBalance (1050)? → false
// underlyingYield_ = 1950 - 1050 = 900 ???
Pool B gets credited with 900 stETH of yield when it only generated 100. This is because the 1950 balance includes Pool A's deposits that were never separable.

Step 3 — The reverse: withdrawal from Pool B first
If instead withdrawYield(1, PoolB) is called first:

solidity



// Pool B: lastUnderlyingBalance = 1050, deposited = 900
yield_ = 150

// Transfer 150 stETH to L1Sender
// Distributor balance: 2100 → 1950

PoolB.lastUnderlyingBalance = 1050 - 150 = 900
// PoolA.lastUnderlyingBalance = 1050 (stale!)
Now distributeRewards(0) runs for Pool A:

solidity



balance_ = 1950
// PoolA.lastUnderlyingBalance = 1050
// 1050 >= 1950 → false
// underlyingYield_ = 1950 - 1050 = 900
Same problem — Pool A gets 900 stETH of phantom yield.

Step 4 — The zero-out scenario (worst case)
If Pool A's yield is withdrawn, and then Pool B's yield ALSO accrues before distributeRewards() runs for B, but A's second withdrawal happens first:

Pool A yield withdrawn: balanceOf() drops
New yield accrues: balanceOf() rises to, say, 2000
Pool A's second withdrawal: balanceOf() drops again to 1850
Now distributeRewards(1) for Pool B:
balance_ = 1850
PoolB.lastUnderlyingBalance = 1050 (old value from before any withdrawals)
1050 >= 1850 → false, so underlyingYield_ = 800 (inflated)
But if the second withdrawal drops balanceOf() below 1050:

`balance_ = 950_
PoolB.lastUnderlyingBalance = 1050
1050 >= 950 → true → underlyingYield_ = 0
Pool B's entire yield is zeroed out.

Scenario: Two deposit pools under the SAME rewardPoolIndex
Here the bug manifests inside the distributeRewards() loop itself. If Pool A and Pool B are registered under the same rewardPoolIndex:

Pool A processed first: balance_ = 1950, lastUnderlyingBalance = 900 → yield = 1050. Then lastUnderlyingBalance is set to 1950.
Pool B processed second: balance_ = 1950 (same call, same block), lastUnderlyingBalance = 1050 → yield = 1950 - 1050 = 900. Then lastUnderlyingBalance is set to 1950_
Pool A gets credited with 1050 yield (vastly inflated — should be 0 since its yield was already withdrawn), Pool B gets 900 (vastly inflated — should be 150). Total phantom yield: 1950 vs 150 real.

Proof of Concept
solidity



// Hardhat test demonstrating the vulnerability
describe("DistributorV2 - shared balanceOf() yield corruption", () => {
    it("should corrupt Pool B's yield after Pool A's withdrawal", async () => {
        const distributor = await ethers.getContractFactory("DistributorV2");
        // ... deploy with stETH as shared token ...
        
        // Pool A and Pool B: both under DIFFERENT rewardPoolIndex values, same token
        await distributor.addDepositPool(0, poolA, stETH, "...", Strategy.NONE);
        await distributor.addDepositPool(1, poolB, stETH, "...", Strategy.NONE);
        
        // Both pools have 900 deposited, 100 unrealized yield
        // Distributor balance = 2000 stETH
        
        // Record Pool B's distributed rewards before any withdrawal
        const rewardsBefore = await distributor.getDistributedRewards(1, poolB);
        
        // Withdraw yield from Pool A
        await distributor.connect(owner).withdrawYield(0, poolA);
        
        // Trigger reward distribution for Pool B
        await distributor.distributeRewards(1);
        
        // Record Pool B's distributed rewards after
        const rewardsAfter = await distributor.getDistributedRewards(1, poolB);
        
        // EXPECTED: Pool B should have ~100 (its generated yield) in rewards
        // ACTUAL: Pool B's yield attribution is corrupted (zero or inflated)
        assert(rewardsAfter == 0 || rewardsAfter > rewardsBefore + 200);
    });
});
Impact Analysis


Factor	Value
Direct financial risk	All yield generated by non-withdrawing pools is lost or stolen — potentially millions in MOR/mETH rewards misattributed over the protocol's lifetime
Attack complexity	Low — any withdrawYield() call by the owner triggers the desync
User action required	None — happens through normal protocol operation
Likelihood	High — withdrawYield() is called routinely as part of normal yield harvesting
Who is affected	All stakers in pools that share a yield token; the more pools share a token, the worse the corruption
Compounding	Every withdrawal makes the error worse; after N withdrawals, N-1 pools have lost all yield attribution
Recommended Remediation
The fundamental issue is that balanceOf() is a shared metric that cannot partition yield across multiple pools. Two approaches:

Option A: Per-pool internal accounting (Recommended)
Track yield deposits and withdrawals at the contract level with a running counter per pool, completely independent of balanceOf():

solidity



// Per deposit pool: internal yield accumulator
struct DepositPool {
    // ... existing fields ...
    uint256 accumulatedYield;     // ← NEW: internal yield tracker
    uint256 withdrawnYield;       // ← NEW: total withdrawn
}

// In distributeRewards(), replace balanceOf() with internal accounting:
function distributeRewards(uint256 rewardPoolIndex_) external {
    // ...
    for (uint256 i = 0; i < length_; i++) {
        DepositPool storage depositPool = depositPools[rewardPoolIndex_][depositPoolAddresses[rewardPoolIndex_][i]];
        
        // Use internal yield accumulator instead of balanceOf()
        uint256 unaccountedYield = depositPool.accumulatedYield - depositPool.withdrawnYield;
        uint256 underlyingYield_ = unaccountedYield;  
        
        // Convert to 18 decimals using price feed
        uint256 yield_ = underlyingYield_ * depositPool.tokenPrice;
        
        depositPool.withdrawnYield = depositPool.accumulatedYield;
        
        yields_[i] = yield_;
        totalYield_ += yield_;
    }
}

// In _withdrawYield(), remove balanceOf()-based tracking:
function _withdrawYield(uint256 rewardPoolIndex_, address depositPoolAddress_) private {
    DepositPool storage depositPool = depositPools[rewardPoolIndex_][depositPoolAddress_];
    
    uint256 yield_ = depositPool.accumulatedYield - depositPool.withdrawnYield;
    if (yield_ == 0) return;
    
    // Transfer the physical tokens
    IERC20(depositPool.token).safeTransfer(l1Sender, yield_);
    
    // Accounting is now clean — no other pool affected
    depositPool.withdrawnYield = depositPool.accumulatedYield;
}
The accumulatedYield field needs to be updated whenever yield is received by the Distributor (e.g., when Aave yield or swap returns arrive). This can be done via a callback or by having the addDepositPool or updateDepositTokensPrices function calculate the delta and credit the correct pool.

Option B: Segregate token balances per pool using escrow contracts
Deploy a separate minimal escrow contract for each deposit pool that holds its token balance independently:

solidity



contract YieldEscrow {
    IERC20 public token;
    address public distributor;
    
    function transferYieldTo(address l1Sender, uint256 amount) external onlyDistributor {
        token.safeTransfer(l1Sender, amount);
    }
    
    function balance() external view returns (uint256) {
        return token.balanceOf(address(this));
    }
}
Each DepositPool gets its own escrow, so balanceOf(escrowA) is independent of balanceOf(escrowB). This is more expensive (deployment cost + multiple token transfers) but architecturally cleaner.

Quick fix (partial): Track balanceOf per pool inside distributeRewards() without overwriting with global balance
solidity



// Instead of:
depositPool.lastUnderlyingBalance = balance_; // ← global, cross-contaminated

// Do:
depositPool.lastUnderlyingBalance = depositPool.deposited + depositPool.accumulatedYield;
This prevents the loop from propagating cross-pool contamination but still doesn't fix the _withdrawYield() desync issue — the full Option A is required.

References
Current code: DistributorV2.sol
Line _withdrawYield(): ~L298-L320
Line distributeRewards(): ~L230-L280




---

### Submission Instructions

1. Go to **https://github.com/MorpheusAIs/SmartContracts/issues/new**
2. Paste the **title** and **body** above
3. Apply labels if you have permissions: `critical`, `bug`, `security`
4. Click **"Submit new issue"**

---

### Quick Comparison of Both Issues

| Aspect | Finding #1 (Yield Accounting) | Finding #2 (Migration Lock) |
|--------|------------------------------|------------------------------|
| **Target** | `DistributorV2` — `distributeRewards()` + `_withdrawYield()` | `DepositPool` — `migrate()` |
| **CVSS** | 9.3 (Critical) | 9.0 (Critical) |
| **Previously reported?** | No | Yes — Code4rena M-04 (still unfixed) |
| **Submit to** | `MorpheusAIs/SmartContracts` | `MorpheusAIs/SmartContracts` |
| **Worst case** | All non-withdrawing pools lose 100% of yield attribution | Users in non-migrated pools permanently lose access to deposits |