Vault Share/Strategy Token Calculation Can Be Broken By First User/Attacker
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.\\nNote: This issue affects MetaStable2 and Boosted3 balancer leverage vaults\\nFor 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.\\n```\\nstrategyToken = (totalBPTHeld == 0) ?  bptClaim : (bptClaim * totalStrategyToken) / totalBPTHeld\\n```\\n\\nThe vault minting formula is taken from the following:\\n```\\nFile: StrategyUtils.sol\\n    /// @notice Converts BPT to strategy tokens\\n    function _convertBPTClaimToStrategyTokens(StrategyContext memory context, uint256 bptClaim)\\n        internal pure returns (uint256 strategyTokenAmount) {\\n        if (context.totalBPTHeld == 0) {\\n            // Strategy tokens are in 8 decimal precision, BPT is in 18. Scale the minted amount down.\\n            return (bptClaim * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / \\n                BalancerConstants.BALANCER_PRECISION;\\n        }\\n\\n        // BPT held in maturity is calculated before the new BPT tokens are minted, so this calculation\\n        // is the tokens minted that will give the account a corresponding share of the new bpt balance held.\\n        // The precision here will be the same as strategy token supply.\\n        strategyTokenAmount = (bptClaim * context.vaultState.totalStrategyTokenGlobal) / context.totalBPTHeld;\\n    }\\n```\\n\\nIf 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`.\\nThe 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.\\nTwo issues can occur from here.\\nIssue 1 - If bptClaim >= totalBPTHeld\\nThe 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.\\n```\\nstrategyToken = (bptClaim * totalStrategyToken) / totalBPTHeld\\nstrategyToken = (19999 * 1) / 10000 = 1\\n```\\n\\nIf the attacker exits the vault right after Alice's deposit, the attacker will receive 14999 BPT. He profited 4999 BPT from this attack\\n```\\nbptReceived = (strategyToken * totalBPTHeld) / totalStrategyToken\\nbptReceived = (1 * 29999) / 2 = 14999\\n```\\n\\nIssue 2 - If bptClaim < totalBPTHeld\\nThe following describes a scenario in which a user's assets are lost entirely. Assume that Alice deposits/borrow some assets and received 9999 BPT\\n```\\nstrategyToken = (bptClaim * totalStrategyToken) / totalBPTHeld\\nstrategyToken = (9999  * 1) / 10000 = 0\\n```\\n\\nAs such, she deposited 9999 BPT but did not receive any strategy tokens in return.\\nHow to increase the total BPT held?\\nUnlike 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.\\nHowever, there is a workaround to increase the total BPT held by the vault, and this can be executed by anyone.\\nThe `totalBPTHeld` within the vault is obtained by calling the `PoolMixin._bptHeld` function.\\n```\\nFile: PoolMixin.sol\\n    function _baseStrategyContext() internal view returns(StrategyContext memory) {\\n        return StrategyContext({\\n            totalBPTHeld: _bptHeld(),\\n            settlementPeriodInSeconds: SETTLEMENT_PERIOD_IN_SECONDS,\\n            tradingModule: TRADING_MODULE,\\n            vaultSettings: BalancerVaultStorage.getStrategyVaultSettings(),\\n            vaultState: BalancerVaultStorage.getStrategyVaultState(),\\n            feeReceiver: FEE_RECEIVER\\n        });\\n    }\\n```\\n\\nWithin 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.\\n```\\nFile: PoolMixin.sol\\n    /// @dev Gets the total BPT held by the aura reward pool\\n    function _bptHeld() internal view returns (uint256) {\\n        return AURA_REWARD_POOL.balanceOf(address(this));\\n    }\\n```\\n\\nThe 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.\\n```\\nFile: BaseRewardPool4626.sol\\n/**\\n * @dev Returns the amount of tokens owned by `account`.\\n */\\nfunction balanceOf(address account) public view override(BaseRewardPool, IERC20) returns (uint256) {\\n    return BaseRewardPool.balanceOf(account);\\n}\\n```\\n\\n```\\nFile: BaseRewardPool.sol\\nfunction balanceOf(address account) public view virtual returns (uint256) {\\n return _balances[account];\\n}\\n```\\n\\nTo 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.\\nHowever, 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.\\n```\\nFile: BaseRewardPool4626.sol\\n/**\\n * @notice Mints `shares` Vault shares to `receiver`.\\n * @dev Because `asset` is not actually what is collected here, first wrap to required token in the booster.\\n */\\nfunction deposit(uint256 assets, address receiver) public virtual override nonReentrant returns (uint256) {\\n    // Transfer "asset" (crvLP) from sender\\n    IERC20(asset).safeTransferFrom(msg.sender, address(this), assets);\\n\\n    // Convert crvLP to cvxLP through normal booster deposit process, but don't stake\\n    uint256 balBefore = stakingToken.balanceOf(address(this));\\n    IDeposit(operator).deposit(pid, assets, false);\\n    uint256 balAfter = stakingToken.balanceOf(address(this));\\n\\n    require(balAfter.sub(balBefore) >= assets, "!deposit");\\n\\n    // Perform stake manually, now that the funds have been received\\n    _processStake(assets, receiver);\\n\\n    emit Deposit(msg.sender, receiver, assets, assets);\\n    emit Staked(receiver, assets);\\n    return assets;\\n}\\n```\\n\\n```\\nFile: BaseRewardPool.sol \\n/**\\n* @dev Generic internal staking function that basically does 3 things: update rewards based\\n*      on previous balance, trigger also on any child contracts, then update balances.\\n* @param _amount    Units to add to the users balance\\n* @param _receiver  Address of user who will receive the stake\\n*/\\nfunction _processStake(uint256 _amount, address _receiver) internal updateReward(_receiver) {\\n    require(_amount > 0, 'RewardPool : Cannot stake 0');\\n\\n    //also stake to linked rewards\\n    for(uint i=0; i < extraRewards.length; i++){\\n    IRewards(extraRewards[i]).stake(_receiver, _amount);\\n    }\\n\\n    _totalSupply = _totalSupply.add(_amount);\\n    _balances[_receiver] = _balances[_receiver].add(_amount);\\n}\\n```\\n
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.\\nReference\\nA similar issue was found in a past Sherlock audit
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.
```\\nstrategyToken = (totalBPTHeld == 0) ?  bptClaim : (bptClaim * totalStrategyToken) / totalBPTHeld\\n```\\n