Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

LibUsdOracle returns the wrong price for Uniswap Oracle

holydevoti0n

Vulnerability Details

The Generalised Oracle is broken for the external tokens that use Uniswap. This is happening for two reasons:

  1. The token passed as a base token for the OracleLibrary.getQuoteAtTick is incorrect. The base token should be the token the protocol wants to fetch the price from, not the quote token. Take into consideration the following scenario: Fetching the price for WBTC.

// LibUsdOracle -> getTokenPriceFromExternal
address chainlinkToken = IUniswapV3PoolImmutables(oracleImpl.target).token0();
chainlinkToken = chainlinkToken == token
? IUniswapV3PoolImmutables(oracleImpl.target).token1()
: token;
tokenPrice = LibUniswapOracle.getTwap(
lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES : uint32(lookback),
oracleImpl.target,
@> chainlinkToken, // @audit baseToken: chainlinkToken is USDC
token, // @audit quoteToken - WBTC
uint128(10) ** uint128(IERC20Decimals(token).decimals()) // @audit base token amount

The function getTwap:

function getTwap(
uint32 lookback,
address pool,
address token1, // @audit USDC
address token2, // @audit WBTC
uint128 oneToken // @audit 1e8
) internal view returns (uint256 price) {
(bool success, int24 tick) = consult(pool, lookback);
if (!success) return 0;
@> price = OracleLibrary.getQuoteAtTick(tick, oneToken, token1, token2); // @audit token1 == USDC
}

Due to this, the protocol is trying to fetch the price of the amount 1e8 in USDC, using as quote price WBTC, instead of the intended behavior that should be quoting the price of 1 WBTC(1e8) in USDC.

  • The second issue is that the getTwap function can return values with varying decimals. The protocol assumes a 6-decimal return value, but some stablecoins, like DAI, have 18 decimals.

Notice that at the end of the flow for returning the price on the getTokenPriceFromExternal the token price is divided by 6.

uint256 chainlinkTokenPrice = LibChainlinkOracle.getTokenPrice(
chainlinkOraclePriceAddress,
LibChainlinkOracle.FOUR_HOUR_TIMEOUT,
lookback
);
return tokenPrice.mul(chainlinkTokenPrice).div(1e6); // @audit assume uniswap price is 6 decimals

If the quote token is DAI, for instance, the value will be significantly higher than expected in the LibUsdOracle. All other oracles in the protocol, such as LibChainlinkOracle and LibWsethUsdOracle, return values based on 6 decimals. The LibUniswapOracle has worked well so far because Beanstalk hasn't introduced stablecoins with decimals != 6. Therefore, the getTwap function should be modified to return values quoted with 6 decimals.

Impact

  • LibUsdOracle can't return the correct price for any external token using Uniswap as Oracle.

Tools Used

Manual Review

Recommendations

  1. 1st: Use token as the base token to ensure the chainlinkToken (quote token) returns the correct price.

​
tokenPrice = LibUniswapOracle.getTwap(
lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES : uint32(lookback),
oracleImpl.target,
- chainlinkToken,
- token,
+ token,
+ chainlinkToken,
uint128(10) ** uint128(IERC20Decimals(token).decimals())
);

  • 2nd: Modify the getTwap function to always return the value in 6 decimal precision. Additionally, rename the variables for better clarity and usability.

+ uint256 constant PRECISION = 1e6;
+
+ /**
+ * @notice Given a tick and a token amount, calculates the amount of token received in exchange
+ * @param baseTokenAmount Amount of baseToken to be converted.
+ * @param baseToken Address of the ERC20 token contract used as the baseAmount denomination.
+ * @param quoteToken Address of the ERC20 token contract used as the quoteAmount denomination.
+ * @return price Amount of quoteToken. Value has 6 decimal precision.
+ */
function getTwap(
uint32 lookback,
address pool,
- address token1,
- address token2,
- uint128 oneToken
+ address baseToken,
+ address quoteToken,
+ uint128 baseTokenAmount
) internal view returns (uint256 price) {
(bool success, int24 tick) = consult(pool, lookback);
if (!success) return 0;
- price = OracleLibrary.getQuoteAtTick(tick, oneToken, token1, token2);
+ price = OracleLibrary.getQuoteAtTick(tick, baseTokenAmount, baseToken, quoteToken);
​
+ uint256 baseTokenDecimals = IERC20Decimals(baseToken).decimals();
+ uint256 quoteTokenDecimals = IERC20Decimals(quoteToken).decimals();
+ int256 factor = int256(baseTokenDecimals) - int256(quoteTokenDecimals);
+
+ // decimals are the same. i.e. DAI/WETH
+ if (factor == 0) return price.mul(PRECISION).div(baseTokenDecimals);
+
+ // scale decimals
+ if (factor > 0) {
+ price = price.mul(10 ** uint256(factor));
+ } else {
+ price = price.div(10 ** uint256(-factor));
+ }
+
+ // set 1e6 precision
+ price.mul(PRECISION).div(baseTokenDecimals);
​
}

Unit tests

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

LibUsdOracle confuses between baseToken and quoteToken leading to incorrect price quotes

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.