DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: high
Valid

LibUsdOracle will compromise Beanstalk peg due to wrong price and DoS

Vulnerability Details

The function getUsdPricefrom LibUsdOracle should return the token value in USD. For example, one of its consumers is the function getMintFertilizerOut:

fertilizerAmountOut = tokenAmountIn.div(LibUsdOracle.getUsdPrice(barnRaiseToken));

The goal of this function is to return the fertilizer amount with 0 decimals, therefore if the WETH value is at $1000 and the user would provide 1 WETH:

tokenAmountIn = 1e18.div(1e15) = 1e3 = 1000

Another critical use case for getUsdPrice is: fetching the ratios to calculate the deltaB.

function getRatiosAndBeanIndex(
IERC20[] memory tokens,
uint256 lookback
) internal view returns (uint[] memory ratios, uint beanIndex, bool success) {
success = true;
ratios = new uint[](tokens.length);
beanIndex = type(uint256).max;
for (uint i; i < tokens.length; ++i) {
if (C.BEAN == address(tokens[i])) {
beanIndex = i;
ratios[i] = 1e6;
} else {
@> ratios[i] = LibUsdOracle.getUsdPrice(address(tokens[i]), lookback); // @audit expect value return in USD
if (ratios[i] == 0) {
success = false;
}
}
}
require(beanIndex != type(uint256).max, "Bean not in Well.");
}

The function getRatiosAndBeanIndex is used throughout the system, including in processes like Sunrise and Convert, to help Bean maintain its peg.

  • The first issue is that the price returned by getUsdPrice will be incorrect for tokens that use "external oracle" due to the duplicated division using 1e24.

// getUsdPrice
@> uint256 tokenPrice = getTokenPriceFromExternal(token, lookback);
if (tokenPrice == 0) return 0;
@> return uint256(1e24).div(tokenPrice);
// getTokenPriceFromExternal
//returns uint256(1e24).div(result from chainlink with 6 decimals)
return
@> uint256(1e24).div(
LibChainlinkOracle.getTokenPrice(
chainlinkOraclePriceAddress,
LibChainlinkOracle.FOUR_HOUR_TIMEOUT,
lookback
)
);

This will result in an inaccurate price. For example, when the price of WBTC is $50,000, it will return 50000e6 instead of the correct value 2e13. This discrepancy will cause Beanstalk to consider an erroneous ratio, resulting in an incorrect DeltaB.

Beanstalk uses deltaB to determine:

  • Whether the system is Above or below the peg

  • Sow and Pods(issuing debt)

  • Flood

  • Convert

In a nutshell, all components from Beanstalk will be affected by DeltaBs originating from tokens using Oracle with encodedType 0x01.

  • The second issue is that the getTokenPriceFromExternal doesn't check for the return from the Oracle value before div:

return
@> uint256(1e24).div(
LibChainlinkOracle.getTokenPrice(
chainlinkOraclePriceAddress,
LibChainlinkOracle.FOUR_HOUR_TIMEOUT,
lookback
)
);

When the Chainlink Oracle fails to fetch the price it returns 0. Hence, the function will revert due to uint256(1e24).div(0).

PoC

  1. For the incorrect price:

Add the following test on oracle.t.sol

function test_getUsdPrice_whenExternalToken_priceIsInvalid() public {
// pre condition: encode type 0x01
// WETH price is 1000
uint256 priceWETH = OracleFacet(BEANSTALK).getUsdPrice(C.WETH);
assertEq(priceWETH, 1e15); // 1e18/1e3 = 1e15
// WBTC price is 50000
uint256 priceWBTC = OracleFacet(BEANSTALK).getUsdPrice(WBTC);
assertEq(priceWBTC, 2e13); // 1e24.div(50000e6) = 2e13
}

Run: forge test --match-test test_getUsdPrice_whenExternalToken_priceIsInvalid

Output:

Failing tests:
Encountered 1 failing test in test/foundry/silo/oracle.t.sol:OracleTest
[FAIL. Reason: assertion failed: 50000000000 != 20000000000000] test_getUsdPrice_whenExternalToken_priceIsInvalid() (gas: 64594)
  1. For the DoS when Chainlink oracle returns 0:
    Add this test on oracle.t.sol

// first - import {MockChainlinkAggregator} from "contracts/mocks/chainlink/MockChainlinkAggregator.sol";
function test_getUsdPrice_willDoS_whenOracleFail() public {
// pre condition: encode type 0x01 and oracle fail
MockChainlinkAggregator(WBTC_USD_CHAINLINK_PRICE_AGGREGATOR).setOracleFailure();
// action
vm.expectRevert();
OracleFacet(BEANSTALK).getUsdPrice(WBTC);
}

Run: forge test --match-test test_getUsdPrice_willDoS_whenOracleFail

Output:

Ran 1 test for test/foundry/silo/oracle.t.sol:OracleTest
[PASS] test_getUsdPrice_willDoS_whenOracleFail() (gas: 21500)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 259.13ms

Impact

High.

  • Incorrect price: This will compromise the entire system, as all peg-related components, including Sunrise and Convert, depend on the price.

  • DoS: Sunrise will revert and disrupt all other components that rely on the price. The risk of a DoS is increased due to stepOracle calling multiple tokens(instead of one as previously) to fetch their prices, raising the chances of an Oracle failure.

Tools Used

Manual Review & Foundry

Recommendations

  • To fix both issues: On getTokenPriceFromExternal return the price from Chainlink directly, instead of scaling it.

return LibChainlinkOracle.getTokenPrice(
chainlinkOraclePriceAddress,
LibChainlinkOracle.FOUR_HOUR_TIMEOUT,
lookback
);
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

holydevoti0n Submitter
about 1 year ago
golanger85 Auditor
about 1 year ago
giovannidisiena Auditor
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`getUsdPrice` returns mixed decimals

Support

FAQs

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