# Changing providers might lead to lost assets in BorrowingVault and YieldVault Crit/High · Three Sigma · Lending aggregator · 6th May, 2023 Finding 3S-FUJI-H01 of the Fuji Finance security review. - Report: /reports/fuji-finance - Source: https://cdn.sanity.io/files/qoqld077/staging/32181a28eac3175d15fb8924d249bb0d91ca350c.pdf --- ### Description totalAssets(...) and totalDebt(...) are fetched from the set of providers, which can be changed. Thus, if providers change, but debt or assets are deposited/borrow from the old providers, totalAssets(...) and/or totalDebt(...) will change. In the case of totalAssets(...), this means that users shares would be worth less, such that existing users would take a loss and new users could profit from depositing at this reduced rate and, after/if the totalAssets(...) were recovered, take a profit. In the case of totalDebt(...), debtShares would be worth less, so existing users would make a profit by being able to payback their debt for less debt. New users would take a loss, following the same reasoning as before. ### Recommendation When removing a provider, check if the debt balance is bigger than 0, in which case, it should be repaid/moved to another provider first. In the case of the assets, ensuring that the deposit balance of a provider is 0 is vulnerable to a DoS attack, where attackers could deposit on the vault's behalf. Thus, when changing providers, if the deposit balance of an old provider is not 0, these assets could be moved atomically to a new provider. ### Status Acknowledged with the following statement: The issue identified is acknowledged by the team. However, current BorrowingVault smart contract bytecode size is a limitation for the proposed change and requires a major refactor that is planned to be introduced at a later stage. The team recognizes the risk which is only a temporary DOS to the vault's funds when a provider with assets is removed by mistake. The provider that was removed mistakenly can always be reestablished by a subsequent timelock call. --- Related findings: - [Changing the epoch duration will completely break the vault and the slashers](https://0xsimao.com/findings/symbiotic-relay-epoch-duration-completely-slashers): Symbiotic Relay - [ERC4626YieldBackend.withdrawSurplus() uses convertToAssets() which doesn't account for fees](https://0xsimao.com/findings/superfluid-erc4626-yield-withdraw-fees): Superfluid Yield Backends - [Lost withdrawals due to `ManagedLeveragedVault::openDen()` using all `asset()` balance](https://0xsimao.com/findings/beraborrow-i-lost-withdrawals-managed-den): Beraborrow Managed Dens - [Withdrawing all `lv` before expiry will lead to lost funds in the Vault](https://0xsimao.com/findings/cork-protocol-withdrawing-expiry-lost-funds): Cork Protocol