xiaoming90 - Users deposit assets to the vault but receives no strategy token in return

[github-actions]

xiaoming90

high

Users deposit assets to the vault but receives no strategy token in return

Summary

Due to a rounding error in Solidity, it is possible that a user deposits assets to the vault, but receives no strategy token in return due to issues in the following functions:

• StrategyUtils._convertBPTClaimToStrategyTokens
• Boosted3TokenPoolUtils._deposit
• TwoTokenPoolUtils._deposit

Vulnerability Detail

This affects both the TwoToken and Boosted3Token vaults

int256 internal constant INTERNAL_TOKEN_PRECISION = 1e8;
uint256 internal constant BALANCER_PRECISION = 1e18;

Within the StrategyUtils._convertBPTClaimToStrategyTokens function, it was observed that the numerator precision (1e8) is much smaller than the denominator precision (1e18).

https://github.com/sherlock-audit/2022-12-notional/blob/main/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.vaultState.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.vaultState.totalBPTHeld;
39:     }

As a result, the StrategyUtils._convertBPTClaimToStrategyTokens function might return zero strategy tokens under the following two conditions:

If the totalBPTHeld is zero (First Deposit)

If the totalBPTHeld is zero, the code at Line 31 will be executed, and the following formula is used:

strategyTokenAmount = (bptClaim * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / BalancerConstants.BALANCER_PRECISION;
strategyTokenAmount = (bptClaim * 1e8) / 1e18
strategyTokenAmount = ((10 ** 10 - 1) * 1e8) / 1e18 = 0

During the first deposit, if the user deposits less than 1e10 BPT, Solidity will round down and strategyTokenAmount will be zero.

If the totalBPTHeld is larger than zero (Subsequently Deposits)

If the totalBPTHeld is larger than zero, the code at Line 38 will be executed, and the following formula is used:

strategyTokenAmount = (bptClaim * context.vaultState.totalStrategyTokenGlobal) / context.vaultState.totalBPTHeld;
strategyTokenAmount = (bptClaim * (x * 1e8))/ (y * 1e18)

If the numerator is less than the denominator, the strategyTokenAmount will be zero.

Therefore, it is possible that the users deposited their minted BPT to the vault, but received zero strategy tokens in return.

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

File: Boosted3TokenPoolUtils.sol
408:     function _deposit(
409:         ThreeTokenPoolContext memory poolContext,
410:         StrategyContext memory strategyContext,
411:         AuraStakingContext memory stakingContext,
412:         BoostedOracleContext memory oracleContext,
413:         uint256 deposit,
414:         uint256 minBPT
415:     ) internal returns (uint256 strategyTokensMinted) {
416:         uint256 bptMinted = poolContext._joinPoolAndStake({
417:             strategyContext: strategyContext,
418:             stakingContext: stakingContext,
419:             oracleContext: oracleContext,
420:             deposit: deposit,
421:             minBPT: minBPT
422:         });
423: 
424:         strategyTokensMinted = strategyContext._convertBPTClaimToStrategyTokens(bptMinted);
425: 
426:         strategyContext.vaultState.totalBPTHeld += bptMinted;
427:         // Update global supply count
428:         strategyContext.vaultState.totalStrategyTokenGlobal += strategyTokensMinted.toUint80();
429:         strategyContext.vaultState.setStrategyVaultState(); 
430:     }

Proof-of-Concept

Assume that Alice is the first depositor, and she forwarded 10000 BPT. During the first mint, the strategy token will be minted in a 1:1 ratio. Therefore, Alice will receive 10000 strategy tokens in return. At this point in time, totalStrategyTokenGlobal = 10000 strategy tokens and totalBPTHeld is 10000 BPT.

When Bob deposits to the vault after Alice, he will be subjected to the following formula:

strategyTokenAmount = (bptClaim * context.vaultState.totalStrategyTokenGlobal) / context.vaultState.totalBPTHeld;
strategyTokenAmount = (bptClaim * (10000 * 1e8))/ (10000 * 1e18)
strategyTokenAmount = (bptClaim * (1e12))/ (1e22)

If Bob deposits less than 1e10 BPT, Solidity will round down and strategyTokenAmount will be zero. Bob will receive no strategy token in return for his BPT.

Another side effect of this issue is that if Alice withdraws all her strategy tokens, she will get back all her 10000 BPT plus the BPT that Bob deposited earlier.

Impact

Loss of assets for the users as they deposited their assets but receive zero strategy tokens in return.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Consider reverting if zero strategy token is minted. This check has been implemented in many well-known vault designs as this is a commonly known issue (e.g. Solmate)

function _deposit(
    ThreeTokenPoolContext memory poolContext,
    StrategyContext memory strategyContext,
    AuraStakingContext memory stakingContext,
    BoostedOracleContext memory oracleContext,
    uint256 deposit,
    uint256 minBPT
) internal returns (uint256 strategyTokensMinted) {
    uint256 bptMinted = poolContext._joinPoolAndStake({
        strategyContext: strategyContext,
        stakingContext: stakingContext,
        oracleContext: oracleContext,
        deposit: deposit,
        minBPT: minBPT
    });

    strategyTokensMinted = strategyContext._convertBPTClaimToStrategyTokens(bptMinted);
+	require(strategyTokensMinted != 0, "zero strategy token minted");    

    strategyContext.vaultState.totalBPTHeld += bptMinted;
    // Update global supply count
    strategyContext.vaultState.totalStrategyTokenGlobal += strategyTokensMinted.toUint80();
    strategyContext.vaultState.setStrategyVaultState(); 
}

[weitianjie2000]

valid, will fix