0xAristos - Liquidity Lockdown: Zero Allocation Bug Traps Assets in Vulnerable Markets!

[sherlock-admin4]

0xAristos

High

Liquidity Lockdown: Zero Allocation Bug Traps Assets in Vulnerable Markets!

Summary

The incorrect handling of zero allocations in CuratedVault::reallocate will cause an inability to fully withdraw assets from affected markets for the protocol and its users. An allocator, attempting to fully withdraw from a market with zero allocation, will be unable to do so, as the function bypasses the intended behavior and fails to execute the withdrawal. This will leave assets trapped in such markets, causing severe liquidity risks and inconsistent reallocation operations.

Root Cause

(https://github.com/sherlock-audit/2024-06-new-scope/blob/c8300e73f4d751796daad3dadbae4d11072b3d79/zerolend-one/contracts/core/vaults/CuratedVault.sol#L232)

the check that sets toWithdraw = 0 when allocation.assets == 0 leads to an incomplete withdrawal from the market. This incorrect behavior occurs due to the bypass of supplyShares withdrawal logic in the zero-allocation case, preventing the removal of the full supply balance from the pool.

Internal pre-conditions

• The protocol's allocator callsCuratedVault::reallocate with one of the market allocations set to zero (allocation.assets == 0).
• The market has a non-zero supply balance (supplyAssets > 0).
• The protocol expects to fully withdraw from that market in the reallocation process.

External pre-conditions

• A market could be under duress due to external factors, such as an exploit or a severe drop in asset value.
• Emergency measures require the protocol to fully withdraw assets from that market to prevent further losses.

Attack Path

• The allocator attempts to reallocate liquidity across several markets, including one where allocation.assets == 0.
• The allocator expects a full withdrawal from the market with zero allocation.
• Due to the misconfigured logic, the protocol sets toWithdraw = 0, failing to withdraw any assets from the pool.
  The allocation remains incomplete, and liquidity remains trapped in the affected market.

Impact

The protocol suffers a major liquidity risk as affected markets cannot be fully withdrawn from when needed. In a worst-case scenario, this could prevent proper fund reallocation during times of crisis, leaving significant amounts of assets vulnerable to external attacks or value degradation. Additionally, the system will revert with an CuratedErrorsLib.InconsistentReallocation() error when the total withdrawn assets do not match the total supplied assets, disrupting normal operations. This leads to operational disruption, liquidity mismanagement as funds get stuck in certain market!

PoC

  function reallocate(MarketAllocation[] calldata allocations) external onlyAllocator {
    uint256 totalSupplied;
    uint256 totalWithdrawn;

    for (uint256 i; i < allocations.length; ++i) {
      MarketAllocation memory allocation = allocations[i];
      IPool pool = allocation.market;

      (uint256 supplyAssets, uint256 supplyShares) = _accruedSupplyBalance(pool);
      uint256 toWithdraw = supplyAssets.zeroFloorSub(allocation.assets);

      if (toWithdraw > 0) {
        if (!config[pool].enabled) revert CuratedErrorsLib.MarketNotEnabled(pool);

        // Guarantees that unknown frontrunning donations can be withdrawn, in order to disable a market.
        uint256 shares;
        if (allocation.assets == 0) {
          shares = supplyShares;
         @> toWithdraw = 0; // No withdrawal happens
        }

        DataTypes.SharesType memory burnt = pool.withdrawSimple(asset(), address(this), toWithdraw, 0);
        emit CuratedEventsLib.ReallocateWithdraw(_msgSender(), pool, burnt.assets, burnt.shares);
        totalWithdrawn += burnt.assets;
      }

Mitigation

Modify the logic to ensure that when allocation.assets == 0, the function correctly withdraws the entire supplyAssets by setting toWithdraw = supplyAssets instead of 0. This will ensure full withdrawal from the market during the reallocation process.

if (allocation.assets == 0) {
    shares = supplyShares;
 -    toWithdraw = 0;
+    toWithdraw = supplyAssets; // Withdraw the full supply when allocation is zero
}

Duplicate of #434