Restoring the vault can result in big losses if balances are changed
Summary
When a vault is in a paused state, it indicates an emergency situation where an emergency withdrawal has been executed. This withdrawal removes the balanced liquidity from all coins of the underlying tokens. Restoring the vault involves adding liquidity using the current balances of the vault, assuming that whatever tokens were withdrawn from the pool during the emergency exit will be balanced when deposited back into the pool during restoration. However, changes in pool balances can occur when the vault decides to restore, potentially resulting in the vault receiving fewer LP tokens based on the current balances.
Vulnerability Detail
Let's consider an example where the underlying Curve pool utilized by the vault contains a total of 100 tokens, with 20 tokens being X and the remaining 80 tokens being Y within the pool. For simplicity, let's assume that half of the LP tokens are held by the vault.
At a certain point, an emergency withdraw is initiated by the emergency role holder, causing the vault to remove its balanced liquidity. Given that the vault possesses half of the liquidity, the withdrawn amounts are 10 X and 40 Y.
After some time, the emergency situation is resolved, and the administrator intends to restore the vault. However, the underlying pool composition has changed, now consisting of 80% X and 20% Y, while maintaining the same total token sum. In this scenario, restoring the vault solely with the existing balances might not be the most prudent choice. This assumption is made based on the possibility of external markets where the vault's underlying tokens can be exchanged and added to the pool more efficiently, aiming to maximize the LP.
Aside from the above example, it is also possible for an attacker to manipulate the pool balances to give Notional a lesser LP share hence lesser value on the strategy token.
Impact
The above scenarios are considering the total sum of tokens are not changed but only the ratios did. It is also possible that the sum of tokens has changed which would lead to Notional to get higher or lower LP token amounts. However, this is not something that Notional admin can control so that's why I didn't include that part in above. However, considering sums are same or very near to what it was when emergency withdraw is called, just because the lack of selling optionality vault will have less LP tokens and users will have less LP token claims. That's why I interpreted this as medium.
Allow the notional owner to sell some tokens to external markets just as it allows in deposits. Notional admin is trusted and also executeDepositTrades are well protected so it does make sense to allow restoreVault sell tokens when adding LP back to pool.
mstpr-brainbot
medium
Restoring the vault can result in big losses if balances are changed
Summary
When a vault is in a paused state, it indicates an emergency situation where an emergency withdrawal has been executed. This withdrawal removes the balanced liquidity from all coins of the underlying tokens. Restoring the vault involves adding liquidity using the current balances of the vault, assuming that whatever tokens were withdrawn from the pool during the emergency exit will be balanced when deposited back into the pool during restoration. However, changes in pool balances can occur when the vault decides to restore, potentially resulting in the vault receiving fewer LP tokens based on the current balances.
Vulnerability Detail
Let's consider an example where the underlying Curve pool utilized by the vault contains a total of 100 tokens, with 20 tokens being X and the remaining 80 tokens being Y within the pool. For simplicity, let's assume that half of the LP tokens are held by the vault.
At a certain point, an emergency withdraw is initiated by the emergency role holder, causing the vault to remove its balanced liquidity. Given that the vault possesses half of the liquidity, the withdrawn amounts are 10 X and 40 Y.
After some time, the emergency situation is resolved, and the administrator intends to restore the vault. However, the underlying pool composition has changed, now consisting of 80% X and 20% Y, while maintaining the same total token sum. In this scenario, restoring the vault solely with the existing balances might not be the most prudent choice. This assumption is made based on the possibility of external markets where the vault's underlying tokens can be exchanged and added to the pool more efficiently, aiming to maximize the LP.
Aside from the above example, it is also possible for an attacker to manipulate the pool balances to give Notional a lesser LP share hence lesser value on the strategy token.
Impact
The above scenarios are considering the total sum of tokens are not changed but only the ratios did. It is also possible that the sum of tokens has changed which would lead to Notional to get higher or lower LP token amounts. However, this is not something that Notional admin can control so that's why I didn't include that part in above. However, considering sums are same or very near to what it was when emergency withdraw is called, just because the lack of selling optionality vault will have less LP tokens and users will have less LP token claims. That's why I interpreted this as medium.
Code Snippet
https://github.com/sherlock-audit/2023-10-notional/blob/7aadd254da5f645a7e1b718e7f9128f845e10f02/leveraged-vaults/contracts/vaults/common/SingleSidedLPVaultBase.sol#L480-L525
Tool used
Manual Review
Recommendation
Allow the notional owner to sell some tokens to external markets just as it allows in deposits. Notional admin is trusted and also executeDepositTrades are well protected so it does make sense to allow restoreVault sell tokens when adding LP back to pool.
Duplicate of #82