GimelSec - removeAdapter() doesn't pop the market index in withdrawSeq, leading to users not being able to call withdraw

[sherlock-admin]

GimelSec

medium

removeAdapter() doesn't pop the market index in withdrawSeq, leading to users not being able to call withdraw

Summary

Admin can remove adapters in AssetManager, but removeAdapter() doesn’t remove the market index from withdrawSeq. It will be reverted when users call withdraw() due to the non-exist index.

Vulnerability Detail

removeAdapter() removes an adapter from moneyMarkets[]:

    function removeAdapter(address adapterAddress) external override onlyAdmin {
        bool isExist = false;
        uint256 index;
        uint256 moneyMarketsLength = moneyMarkets.length;

        for (uint256 i = 0; i < moneyMarketsLength; i++) {
            if (adapterAddress == address(moneyMarkets[i])) {
                isExist = true;
                index = i;
                break;
            }
        }

        if (isExist) {
            moneyMarkets[index] = moneyMarkets[moneyMarketsLength - 1];
            moneyMarkets.pop();
        }
    }

Because addAdapter() also push market index into withdrawSeq in L430, removeAdapter() should also remove the market index (index of moneyMarkets[]) from withdrawSeq.

Impact

If removeAdapter() doesn’t remove the market index from withdrawSeq, users will not be able to call withdraw function due to the non-exist index in L349.

Code Snippet

https://github.com/sherlock-audit/2022-10-union-finance/blob/main/union-v2-contracts/contracts/asset/AssetManager.sol#L440

Tool used

Manual Review

Recommendation

Pop the index of adapterAddress in withdrawSeq.

Duplicate of #76