CS - BaseVault out-of-date provider

[0xdcota]

Where: https://github.com/Fujicracy/fuji-v2/blob/ab02d2308797577973ac358af8c7aadf973bcec7/packages/protocol/src/abstracts/BaseVault.sol#L487

Description: The BaseVault has the setActiveProvider functions, callable by rebalancer, that allows to change the current provider. The function is called during rebalancing. It is crucial to call this function within one transaction to make the rebalancing process an atomic process. Otherwise, the vault might be used by users after the provider is changed and before the funds are moved, which would result in transactions being reverted (e.g. borrowing from a new provider before it gets collateral).

Recommendation: Make the function internal and call it during the rebalancing process.

[0xdcota]

Notes regarding this vulnerability.

Challenges:

• We use in our testing framework the setActiveProvider to specifically test features of a newly implemented provider.
• We also assign the first provider of a freshly deployed vault calling setActiveProvider.

Assumptions:

• We do not intend to assign the rebalancer role to arbitrary externally owned accounts (EOA).
• The "rebalancer role" should be restricted to RebalancerManager.sol type contracts.

Facts:

• The only account that gets assigned "rebalancer role" is the EOA deployer of the Chief.sol contract. This is a requirement in order to setActiveProvider in our testing set-ups.
• "Rebalancer role" can only be assigned via the timelock to a new address.
• We do not have a way to restrict-check that the "rebalancer role" gets assigned to an arbitrary EOA.

[0xdcota]

With the notes from my previous post I suggest the safest, simplest solution is to change that function setActiveProvider() can only be called by the timelock.

Why this is good:

1. In case of setting an active provider that has no liquidity, the users will have an open window of time to withdraw (in production).
2. We can still set and change provider in our test environment.
3. We keep have the possibility, in production and via timelock, to hard set the active provider in case some future exploit happens with the current provider.

@brozorec @pedrovalido please share your thoughts on this.

[brozorec]

What to do with L82 of RebalancerManager?