wickie - The vault cannot empty funds from a market when removing it.

[sherlock-admin2]

wickie

Medium

The vault cannot empty funds from a market when removing it.

Summary

To remove a market from the vault, the protocol recommends emptying a market first using CuratedVault::reallocate() and removing it with updateWithdrawQueue() as stated in the docs.( https://github.com/zerolend/zerolend-one/wiki/CuratedVault ) However, this is not possible because of a mistake in the reallocate(). CuratedVault.sol:250

Root Cause

To empty a market the allocator is to pass 0 in allocations.assets.
CuratedVault.sol:232

  function reallocate(MarketAllocation[] calldata allocations) external onlyAllocator {

However, In CuratedVault.sol:247-250, if allocation.assets == 0, it sets the toWithdraw to 0

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

Then the function tries to call pool.withdrawSimple with toWithdraw as 0. (CuratedVault.sol:253)

        DataTypes.SharesType memory burnt = pool.withdrawSimple(asset(), address(this), toWithdraw, 0);

This would result in the function reverting, as it does not take zero as input. (ValidationLogic.sol:97)

Internal pre-conditions

The allocator of the vault needs to call reallocate().

External pre-conditions

None

Attack Path

None

Impact

This would leave the vault no choice but to remove a market via CuratedVault::submitMarketRemoval() and updateWithdrawQueue(), which would leave to loss of funds of the market. Because of this bug, the only way to reallocate or empty funds from a market would be to ask all the suppliers of the vault to withdraw their funds, removing the market and asking them to supply back.

PoC

No response

Mitigation

If the developers want to make sure unknown front-running donations are withdrawn, simply set the toWithdraw to type(uint256).max. This will withdraw the whole vault balance of this particular market.(SupplyLogic.sol:121)

solidity
-       uint256 shares;
        if (allocation.assets == 0) {
-       shares = supplyShares;
-          toWithdraw = 0;
+         toWithdraw = type(uint256).max;
        }

Duplicate of #434