sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

xiaoming90 - Boosted Balancer Leverage Vault Might Not Be Able To Exit Its Position #93

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

medium

Boosted Balancer Leverage Vault Might Not Be Able To Exit Its Position

Summary

The boosted balancer leverage vault might not be able to exit its position under certain conditions as the BPT threshold check was not performed against all the involved Balancer pools.

Vulnerability Detail

Per the Notional walkthrough video (see 4:11min), the reason for having the BPT threshold is to ensure that the strategy does not hold too large of a share of the liquidity within the pool. Otherwise, the strategy will have a problem exiting its position.

At Line 335-342, if the number of BPT the vault held within a pool after joining exceeds the BPT threshold, the code will revert.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L325

File: Boosted3TokenPoolUtils.sol
325:     function _joinPoolAndStake(
326:         ThreeTokenPoolContext memory poolContext,
327:         StrategyContext memory strategyContext,
328:         AuraStakingContext memory stakingContext,
329:         BoostedOracleContext memory oracleContext,
330:         uint256 deposit,
331:         uint256 minBPT
332:     ) internal returns (uint256 bptMinted) {
333:         bptMinted = _joinPoolExactTokensIn(poolContext, deposit, minBPT);
334: 
335:         // Check BPT threshold to make sure our share of the pool is
336:         // below maxBalancerPoolShare
337:         uint256 bptThreshold = strategyContext.vaultSettings._bptThreshold(
338:             poolContext._getVirtualSupply(oracleContext)
339:         );
340:         uint256 bptHeldAfterJoin = strategyContext.totalBPTHeld + bptMinted;
341:         if (bptHeldAfterJoin > bptThreshold)
342:             revert Errors.BalancerPoolShareTooHigh(bptHeldAfterJoin, bptThreshold);
343: 
344:         // Transfer token to Aura protocol for boosted staking
345:         stakingContext.auraBooster.deposit(stakingContext.auraPoolId, bptMinted, true); // stake = true
346:     }

Balancer WETH/wstETH Pool

The following diagram shows the workflow for WETH/wstETH Balancer leverage vault during depositing. The leverage vault will join the WETH/wstETH Pool with the appropriate amount of WETH. Subsequently, the appropriate amount of BPT will be minted for the leverage vault.

In this context, if the number of BPT held by the vault exceeds 20% of the total supply of WETH/wsETH tokens, the transaction will revert to ensure that the strategy does not hold more than 20% of the total supply of WETH/wsETH BPT tokens. This is to ensure that the strategy will not have a problem exiting its position later.

The current design of BPT threshold check works perfectly for WETH/wstETH Balancer leverage vault in ensuring the vault remains liquidity.

flowchart LR;
    A[Notional WETH/wstETH Leverage Vault]--WETH-->B[WETH/wstETH Pool]--BPT-->A;

Balancer Boosted Aave USD Pool (bb-a-USD)

The following diagram shows the workflow for Boosted Balancer leverage vault during depositing. First, the leverage vault will swap in an appropriate amount of USDC to the Linear USDC Pool and receive an appropriate amount of BPT (bb-a-USDC) back. Next, the BPT (bb-a-USDC) will be swapped into the Boosted Stable USDC Pool, and an appropriate amount of Boosted BPT received back.

In this context, if the number of Boosted BPT held by the vault exceeds 20% of the total supply of Boosted BPT, the transaction will revert to ensure that the strategy does not hold more than 20% of the total supply of Boosted BPT tokens. This is to ensure that the strategy will not have a problem exiting its Boosted BPT position later.

The current design of BPT threshold works fine for MetaStable2Token vault. However, it does not work well for Boosted3Token vault. When the vault is trying to exit its position, the exact opposite will happen. The vault will not have any problem exiting from the Boosted Stable USD Pool because the code has ensured that the vault will never hold too large of a share of the liquidity within the Boosted Stable USD Pool.

However, the problem is that when exiting the Linear USDC Pool, there might not be sufficient liquidity within the Linear USDC Pool for the vault to exit its bb-a-USDC BPT position. This is because the code did not check whether or not the vault was holding too large of a share of the liquidity within the Linear USDC Pool. Thus, the vault will be illiquid and cannot exit its position.

flowchart LR;
    A[Notional Boosted Leverage Vault]--USDC-->B[Linear USDC Pool]--bb-a-USDC-->C[Boosted Stable USD Pool]--Boosted BPT-->A;

The following code shows how the workflow for depositing/joining a Boosted Balancer Pool works.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L234

File: Boosted3TokenPoolUtils.sol
234:     function _joinPoolExactTokensIn(ThreeTokenPoolContext memory context, uint256 primaryAmount, uint256 minBPT)
235:         private returns (uint256 bptAmount) {
236:         IBoostedPool underlyingPool = IBoostedPool(address(context.basePool.primaryToken));
237: 
238:         // Swap underlyingToken for LinearPool BPT
239:         uint256 linearPoolBPT = BalancerUtils._swapGivenIn({
240:             poolId: underlyingPool.getPoolId(),
241:             tokenIn: underlyingPool.getMainToken(),
242:             tokenOut: address(underlyingPool),
243:             amountIn: primaryAmount,
244:             limit: 0 // slippage checked on the second swap
245:         });
246: 
247:         // Swap LinearPool BPT for Boosted BPT
248:         bptAmount = BalancerUtils._swapGivenIn({
249:             poolId: context.basePool.basePool.poolId,
250:             tokenIn: address(underlyingPool),
251:             tokenOut: address(context.basePool.basePool.pool), // Boosted pool
252:             amountIn: linearPoolBPT,
253:             limit: minBPT
254:         });
255:     }

Impact

The Boosted Balancer leverage vault will be illiquid and cannot exit its position.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L325 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L234

Tool used

Manual Review

Recommendation

For Boosted Balancer leverage vault, consider implementing a BPT threshold check on both the Linear USDC Pool and Boosted Stable USD Pool.

Implement the same for the DAI variant of the Boosted Balancer leverage vault.

jeffywu commented 1 year ago

@weitianjie2000

This seems like a good recommendation although I'm curious in practice how this might actually turn into an attack since you have to hold Linear BPT to get into the Boosted BPT pool.

kaiserpy commented 1 year ago

The BPT threshold check does not guarantee that the pool will be sufficiently liquid to redeem BPTs at an appropriate price. For example, the MetaStable2Token could hold 99% wstETH (i.e. be highly illiquid) without infringing on the BPT threshold check.

It is the case that when exiting the Boosted3Token vault, it might push the Linear USDC Pool price upward to a point where it causes an undue amount of slippage. To mitigate this, we will allow users to redeem their BPTs in a different currency than the borrow currency and use the trading module to swap to the borrowed currency.

As far as the liquidity of the Linear pool itself, a large redemption will increase the proportion of aUSDC in relation to USDC in the pool. If the Linear pool is out of range an arbitrageur can flash borrow USDC, swap USDC for aUSDC in the linear pool at a premium, repay the flash loan, and pocket the premium (see: https://github.com/balancer-labs/balancer-v2-monorepo/blob/927a45424d74c43db26d83202e43a18fb13b5620/pkg/pool-linear/contracts/LinearPool.sol#L33-L52 ).

Implementing a check on the liquidity of the Linear Pool (i.e. the proportion of USDC vs aUSDC) is unnecessary as the pool already offers arbitrageurs an incentive to rebalance the pool within a predetermined range.

@T-Woodward

Evert0x commented 1 year ago

Based on the comments I will close this one @kaiserpy @jeffywu