xiaoming90 - Curve vault will undervalue or overvalue the LP Pool tokens if it comprises tokens with different decimals

[sherlock-admin]

xiaoming90

high

Curve vault will undervalue or overvalue the LP Pool tokens if it comprises tokens with different decimals

Summary

A Curve vault that comprises tokens with different decimals will undervalue or overvalue the LP Pool tokens. As a result, users might be liquidated prematurely or be able to borrow more than they are allowed. Additionally, the vault settlement process might break.

Vulnerability Detail

The TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function, which is utilized by the Curve vault, is used to compute the total value of the LP Pool tokens (poolClaim) denominated in the primary token.

https://github.com/sherlock-audit/2023-02-notional/blob/main/leveraged-vaults/contracts/vaults/common/internal/pool/TwoTokenPoolUtils.sol#L67

File: TwoTokenPoolUtils.sol
67:     function _getTimeWeightedPrimaryBalance(
68:         TwoTokenPoolContext memory poolContext,
69:         StrategyContext memory strategyContext,
70:         uint256 poolClaim,
71:         uint256 oraclePrice,
72:         uint256 spotPrice
73:     ) internal view returns (uint256 primaryAmount) {
74:         // Make sure spot price is within oracleDeviationLimit of pairPrice
75:         strategyContext._checkPriceLimit(oraclePrice, spotPrice);
76:         
77:         // Get shares of primary and secondary balances with the provided poolClaim
78:         uint256 totalSupply = poolContext.poolToken.totalSupply();
79:         uint256 primaryBalance = poolContext.primaryBalance * poolClaim / totalSupply;
80:         uint256 secondaryBalance = poolContext.secondaryBalance * poolClaim / totalSupply;
81: 
82:         // Value the secondary balance in terms of the primary token using the oraclePairPrice
83:         uint256 secondaryAmountInPrimary = secondaryBalance * strategyContext.poolClaimPrecision / oraclePrice;
84: 
85:         // Make sure primaryAmount is reported in primaryPrecision
86:         uint256 primaryPrecision = 10 ** poolContext.primaryDecimals;
87:         primaryAmount = (primaryBalance + secondaryAmountInPrimary) * primaryPrecision / strategyContext.poolClaimPrecision;
88:     }

If a leverage vault supports a Curve Pool that contains two tokens with different decimals, the math within the TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function would not work, and the value returned from it will be incorrect. Consider the following two scenarios:

If primary token's decimals (e.g. 18) > secondary token's decimals (e.g. 6)

To illustrate the issue, assume the following:

• The leverage vault supports the DAI-USDC Curve Pool, and its primary token of the vault is DAI.
• DAI's decimals are 18, while USDC's decimals are 6.
• Curve Pool's total supply is 100
• The Curve Pool holds 100 DAI and 100 USDC
• For the sake of simplicity, the price of DAI and USDC is 1:1. Thus, the oraclePrice within the function will be 1 * 10^18. Note that the oracle price is always scaled up to 18 decimals within the vault.

The caller of the TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function wanted to compute the total value of 50 LP Pool tokens.

primaryBalance = poolContext.primaryBalance * poolClaim / totalSupply; // 100 DAI * 50 / 100
secondaryBalance = poolContext.secondaryBalance * poolClaim / totalSupply; // 100 USDC * 50 / 100

The primaryBalance will be 50 DAI. 50 DAI denominated in WEI will be 50 * 10^18 since the decimals of DAI are 18.

The secondaryBalance will be 50 USDC. 50 USDC denominated in WEI will be 50 * 10^6 since the decimals of USDC are 6.

Next, the code logic attempts to value the secondary balance (50 USDC) in terms of the primary token (DAI) using the oracle price (1 * 10^18).

secondaryAmountInPrimary = secondaryBalance * strategyContext.poolClaimPrecision / oraclePrice;
secondaryAmountInPrimary = 50 USDC * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = (50 * 10^6) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = 50 * 10^6

50 USDC should be worth 50 DAI (50 * 10^18). However, the secondaryAmountInPrimary shows that it is only worth 0.00000000005 DAI (50 * 10^6).

primaryAmount = (primaryBalance + secondaryAmountInPrimary) * primaryPrecision / strategyContext.poolClaimPrecision;
primaryAmount = [(50 * 10^18) + (50 * 10^6)] * 10^18 / 10^18
primaryAmount = [(50 * 10^18) + (50 * 10^6)] // cancel out the 10^18
primaryAmount = 50 DAI + 0.00000000005 DAI = 50.00000000005 DAI

50 LP Pool tokens should be worth 100 DAI. However, the TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function shows that it is only worth 50.00000000005 DAI, which undervalues the LP Pool tokens.

If primary token's decimals (e.g. 6) < secondary token's decimals (e.g. 18)

To illustrate the issue, assume the following:

• The leverage vault supports the DAI-USDC Curve Pool, and its primary token of the vault is USDC.
• USDC's decimals are 6, while DAI's decimals are 18.
• Curve Pool's total supply is 100
• The Curve Pool holds 100 USDC and 100 DAI
• For the sake of simplicity, the price of DAI and USDC is 1:1. Thus, the oraclePrice within the function will be 1 * 10^18. Note that the oracle price is always scaled up to 18 decimals within the vault.

The caller of the TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function wanted to compute the total value of 50 LP Pool tokens.

primaryBalance = poolContext.primaryBalance * poolClaim / totalSupply; // 100 USDC * 50 / 100
secondaryBalance = poolContext.secondaryBalance * poolClaim / totalSupply; // 100 DAI * 50 / 100

The primaryBalance will be 50 USDC. 50 USDC denominated in WEI will be 50 * 10^6 since the decimals of USDC are 6.

The secondaryBalance will be 50 DAI. 50 DAI denominated in WEI will be 50 * 10^18 since the decimals of DAI are 18.

Next, the code logic attempts to value the secondary balance (50 DAI) in terms of the primary token (USDC) using the oracle price (1 * 10^18).

secondaryAmountInPrimary = secondaryBalance * strategyContext.poolClaimPrecision / oraclePrice;
secondaryAmountInPrimary = 50 DAI * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = (50 * 10^18) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = 50 * 10^18

50 DAI should be worth 50 USDC (50 * 10^6). However, the secondaryAmountInPrimary shows that it is worth 50,000,000,000,000 USDC (50 * 10^18).

primaryAmount = (primaryBalance + secondaryAmountInPrimary) * primaryPrecision / strategyContext.poolClaimPrecision;
primaryAmount = [(50 * 10^6) + (50 * 10^18)] * 10^6 / 10^18
primaryAmount = [(50 * 10^6) + (50 * 10^18)] / 10^12
primaryAmount = 50,000,000.00005 = 50 million

50 LP Pool tokens should be worth 100 USDC. However, the TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function shows that it is worth 50 million USDC, which overvalues the LP Pool tokens.

In summary, if a leverage vault has two tokens with different decimals:

• If primary token's decimals (e.g. 18) > secondary token's decimals (e.g. 6), then TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function will undervalue the LP Pool tokens
• If primary token's decimals (e.g. 6) < secondary token's decimals (e.g. 18), then TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function will overvalue the LP Pool tokens

Impact

A vault supporting tokens with two different decimals will undervalue or overvalue the LP Pool tokens.

The affected TwoTokenPoolUtils._getTimeWeightedPrimaryBalance function is called within the Curve2TokenPoolUtils._convertStrategyToUnderlying function that is used for valuing strategy tokens in terms of the primary balance. As a result, the strategy tokens will be overvalued or undervalued

Following are some of the impacts of this issue:

• If the strategy tokens are overvalued or undervalued, the users might be liquidated prematurely or be able to borrow more than they are allowed to since the Curve2TokenPoolUtils._convertStrategyToUnderlying function is indirectly used for computing the collateral ratio of an account within Notional's VaultConfiguration.calculateCollateralRatio function.
• expectedUnderlyingRedeemed is computed based on the Curve2TokenPoolUtils._convertStrategyToUnderlying function. If the expectedUnderlyingRedeemed is incorrect, it will break the vault settlement process.

Code Snippet

https://github.com/sherlock-audit/2023-02-notional/blob/main/leveraged-vaults/contracts/vaults/common/internal/pool/TwoTokenPoolUtils.sol#L67

Tool used

Manual Review

Recommendation

When valuing the secondary balance in terms of the primary token using the oracle price, the result should be scaled up or down the decimals of the primary token accordingly if the decimals of the two tokens are different.

The root cause of this issue is in the following portion of the code, which attempts to add the primaryBalance and secondaryAmountInPrimary before multiplying with the primaryPrecision. The primaryBalance and secondaryAmountInPrimary might not be denominated in the same decimals. Therefore, they cannot be added together without scaling them if the decimals of two tokens are different.

primaryAmount = (primaryBalance + secondaryAmountInPrimary) * primaryPrecision / strategyContext.poolClaimPrecision;

Consider implementing the following changes to ensure that the math within the _getTimeWeightedPrimaryBalance function work with tokens with different decimals. The below approach will scale the secondary token to match the primary token's precision before performing further computation.

function _getTimeWeightedPrimaryBalance(
	TwoTokenPoolContext memory poolContext,
	StrategyContext memory strategyContext,
	uint256 poolClaim,
	uint256 oraclePrice,
	uint256 spotPrice
) internal view returns (uint256 primaryAmount) {
	// Make sure spot price is within oracleDeviationLimit of pairPrice
	strategyContext._checkPriceLimit(oraclePrice, spotPrice);
	
	// Get shares of primary and secondary balances with the provided poolClaim
	uint256 totalSupply = poolContext.poolToken.totalSupply();
	uint256 primaryBalance = poolContext.primaryBalance * poolClaim / totalSupply;
	uint256 secondaryBalance = poolContext.secondaryBalance * poolClaim / totalSupply;

+	// Scale secondary balance to primaryPrecision
+	uint256 primaryPrecision = 10 ** poolContext.primaryDecimals;
+	uint256 secondaryPrecision = 10 ** poolContext.secondaryDecimals;
+	secondaryBalance = secondaryBalance * primaryPrecision / secondaryPrecision
			
	// Value the secondary balance in terms of the primary token using the oraclePairPrice
	uint256 secondaryAmountInPrimary = secondaryBalance * strategyContext.poolClaimPrecision / oraclePrice;
	
-	// Make sure primaryAmount is reported in primaryPrecision
-	uint256 primaryPrecision = 10 ** poolContext.primaryDecimals;
-	primaryAmount = (primaryBalance + secondaryAmountInPrimary) * primaryPrecision / strategyContext.poolClaimPrecision;
+	primaryAmount = primaryBalance + secondaryAmountInPrimary
}

The poolContext.primaryBalance or poolClaim are not scaled up to strategyContext.poolClaimPrecision. Thus, the primaryBalance is not scaled in any form. Thus, I do not see the need to perform any conversion at the last line of the _getTimeWeightedPrimaryBalance function.

uint256 primaryBalance = poolContext.primaryBalance * poolClaim / totalSupply;

The following attempts to run through the examples in the previous section showing that the updated function produces valid results after the changes.

If primary token's decimals (e.g. 18) > secondary token's decimals (e.g. 6)

Primary Balance = 50 DAI (18 Deci), Secondary Balance = 50 USDC (6 Deci)

secondaryBalance = secondaryBalance * primaryPrecision / secondaryPrecision
secondaryBalance = 50 USDC * 10^18 / 10^6
secondaryBalance = (50 * 10^6) * 10^18 / 10^6 = (50 * 10^18)

secondaryAmountInPrimary = secondaryBalance * strategyContext.poolClaimPrecision / oraclePrice;
secondaryAmountInPrimary = (50 * 10^18) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = (50 * 10^18) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = 50 * 10^18

primaryAmount = primaryBalance + secondaryAmountInPrimary
primaryAmount = (50 * 10^18) + (50 * 10^18) = (100 * 10^18) = 100 DAI

If primary token's decimals (e.g. 6) < secondary token's decimals (e.g. 18)

Primary Balance = 50 USDC (6 Deci), Secondary Balance = 50 DAI (18 Deci)

secondaryBalance = secondaryBalance * primaryPrecision / secondaryPrecision
secondaryBalance = 50 DAI * 10^6 / 10^18
secondaryBalance = (50 * 10^18) * 10^6 / 10^18 = (50 * 10^6)

secondaryAmountInPrimary = secondaryBalance * strategyContext.poolClaimPrecision / oraclePrice;
secondaryAmountInPrimary = (50 * 10^6) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = (50 * 10^6) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = 50 * 10^6

primaryAmount = primaryBalance + secondaryAmountInPrimary
primaryAmount = (50 * 10^6) + (50 * 10^6) = (100 * 10^6) = 100 USDC

If primary token's decimals (e.g. 6) == secondary token's decimals (e.g. 6)

Primary Balance = 50 USDC (6 Deci), Secondary Balance = 50 USDT (6 Deci)

secondaryBalance = secondaryBalance * primaryPrecision / secondaryPrecision
secondaryBalance = 50 USDT * 10^6 / 10^6
secondaryBalance = (50 * 10^6) * 10^6 / 10^6 = (50 * 10^6)

secondaryAmountInPrimary = secondaryBalance * strategyContext.poolClaimPrecision / oraclePrice;
secondaryAmountInPrimary = (50 * 10^6) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = (50 * 10^6) * 10^18 / (1 * 10^18)
secondaryAmountInPrimary = 50 * 10^6

primaryAmount = primaryBalance + secondaryAmountInPrimary
primaryAmount = (50 * 10^6) + (50 * 10^6) = (100 * 10^6) = 100 USDC

strategyContext.poolClaimPrecision set to CurveConstants.CURVE_PRECISION, which is 1e18. oraclePrice is always in 1e18 precision.

[jeffywu]

Valid, since this is shared code it is better to always scale decimals up to 18 rather than assume that they are. Even if this results in duplicate work for the Balancer strategies.