xiaoming90 - Gain From Balancer Vaults Can Be Stolen

[sherlock-admin]

xiaoming90

high

Gain From Balancer Vaults Can Be Stolen

Summary

The BPT gain (rewards) of the vault can be stolen by an attacker.

Vulnerability Detail

At T0 (Time 0), assume that the state of the WETH/wstETH MetaPool Vault is as follows:

• totalBPTHeld = 1000 BPT
• totalStrategyTokenGlobal = 1000
• 1 Strategy Token can claim 1 BPT
• Alice holds 1000 Strategy Tokens, and she is the only person invested in the vault at this point in time

Assume that if the reinvestReward is called, it will reinvest 1000 BPT back into the vault. Thus, if the reinvestReward is called, the totalBPTHeld of the vault will become 2000 BPT.

Following is the description of the attack:

1. The attacker notice that if the reinvestReward is called, it will result in a large increase in the total BPT held by the vault

2. The attacker flash-loan a large amount of WETH (e.g. 1,000,000) from a lending protocol (e.g. dydx)

3. Enter the vault by depositing 1,000,000 WETH by calling the VaultAccountAction.enterVault function. However, do not borrow any cash from Notional by setting the fCash parameter of the VaultAccountAction.enterVault function to 0.

4. There is no need to borrow from Notional as the attacker could already flash-loan a large amount of WETH with a non-existence fee rate (e.g. 1 Wei in dydx). Most importantly, the vault fee will only be charged if the user borrows from Notional. The fee is assessed within the VaultAccount._borrowIntoVault, which will be skipped if users are not borrowing. By not borrowing from Notional, the attacker does not need to pay any fee when entering the vault and this will make the attacker more profitable.

5. The vault will deposit 1,000,000 WETH to the Balancer pool and receive a large amount of BPT in return. For simplicity's sake, assume that the vault receives 1,000,000 BPT in return.

6. Based on the StrategyUtils._convertBPTClaimToStrategyTokens function, the attacker will receive 100,000 strategy tokens. The state of the vault will be as follows after the attacker deposits:

   • totalBPTHeld = 1,001,000 BPT

   • totalStrategyTokenGlobal = 1,001,000

   • 1 Strategy Token can claim 1 BPT

   • Alice holds 1000 Strategy Tokens

   • Attacker holds 1,000,000 Strategy Tokens

7. The attacker calls the reinvestReward function, and reward tokens will be reinvested. Assume that the vault receives 1000 BPT. The state of the vault will be as follows after the reinvest:

   • totalBPTHeld = 1,002,000 BPT

   • totalStrategyTokenGlobal = 1,001,000

   • 1 Strategy Token can claim ~1.0009 BPT

   • Alice holds 1000 Strategy Tokens

   • Attacker holds 1,000,000 Strategy Tokens

8. The attacker exits the vault with all his strategy tokens by calling the VaultAccountAction.exitVault function. This will cause the vault the redeem all the 100,000 Strategy Tokens owned by the attacker. Based on the StrategyUtils._convertStrategyTokensToBPTClaim function, the attacker will receive 1,000,999 BPT in return. Note that there is no fee for exiting the vault and there is no need for repaying the debt as the attacker did not borrow any assets from Notional at the beginning.

bptClaim = (strategyTokenAmount * context.totalBPTHeld) / context.vaultState.totalStrategyTokenGlobal;
1,000,999 = (1000000 * 1002000) / 1001000

9. Proceed to repay the flash-loan at the end of the transaction. All the above steps are executed within a single transaction. Within a single transaction/block, the attacker is able to increase his holding of 1,000,000 BPT to 1,000,999 BPT after calling the reinvestReward function, and effectively gain around 999 BPT.
10. Alice who had been invested in the vault since the vault was first launched should be entitled to the majority of the rewards (Close to 1000 BPT). However, the attacker who came in right before the reinvestReward function was triggered managed to obtain almost all of her allocated shares of rewards (999 BPT) and left only 1 BPT for Alice.

Note: A flash-loan is not required if the attacker has sufficient liquidity to carry out the attack or the vault does not have much liquidity.

Following are the two functions for converting between BPT and Strategy Token for reference.

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

/// @notice Converts BPT to strategy tokens
function _convertBPTClaimToStrategyTokens(StrategyContext memory context, uint256 bptClaim)
    internal pure returns (uint256 strategyTokenAmount) {
    if (context.totalBPTHeld == 0) {
        // Strategy tokens are in 8 decimal precision, BPT is in 18. Scale the minted amount down.
        return (bptClaim * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / 
            BalancerConstants.BALANCER_PRECISION;
    }

    // BPT held in maturity is calculated before the new BPT tokens are minted, so this calculation
    // is the tokens minted that will give the account a corresponding share of the new bpt balance held.
    // The precision here will be the same as strategy token supply.
    strategyTokenAmount = (bptClaim * context.vaultState.totalStrategyTokenGlobal) / context.totalBPTHeld;
}

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

/// @notice Converts strategy tokens to BPT
function _convertStrategyTokensToBPTClaim(StrategyContext memory context, uint256 strategyTokenAmount)
    internal pure returns (uint256 bptClaim) {
    require(strategyTokenAmount <= context.vaultState.totalStrategyTokenGlobal);
    if (context.vaultState.totalStrategyTokenGlobal > 0) {
        bptClaim = (strategyTokenAmount * context.totalBPTHeld) / context.vaultState.totalStrategyTokenGlobal;
    }
}

Impact

Loss of assets for the users as their BPT gain (rewards) can be stolen. This issue affects all balancer-related vaults that contain the permissionless reinvestReward function.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/strategy/StrategyUtils.sol#L27 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/strategy/StrategyUtils.sol#L18

Tool used

Manual Review

Recommendation

Following are the list of root causes of the issue and some recommendation to mitigate them.

• reinvestReward function is permissionless and can be called by anyone. It is recommended to implement access control to ensure that this function can only be triggered by Notional. Do note that even if the attacker cannot trigger the reinvestReward function, it is still possible for the attacker to front-run and back-end the reinvestReward transaction to carry out the attack if they see this transaction in the public mempool. Thus, consider sending the reinvestReward transaction as a private transaction via Flashbot so that the attacker cannot sandwich the transaction.
• There is no withdrawal fee. Also, there is no deposit fee as long as users did not borrow from Notional. Therefore, this attack is mostly profitable. It is recommended to impose a fee on the users of the vault even if the users did not borrow from Notional. All users should be charged a fee for the use of the vault. This will make the attack less likely to be profitable in most cases.
• Users can enter and exit the vault within the same transaction/block. This allows the attacker to leverage the flash-loan facility to reduce the cost of the attack to almost nothing. It is recommended to prevent users from entering and exiting the vault within the same transaction/block. If the user entered the vault in this block, he/she could only exit at the next block.
• There is no snapshotting to keep track of the deposit to ensure that BPT gain/rewards distributions are weighted according to deposit duration. Thus, a whale could deposit right before the reinvestReward function is triggered and exit the vault afterward and reap most of the gains. Consider implementing snapshotting within the vault.

[jeffywu]

@T-Woodward / @weitianjie2000 we should discuss how to remediate this issue. I think the auditor has a good point about enter / exits within the same block that we should take a look at.

At the same time, I believe this attack is more pronounced when the attacker can get much higher leverage than the entire vault value (as in this example), so in practice it might be difficult.

Note to self: it looks like a more strict enforcement of the minAccountBorrowSize would be sufficient to reduce the profitability of these attacks by forcing the account to borrow, will have to investigate how to do that without hampering other UX.

[T-Woodward]

Yeah I think this is a legitimate issue. We are implementing the following changes:

1. We're permissioning the reinvestReward function.
2. We're adding a five block minimum holding period (you can't exit the vault until five blocks after you last entered). This means you can't use flash loans and actually have to have the capital.
3. We're adding minimum leverage ratios which will force you to borrow on entry and lend on exit and pay the fees associated with doing so. Additionally, there are transaction costs associated with entering and exiting the vault apart from the lending/borrowing fee on Notional.

Together, these changes will make the attack uneconomical because of the fees involved, it will require substantial capital, and you wouldn't know when we are going to call reinvestReward so you would have to basically always have your capital in to make sure you caught the reward reinvestment which would defeat the purpose of the whole thing.

[jeffywu]

Minimum Leverage Ratio here: https://github.com/notional-finance/contracts-v2/pull/104/commits/0dbaac7bc559d04e1006cfbe12d5e6d73e82e306 5 block minimum hold time here: https://github.com/notional-finance/contracts-v2/pull/104/commits/bf98affbf49e2b15f6cd0714dade9f1c138df7db

[rayn731]

The issue is fixed.

reinvestReward is not permissionless now. https://github.com/notional-finance/leveraged-vaults/commit/cc0401752f5c8b5be9a7dd5e48c249cb16018d87

And this fix(https://github.com/notional-finance/contracts-v2/commit/0dbaac7bc559d04e1006cfbe12d5e6d73e82e306) makes the flash-loan attack uneconomical.