Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

xiaoming90 - Two token vault will be broken if it comprises tokens with different decimals #18

Open
github-actions bot opened this issue Jan 13, 2023 · 1 comment
Labels
High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@github-actions
Copy link

xiaoming90

high

Two token vault will be broken if it comprises tokens with different decimals

Summary

A two token vault that comprises tokens with different decimals will have many of its key functions broken. For instance, rewards cannot be reinvested and vault cannot be settled.

Vulnerability Detail

The Stable2TokenOracleMath._getSpotPrice function is used to compute the spot price of two tokens.

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/math/Stable2TokenOracleMath.sol#L15

File: Stable2TokenOracleMath.sol
11: library Stable2TokenOracleMath {
12:     using TypeConvert for int256;
13:     using Stable2TokenOracleMath for StableOracleContext;
14: 
15:     function _getSpotPrice(
16:         StableOracleContext memory oracleContext, 
17:         TwoTokenPoolContext memory poolContext, 
18:         uint256 primaryBalance,
19:         uint256 secondaryBalance,
20:         uint256 tokenIndex
21:     ) internal view returns (uint256 spotPrice) {
22:         require(tokenIndex < 2); /// @dev invalid token index
23: 
24:         /// Apply scale factors
25:         uint256 scaledPrimaryBalance = primaryBalance * poolContext.primaryScaleFactor 
26:             / BalancerConstants.BALANCER_PRECISION;
27:         uint256 scaledSecondaryBalance = secondaryBalance * poolContext.secondaryScaleFactor 
28:             / BalancerConstants.BALANCER_PRECISION;
29: 
30:         /// @notice poolContext balances are always in BALANCER_PRECISION (1e18)
31:         (uint256 balanceX, uint256 balanceY) = tokenIndex == 0 ?
32:             (scaledPrimaryBalance, scaledSecondaryBalance) :
33:             (scaledSecondaryBalance, scaledPrimaryBalance);
34: 
35:         uint256 invariant = StableMath._calculateInvariant(
36:             oracleContext.ampParam, StableMath._balances(balanceX, balanceY), true // round up
37:         );
38: 
39:         spotPrice = StableMath._calcSpotPrice({
40:             amplificationParameter: oracleContext.ampParam,
41:             invariant: invariant,
42:             balanceX: balanceX, 
43:             balanceY: balanceY
44:         });
45: 
46:         /// Apply secondary scale factor in reverse
47:         uint256 scaleFactor = tokenIndex == 0 ?
48:             poolContext.secondaryScaleFactor * BalancerConstants.BALANCER_PRECISION / poolContext.primaryScaleFactor :
49:             poolContext.primaryScaleFactor * BalancerConstants.BALANCER_PRECISION / poolContext.secondaryScaleFactor;
50:         spotPrice = spotPrice * BalancerConstants.BALANCER_PRECISION / scaleFactor;
51:     }

Two tokens (USDC and DAI) with different decimals will be used below to illustrate the issue:

USDC/DAI Spot Price

Assume that the primary token is DAI (18 decimals) and the secondary token is USDC (6 decimals). As such, the scaling factors would be as follows. The token rate is ignored and set to 1 for simplicity.

  • Primary Token (DAI)'s scaling factor = 1e18

    scaling factor = FixedPoint.ONE (1e18) * decimals difference to reach 18 decimals (1e0) * token rate (1)
    scaling factor = 1e18
  • Secondary Token (USDC)'s scaling factor = 1e30

    scaling factor = FixedPoint.ONE (1e18) * decimals difference to reach 18 decimals (1e12) * token rate (1)
    scaling factor = 1e18 * 1e12 = 1e30

Assume that the primaryBalance is 100 DAI (100e18), and the secondaryBalance is 100 USDC (100e6). Line 25 - 28 of the _getSpotPrice function will normalize the tokens balances to 18 decimals as follows:

  • scaledPrimaryBalance will be 100e18 (It remains the same as no scaling is needed because DAI is already denominated in 18 decimals)

    scaledPrimaryBalance = primaryBalance * poolContext.primaryScaleFactor / BalancerConstants.BALANCER_PRECISION;
    scaledPrimaryBalance = 100e18 * 1e18 / 1e18
    scaledPrimaryBalance = 100e18
  • scaledSecondaryBalance will upscale to 100e18

    scaledSecondaryBalance = scaledSecondaryBalance * poolContext.primaryScaleFactor / BalancerConstants.BALANCER_PRECISION;
    scaledSecondaryBalance = 100e6 * 1e30 / 1e18
    scaledSecondaryBalance = 100e18

The StableMath._calcSpotPrice function at Line 39 returns the spot price of Y/X. In this example, balanceX is DAI, and balanceY is USDC. Thus, the spot price will be USDC/DAI. This means the amount of USDC I will get for each DAI.

Within Balancer, all stable math calculations within the Balancer's pools are performed in 1e18. With both the primary and secondary balances normalized to 18 decimals, they can be safely passed to the StableMath._calculateInvariant and StableMath._calcSpotPrice functions to compute the spot price. Assuming that the price of USDC and DAI is perfectly symmetric (1 DAI can be exchanged for exactly 1 USDC, and vice versa), the spot price returned from the StableMath._calcSpotPrice will be 1e18. Note that the spot price returned by the StableMath._calcSpotPrice function will be denominated in 18 decimals.

In Line 47-50 within the Stable2TokenOracleMath._getSpotPrice function, it attempts to downscale the spot price to normalize it back to the original decimals and token rate (e.g. stETH back to wstETH) of the token.

The scaleFactor at Line 47 will be evaluated as follows:

scaleFactor = poolContext.secondaryScaleFactor * BalancerConstants.BALANCER_PRECISION / poolContext.primaryScaleFactor
scaleFactor = 1e30 * 1e18 / 1e18
scaleFactor = 1e30

Finally, the spot price will be scaled in reverse order and it will be evaluated to 1e6 as shown below:

spotPrice = spotPrice * BalancerConstants.BALANCER_PRECISION / scaleFactor;
spotPrice = 1e18 * 1e18 / 1e30
spotPrice = 1e6

DAI/USDC Spot Price

If it is the opposite where the primary token is USDC (6 decimals) and the secondary token is DAI (18 decimals), the calculation of the spot price will be as follows:

The scaleFactor at Line 47 will be evaluated to as follows:

scaleFactor = poolContext.secondaryScaleFactor * BalancerConstants.BALANCER_PRECISION / poolContext.primaryScaleFactor
scaleFactor = 1e18 * 1e18 / 1e30
scaleFactor = 1e6

Finally, the spot price will be scaled in reverse order and it will be evaluated to 1e30 as shown below:

spotPrice = spotPrice * BalancerConstants.BALANCER_PRECISION / scaleFactor;
spotPrice = 1e18 * 1e18 / 1e6
spotPrice = 1e30

Note about the spot price

Assuming that the spot price of USDC and DAI is 1:1. As shown above, if the decimals of two tokens are not the same, the final spot price will end up either 1e6 (USDC/DAI) or 1e30 (DAI/USDC). However, if the decimals of two tokens (e.g. wstETH and WETH) are the same, this issue stays hidden as the scaleFactor in Line 47 will always be 1e18 as both secondaryScaleFactor and primaryScaleFactor cancel out each other.

It was observed that the spot price returned from the Stable2TokenOracleMath._getSpotPrice function is being compared with the oracle price from the TwoTokenPoolUtils._getOraclePairPrice function to determine if the pool has been manipulated within many functions.

uint256 oraclePrice = poolContext._getOraclePairPrice(strategyContext.tradingModule);

Based on the implementation of the TwoTokenPoolUtils._getOraclePairPrice function , the oraclePrice returned by this function is always denominated in 18 decimals regardless of the decimals of the underlying tokens. For instance, assume the spot price of USDC (6 decimals) and DAI (18 decimals) is 1:1. The spot price returned by this oracle function for USDC/DAI will be 1e18 and DAI/USDC will be 1e18.

In many functions, the spot price returned from the Stable2TokenOracleMath._getSpotPrice function is compared with the oracle price via the Stable2TokenOracleMath._checkPriceLimit. Following is one such example. The oraclePrice will be 1e18, while the spotPrice will be either 1e6 or 1e30 in our example. This will cause the _checkPriceLimit to always revert because of the large discrepancy between the two prices.

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/math/Stable2TokenOracleMath.sol#L71

File: Stable2TokenOracleMath.sol
71:     function _getMinExitAmounts(
72:         StableOracleContext calldata oracleContext,
73:         TwoTokenPoolContext calldata poolContext,
74:         StrategyContext calldata strategyContext,
75:         uint256 oraclePrice,
76:         uint256 bptAmount
77:     ) internal view returns (uint256 minPrimary, uint256 minSecondary) {
78:         // Oracle price is always specified in terms of primary, so tokenIndex == 0 for primary
79:         // Validate the spot price to make sure the pool is not being manipulated
80:         uint256 spotPrice = _getSpotPrice({
81:             oracleContext: oracleContext,
82:             poolContext: poolContext,
83:             primaryBalance: poolContext.primaryBalance,
84:             secondaryBalance: poolContext.secondaryBalance,
85:             tokenIndex: 0
86:         });
87:         _checkPriceLimit(strategyContext, oraclePrice, spotPrice);

Other affected functions include the following:

  • Stable2TokenOracleMath._validateSpotPriceAndPairPrice
  • Stable2TokenOracleMath._getTimeWeightedPrimaryBalance

Impact

A vault supporting tokens with two different decimals will have many of its key functions will be broken as the _checkPriceLimit will always revert. For instance, rewards cannot be reinvested and vaults cannot be settled since they rely on the _checkPriceLimit function.

If the reward cannot be reinvested, the strategy tokens held by the users will not appreciate. If the vault cannot be settled, the vault debt cannot be repaid to Notional and the gain cannot be realized. Loss of assets for both users and Notional

Code Snippet

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/math/Stable2TokenOracleMath.sol#L15

Tool used

Manual Review

Recommendation

Within the Stable2TokenOracleMath._getSpotPrice, normalize the spot price back to 1e18 before returning the result. This ensures that it can be compared with the oracle price, which is denominated in 1e18 precision.

This has been implemented in the spot price function (Boosted3TokenPoolUtils._getSpotPriceWithInvariant) of another pool (Boosted3Token). However, it was not consistently applied in TwoTokenPool.

@github-actions github-actions bot added the High A valid High severity issue label Jan 13, 2023
weitianjie2000 added a commit to notional-finance/leveraged-vaults that referenced this issue Jan 13, 2023
@weitianjie2000
Copy link

valid, will fix

@hrishibhat hrishibhat added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Jan 18, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Jan 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants