`RebalanceManager.sol#setRebalanceManager()` should implement two-step transfer pattern

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-12-amun/blob/98f6e2ff91f5fcebc0489f5871183566feaec307/contracts/basket/contracts/callManagers/RebalanceManager.sol#L57-L63

function setRebalanceManager(address _rebalanceManager)
    external
    onlyRebalanceManager
{
    rebalanceManager = _rebalanceManager;
    emit RebalanceManagerSet(_rebalanceManager);
}

RebalanceManager.rebalanceManager is a critical role, if the current rebalanceManager mistakenly calls setRebalanceManager() with a wrong address, it can result in all the onlyRebalanceManager() methods being unaccessible, and it cannot be undo.

Recomandation

Consider changing the setRebalanceManager() function to first nominate an address as the pending rebalanceManager and adding an acceptRebalanceManager() function which is called by the pending rebalanceManager to confirm the transfer.

Also in:

https://github.com/code-423n4/2021-12-amun/blob/98f6e2ff91f5fcebc0489f5871183566feaec307/contracts/basket/contracts/callManagers/RebalanceManagerV2.sol#L43-L49

function setRebalanceManager(address _rebalanceManager)
        external
        onlyRebalanceManager
    {
        rebalanceManager = _rebalanceManager;
        emit RebalanceManagerSet(_rebalanceManager);
    }

https://github.com/code-423n4/2021-12-amun/blob/98f6e2ff91f5fcebc0489f5871183566feaec307/contracts/basket/contracts/callManagers/RebalanceManagerV3.sol#L48-L54

function setRebalanceManager(address _rebalanceManager)
        external
        onlyRebalanceManager
    {
        rebalanceManager = _rebalanceManager;
        emit RebalanceManagerSet(_rebalanceManager);
    }

[0xleastwood]

I think this can only be argued as non-critical as it involves mis-use of a transfer function by the previous rebalance manager.