search

sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

Jeiwan - Deprecated Balancer Price Oracles could lead to locked funds in the Balancer strategy vaults #46

Open sherlock-admin opened 2 years ago

sherlock-admin commented 2 years ago

Jeiwan

medium

Deprecated Balancer Price Oracles could lead to locked funds in the Balancer strategy vaults

Summary

The Balancer strategy vaults (Boosted3TokenAuraVault and MetaStable2TokenAuraVault) use the price oracle of related Balancer vaults during settlement. However, price oracles in Balancer vaults were deprecated. It's likely that liquidity will be drained from such vaults and will be moved to new vaults. Lowered liquidity will result it price deviation, which will lead to failing settlement due to the cross-checking with Chainlink oracles.

Vulnerability Detail

During settlement of the Balancer strategy vaults, token spot prices are queried from Balancer Price Oracle. The spot prices are then compared to those reported by Chainlink. If the difference is too big, settlement will fail. Lowered liquidity in the deprecated Balancer vaults will result in high price deviation, blocked settlement, and locked funds.

Impact

Since Balancer has deprecated price oracles in its vaults and advised against using the vaults with price oracles enabled (they won't be disabled), it's likely that liquidity will be removed from such vaults and will be moved to new Balancer vaults that don't have the price oracle functionality. Since the Balancer strategy vaults of Notional are integrated with such deprecated Balancer vaults, it's likely that the strategy vaults will be impacted by lowered liquidity of the Balancer vaults. Lower liquidity will result in higher slippage, which means higher deviation of Balancer price oracle reported spot prices compared to those of Chainlink. In the case when price deviation is higher than defined in the oraclePriceDeviationLimitPercent setting (which is very likely due to Balancer recommending against using the deprecated vaults), settlement won't be possible and funds will be locked.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/math/Stable2TokenOracleMath.sol#L76-L77 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/math/Stable2TokenOracleMath.sol#L43-L65

Tool used

Manual Review

Recommendation

Short term, don't revert (Errors.InvalidPrice) in case of a price deviation and use the Chainlink price instead. Long term, migrate to the new Balancer vaults that don't have a price oracle and use Chainlink only.

jeffywu commented 2 years ago

@T-Woodward / @weitianjie2000

Good call on the balancer oracle deprecation, although existing pools will continue to have them.

T-Woodward commented 2 years ago

Yup confirmed, we're removing the balancer oracle dependency

jeffywu commented 1 year ago

Fixed here: https://github.com/notional-finance/leveraged-vaults/pull/35