sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

xiaoming90 - Attackers Can DOS Balancer Vaults By Bypassing The BPT Threshold #66

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

high

Attackers Can DOS Balancer Vaults By Bypassing The BPT Threshold

Summary

Malicious users can lock up all the leverage vaults offered by Notional causing denial-of-service by bypassing the BPT threshold and subseqently trigger an emergency settlement against the vaults.

Vulnerability Detail

The current BPT threshold is set to 20% of the total BTP supply based on the environment file provided during the audit.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/scripts/BalancerEnvironment.py#L41

File: BalancerEnvironment.py
40:             "oracleWindowInSeconds": 3600,
41:             "maxBalancerPoolShare": 2e3, # 20%
42:             "settlementSlippageLimitPercent": 5e6, # 5%

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

File: BalancerVaultStorage.sol
60:     function _bptThreshold(StrategyVaultSettings memory strategyVaultSettings, uint256 totalBPTSupply) 
61:         internal pure returns (uint256) {
62:         return (totalBPTSupply * strategyVaultSettings.maxBalancerPoolShare) / BalancerConstants.VAULT_PERCENT_BASIS;
63:     }

When the total number of BPT owned by the vault exceeds the BPT threshold, no one will be able to enter the vault as per the require check at Line 295-296 within the TwoTokenPoolUtils._joinPoolAndStake function.

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

File: TwoTokenPoolUtils.sol
268:     function _joinPoolAndStake(
269:         TwoTokenPoolContext memory poolContext,
270:         StrategyContext memory strategyContext,
271:         AuraStakingContext memory stakingContext,
272:         uint256 primaryAmount,
273:         uint256 secondaryAmount,
274:         uint256 minBPT
275:     ) internal returns (uint256 bptMinted) {
276:         // prettier-ignore
277:         PoolParams memory poolParams = poolContext._getPoolParams( 
278:             primaryAmount, 
279:             secondaryAmount,
280:             true // isJoin
281:         );
282: 
283:         bptMinted = BalancerUtils._joinPoolExactTokensIn({
284:             context: poolContext.basePool,
285:             params: poolParams,
286:             minBPT: minBPT
287:         });
288: 
289:         // Check BPT threshold to make sure our share of the pool is
290:         // below maxBalancerPoolShare
291:         uint256 bptThreshold = strategyContext.vaultSettings._bptThreshold(
292:             poolContext.basePool.pool.totalSupply()
293:         );
294:         uint256 bptHeldAfterJoin = strategyContext.totalBPTHeld + bptMinted;
295:         if (bptHeldAfterJoin > bptThreshold)
296:             revert Errors.BalancerPoolShareTooHigh(bptHeldAfterJoin, bptThreshold);
297: 
298:         // Transfer token to Aura protocol for boosted staking
299:         stakingContext.auraBooster.deposit(stakingContext.auraPoolId, bptMinted, true); // stake = true
300:     }

Another key point that is critical for this issue is that when the total number of BPT owned by the vault exceeds the BPT threshold, an emergency settlement can be triggered against the vault and anyone can triggered it as it is permissionless. A major side-effect of an emergency settlement is that the vault will be locked up after the emergency settlement. No one is allowed to enter the vault and users are only allowed to exit from the vault by taking their proportional share of cash and strategy tokens. The reason is that after the emergency settlement, there will be some asset cash balance in the vault and this will cause no one to be able to enter the vault due to the require check at Line 218. This side-effect has been verified by reviewing the codebase and clarifying with the sponsors.

https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultState.sol#L207

File: VaultState.sol
207:     function enterMaturity(
208:         VaultState memory vaultState,
209:         VaultAccount memory vaultAccount,
210:         VaultConfig memory vaultConfig,
211:         uint256 strategyTokenDeposit,
212:         uint256 additionalUnderlyingExternal,
213:         bytes calldata vaultData
214:     ) internal returns (uint256 strategyTokensAdded) {
215:         // If the vault state is holding asset cash this would mean that there is some sort of emergency de-risking
216:         // event or the vault is in the process of settling debts. In both cases, we do not allow accounts to enter
217:         // the vault.
218:         require(vaultState.totalAssetCash == 0);

If an attacker could force an emergency settlement on a vault anytime, he would be able to perform a DOS on the vault since the vault will basically be locked up after it. The following demonstrates how this can be performed:

1) Assume that the total supply of BTP in the WETH/stETH Balancer Pool is 100,000 Therefore, the BPT threshold of the vault will be 20,000. 2) Assume that the total number of BPT held by the vault is 19,900. 3) Note that under normal circumstances, it is not possible for the users to exceed the BPT threshold because the transaction will revert if the bptHeldAfterJoin > bptThreshold after the user enters the vault. 4) Note that at this point, the emergency settlement CANNOT be triggered against the vault because the vault has not exceeded BPT threshold yet 5) Bob (attacker) flash-loans a large amount of ETH from dydx where the fee is almost non-existence (1 Wei Only) 6) Bob allocates a portion of his ETH to join the WETH/stETH Balancer Pool. This will cause the total supply of BPT to increase significantly to 200,000. 7) Bob allocates a portion of his ETH to enter the vault and causes the total number of BPT held by the vault to increase by 150 from 19,900 to 20,050. This is allowed because the total supply of BPT has increased to 200,000, and thus the BPT threshold has increased to 40,000. Also, Bob does not leverage himself and does not borrow from Notional since the flash loan already provided him with access to a large number of funds, and thus he does not need to pay for any borrowing cost to minimize the cost of this attack. 8) At this point, due to the inflow of 150 BPT to the Balancer Pool, the total supply of BPT increase from 200,000 to 200,150. 9) After entering the vault, Bob exits the WETH/stETH Balancer Pool entirely with all his 100,000 BPT position. This will cause the total supply of BPT to fall back to 100,150. Per my research, there is no exit fee when a Liquidity Provider exits a pool. Also, a Liquidity Provider only suffers a loss due to impermanent loss. However, since all these steps are executed within the same transaction, there is no impermanent loss because no one perform any token swap. Thus, there is no cost incurred by Bob for this step. 10) Note that at this point, the emergency settlement CAN be triggered against the vault because the vault has exceeded the BPT threshold. The total number of BPT held by the vault is 20,050, and the BPT threshold is 20,030 (=100,150 * 0.2). 11) Anyone can trigger the emergency settlement as it is permissionless. Bob triggered an emergency settlement against the vault, and 20 BPT will be sold off in the market so that the vault will not exceed the BPT threshold. It is important to ensure that the number of BPTs to be sold is kept as low as possible so that the total value of the vault will not be reduced by slippage during the trade. This is because Bob still owns the shares of the vault and he wants to get back as much of his original deposit as possible later. This value can be optimized further with Math. 12) As mentioned earlier, after an emergency settlement, the vault will be locked up. No one is allowed to enter the vault and users are only allowed to exit from the vault by taking their proportional share of cash and strategy tokens. 13) Bob proceeds to redeem all his shares from the vault. He will get back all of his deposits minus the 20 BPT slippage loss during the emergency settlement that is split proportionally among all vault shareholders which is insignificant. Note that the Notional's leverage vault does not impose any exit fee. 14) Bob proceeds to repay back his loan and pay 1 wei as the fee to dydx. 15) The cost of attack is 1 wei (flash-loan fee) + 20 BPT slippage loss during the emergency settlement that is split proportionally among all vault shareholders, which is insignificant. The slippage loss during emergency settlement can be minimized by causing the total number of BPT held by the vault to exceed the BPT threshold by the smallest possible value. 16) All the above steps will be executed within a single block/transaction.

Impact

Malicious users can lock up all the leverage vaults offered by Notional causing denial-of-service. This results in a loss of funds for the protocol as the vault is no longer generating profit for the protocol, and also a loss of funds for vault users as they cannot realize the profits because they are forced to exit the vault prematurely.

The following are various reasons why someone would want to perform a DOS on Notional vaults:

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/scripts/BalancerEnvironment.py#L41 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/BalancerVaultStorage.sol#L60 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/TwoTokenPoolUtils.sol#L268 https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultState.sol#L207

Tool used

Manual Review

Recommendation

Short term, consider the following measures to mitigate the issue:

Long term, update the implementation of the vault so that the vault will not be locked up after an emergency settlement. After selling off the excess BPT, the vault should allow users to enter the vault as per normal.

jeffywu commented 1 year ago

This is an interesting attack, we will think about the recommendations a bit. Since there is no loss of funds I'm not sure if this should be categorized as high. If this did happen, the most likely thing we would do is to upgrade the vault such that it could re-deposit its assets back into the Balancer vault which would mitigate the effects of the DOS altogether.

T-Woodward commented 1 year ago

Yeah I agree with Jeff that this shouldn't be high severity because it's really more a way that an unscrupulous competitor could sabotage Notional, it wouldn't threaten user funds. I would call it medium.

We are making some changes here:

  1. We're permissioning the emergency settle function.
  2. We're implementing a minimum leverage ratio so that you can't enter/exit without paying the fees associated with borrowing and lending on Notional.

That should do it

jeffywu commented 1 year ago

Minimum Leverage Ratio (i.e. maxAccountRequiredCollateralRatio) fixed here: https://github.com/notional-finance/contracts-v2/pull/104/commits/0dbaac7bc559d04e1006cfbe12d5e6d73e82e306

rayn731 commented 1 year ago

The fix seems good.

Too prevent free riding, Notional add maxRequiredAccountCollateralRatioBPS in VaultConfig. When doing checkCollateralRatio, it would check whether collateralRatio <= vaultConfig.maxRequiredAccountCollateralRatio if vaultAccount.vaultShares > 0. An attacker's collateralRatio is usually type(int256).max due to debtOutstanding == 0(So he/she don't need to pay the fees associated with borrowing and lending on Notional). Therefore the attacker couldn't pass the check in checkCollateralRatio when he/she tries to exit the vault. In conclusion, the attack described in this issue is blocked.