xiaoming90

high

Vault Share/Strategy Token Calculation Can Be Broken By First User/Attacker

Summary

A well-known attack vector for almost all shares-based liquidity pool contracts, where an early user can manipulate the price per share and profit from late users' deposits because of the precision loss caused by the rather large value of price per share.

Vulnerability Detail

Note: This issue affects MetaStable2 and Boosted3 balancer leverage vaults

For simplicity's sake, we will simplify the strategy token minting formula as follows. Also, assume that the 1 vault share is equivalent to 1 strategy token for this particular strategy vault, therefore, we will use the term vault share and strategy token interchangeably here.

strategyToken = (totalBPTHeld == 0) ?  bptClaim : (bptClaim * totalStrategyToken) / totalBPTHeld

The vault minting formula is taken from the following:

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

File: StrategyUtils.sol
26:     /// @notice Converts BPT to strategy tokens
27:     function _convertBPTClaimToStrategyTokens(StrategyContext memory context, uint256 bptClaim)
28:         internal pure returns (uint256 strategyTokenAmount) {
29:         if (context.totalBPTHeld == 0) {
30:             // Strategy tokens are in 8 decimal precision, BPT is in 18. Scale the minted amount down.
31:             return (bptClaim * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / 
32:                 BalancerConstants.BALANCER_PRECISION;
33:         }
34: 
35:         // BPT held in maturity is calculated before the new BPT tokens are minted, so this calculation
36:         // is the tokens minted that will give the account a corresponding share of the new bpt balance held.
37:         // The precision here will be the same as strategy token supply.
38:         strategyTokenAmount = (bptClaim * context.vaultState.totalStrategyTokenGlobal) / context.totalBPTHeld;
39:     }

If the attacker who is the first depositor claims 1 BPT, he will receive 1 Strategy Token. So 1 BPT per Strategy Token. At this point in time, totalBPTHeld = 1 and totalStrategyToken = 1.

The attacker obtains 9999 BPT can be obtained from the open market. He proceeds to deposit the 9999 BPT into the Aura reward pool on behalf of the vault. At this point in time, totalBPTHeld = 10000 and totalStrategyToken = 1. So 10000 BPT per Strategy Token. Refer to the "How to increase the total BPT held?" section below for more details.

Two issues can occur from here.

Issue 1 - If bptClaim >= totalBPTHeld

The following describes a scenario in which a user's assets are lost and stolen by an attacker. Assume that Alice deposits/borrow some assets and received 19999 BPT. Based on the formula, Alice will only receive 1 Strategy Token. She immediately loses 9999 BPT or half of her assets if she exits the vault or redeems the strategy tokens right after the deposit.

strategyToken = (bptClaim * totalStrategyToken) / totalBPTHeld
strategyToken = (19999 * 1) / 10000 = 1

If the attacker exits the vault right after Alice's deposit, the attacker will receive 14999 BPT. He profited 4999 BPT from this attack

bptReceived = (strategyToken * totalBPTHeld) / totalStrategyToken
bptReceived = (1 * 29999) / 2 = 14999

Issue 2 - If bptClaim < totalBPTHeld

The following describes a scenario in which a user's assets are lost entirely. Assume that Alice deposits/borrow some assets and received 9999 BPT

strategyToken = (bptClaim * totalStrategyToken) / totalBPTHeld
strategyToken = (9999  * 1) / 10000 = 0

As such, she deposited 9999 BPT but did not receive any strategy tokens in return.

How to increase the total BPT held?

Unlike the vault design seen in other protocols, Notional's leverage vault does not compute the total BPT held by the vault directly via BTP.balanceOf(address(vault)). The vault deposit its BPT to the Aura Reward Pool. Therefore, it is not possible to increase the total BPT held by the vault simply by performing a direct BPT token transfer to the vault or Aura Reward Pool in an attempt to increase it.

However, there is a workaround to increase the total BPT held by the vault, and this can be executed by anyone.

The totalBPTHeld within the vault is obtained by calling the PoolMixin._bptHeld function.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/PoolMixin.sol#L41

File: PoolMixin.sol
41:     function _baseStrategyContext() internal view returns(StrategyContext memory) {
42:         return StrategyContext({
43:             totalBPTHeld: _bptHeld(),
44:             settlementPeriodInSeconds: SETTLEMENT_PERIOD_IN_SECONDS,
45:             tradingModule: TRADING_MODULE,
46:             vaultSettings: BalancerVaultStorage.getStrategyVaultSettings(),
47:             vaultState: BalancerVaultStorage.getStrategyVaultState(),
48:             feeReceiver: FEE_RECEIVER
49:         });
50:     }

Within the PoolMixin._bptHeld function, it will call the AURA_REWARD_POOL.balanceOf(address(this)) to retrieve the number of BPT that the vault has deposited into the Aura Reward Pool.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/PoolMixin.sol#L37

File: PoolMixin.sol
36:     /// @dev Gets the total BPT held by the aura reward pool
37:     function _bptHeld() internal view returns (uint256) {
38:         return AURA_REWARD_POOL.balanceOf(address(this));
39:     }

The following is the contract of the AURA_REWARD_POOL taken from the Etherscan. Note that the AURA_REWARD_POOL.balanceOf will retrieve the number of BPT tokens held by an account. In this example, the account will be the vault's address.

https://etherscan.io/address/0xdcee1c640cc270121faf145f231fd8ff1d8d5cd4#code

File: BaseRewardPool4626.sol
/**
 * @dev Returns the amount of tokens owned by `account`.
 */
function balanceOf(address account) public view override(BaseRewardPool, IERC20) returns (uint256) {
    return BaseRewardPool.balanceOf(account);
}

File: BaseRewardPool.sol
function balanceOf(address account) public view virtual returns (uint256) {
    return _balances[account];
}

To increase the balance, the deposit(uint256 _pid, uint256 _amount, bool _stake) function of Aura's Booster contract can be called. However, the problem is that this function will deposit to the msg.sender and there is no way to spoof the vault's address. Thus, using this function will not work.

However, there is a second method that can be used to perform a deposit. The AURA_REWARD_POOL point to the BaseRewardPool4626, thus the reward pool is an ERC4626 vault. The Aura's ERC4626 vault supports an alternative deposit function called BaseRewardPool4626.deposit that allows anyone to deposit on behalf of another account. An attacker can leverage the BaseRewardPool4626.deposit function by specifying the receiver parameter to be the vault.address in an attempt to increase the total BPT tokens held by the vault.

https://etherscan.io/address/0xdcee1c640cc270121faf145f231fd8ff1d8d5cd4#code

File: BaseRewardPool4626.sol
/**
 * @notice Mints `shares` Vault shares to `receiver`.
 * @dev Because `asset` is not actually what is collected here, first wrap to required token in the booster.
 */
function deposit(uint256 assets, address receiver) public virtual override nonReentrant returns (uint256) {
    // Transfer "asset" (crvLP) from sender
    IERC20(asset).safeTransferFrom(msg.sender, address(this), assets);

    // Convert crvLP to cvxLP through normal booster deposit process, but don't stake
    uint256 balBefore = stakingToken.balanceOf(address(this));
    IDeposit(operator).deposit(pid, assets, false);
    uint256 balAfter = stakingToken.balanceOf(address(this));

    require(balAfter.sub(balBefore) >= assets, "!deposit");

    // Perform stake manually, now that the funds have been received
    _processStake(assets, receiver);

    emit Deposit(msg.sender, receiver, assets, assets);
    emit Staked(receiver, assets);
    return assets;
}

File: BaseRewardPool.sol 
/**
* @dev Generic internal staking function that basically does 3 things: update rewards based
*      on previous balance, trigger also on any child contracts, then update balances.
* @param _amount    Units to add to the users balance
* @param _receiver  Address of user who will receive the stake
*/
function _processStake(uint256 _amount, address _receiver) internal updateReward(_receiver) {
    require(_amount > 0, 'RewardPool : Cannot stake 0');

    //also stake to linked rewards
    for(uint i=0; i < extraRewards.length; i++){
    IRewards(extraRewards[i]).stake(_receiver, _amount);
    }

    _totalSupply = _totalSupply.add(_amount);
    _balances[_receiver] = _balances[_receiver].add(_amount);
}

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/mixins/PoolMixin.sol#L41 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/PoolMixin.sol#L37

Tool used

Manual Review

Impact

The attacker can profit from future users' deposits while the late users will lose part of their funds to the attacker. Additionally, it is also possible for users to get no share in return for their deposited funds.

Recommendation

Consider requiring a minimal amount of strategy tokens to be minted for the first minter, and send a portion of the initial mints as a reserve to the Notional Treasury so that the pricePerShare/pricePerStrategyToken can be more resistant to manipulation.

Reference

A similar issue was found in a past Sherlock audit

https://github.com/sherlock-audit/2022-08-sentiment-judging#issue-h-1-a-malicious-early-userattacker-can-manipulate-the-ltokens-pricepershare-to-take-an-unfair-share-of-future-users-deposits

sherlock-audit / 2022-09-notional-judging

xiaoming90 - Vault Share/Strategy Token Calculation Can Be Broken By First User/Attacker #70