Beanstalk Part 1

Beanstalk

DeFiHardhatOracleProxyUpdates

100,000 USDC

View results

Previous Next

Submission Details

Severity: low

Invalid

Oracle is vulnerable to price manipulation due to faulty logic

holydevoti0n

Summary

The LibEthUsdOracle has the goal of providing functions that are resistant to price manipulation of ETH/USD. It contains the function getEthUsdPrice(uint256 lookback), according to the func's docs:

* A lookback of 900 seconds is used in Uniswap V3 pools for instantaneous price queries.

* If using a non-zero lookback, it is recommended to use a substantially large

* `lookback` (> 900 seconds) to protect against manipulation.

It is recommended to use a lookback of at least 900 seconds to be protected against price manipulation. The problem is that the function isn't protected against any lookback > 0 and < 900.

Vulnerability Details

Any value that is greater than 0 will be accepted by the getEthUsdPrice(uint256 lookback) function. i.e. a lookback can be set with a value like 1 and then be used on Chainlink and in the Uniswap Pools.

The current check only checks whether lookback = 0 or > lookback > type(uint32).max:

// Use a lookback of 900 seconds for an instantaneous price query for manipulation resistance.

if (lookback == 0) lookback = INSTANT_LOOKBACK;

if (lookback > type(uint32).max) return 0;

.....

uint256 usdcPrice = LibUniswapOracle.getEthUsdcPrice(uint32(lookback));

uint256 usdtPrice = LibUniswapOracle.getEthUsdtPrice(uint32(lookback));

Impact

The lack of the check for lookback > 1 && lookback < 900 will break the purpose of the oracle which is to provide "manipulation resistant ETH/USD price" using the greedy/max difference among Chainlink/Uniswap pools.

The current check accepts a very large lookback(type(uint32).max == 4294,967,295 seconds/136 years), if any large lookback is set i.e. days, it will potentially return a quite different price for ETH/USD when compared to the current price.

An attacker can take advantage of a very short/very long lookback when there is a big divergence between the Uniswap pools and the Chainlink oracle(which can also be different from the current market value when a distant lookback is used for instance), to use the Chainlink oracle price to his advantage.

PoC

Prepare the environment to work with Foundry + Updated Mocks
https://gist.github.com/h0lydev/fcdb00c797adfdf8e4816031e095fd6c
Add the function below to the MockSeasonFacet:

function getUsdPriceUsingLookback(uint256 lookback) external view returns (uint256) {

return LibUsdOracle.getUsdPrice(C.WETH, lookback);

}

Add import "forge-std/console.sol"; on LibEthUsdOracle and add the console.log("getEthUsdPrice lookback: %s", lookback); on line 74.(Right before passing the lookback to the uniswap pools)
Make sure to have the mainnet forked through Anvil: anvil --fork-url https://rpc.ankr.com/eth
Create the Oracle.t.sol file under the folder foundry and paste the code below. Then run forge test --match-contract OracleTest -vv.

// SPDX-License-Identifier: MIT

pragma solidity =0.7.6;

pragma abicoder v2;

import { Sun } from "contracts/beanstalk/sun/SeasonFacet/Sun.sol";

import { MockSeasonFacet } from "contracts/mocks/mockFacets/MockSeasonFacet.sol";

import { MockSiloFacet } from "contracts/mocks/mockFacets/MockSiloFacet.sol";

import { MockFieldFacet } from "contracts/mocks/mockFacets/MockFieldFacet.sol";

import { MockWhitelistFacet } from "contracts/mocks/mockFacets/MockWhitelistFacet.sol";

import {LibWhitelist} from "contracts/libraries/Silo/LibWhitelist.sol";

import { Utils } from "./utils/Utils.sol";

import "./utils/TestHelper.sol";

import "contracts/libraries/LibSafeMath32.sol";

import "contracts/C.sol";

contract OracleTest is MockSeasonFacet, TestHelper {

using SafeMath for uint256;

using LibSafeMath32 for uint32;

function setUp() public {

vm.createSelectFork('local');

setupDiamond();

setOraclePrices(1000e6, 1000e6, 1000e6);

}

function testOracle_whenCalledWithLessThan900secs_shouldReturnCallInvalid() public {

// Given a invalid lookback

uint256 lookback = 5;

// When

uint256 result = season.getUsdPriceUsingLookback(lookback);

// Then should return zero

// As a lookback < 900 increases the risk of price manipulation

assertEq(result, 0, "result should be 0");

}

function setOraclePrices(int256 chainlinkPrice, uint256 ethUsdtPrice, uint256 ethUsdcPrice) internal {

_setEthUsdtPrice(ethUsdtPrice);

_setEthUsdcPrice(ethUsdcPrice);

// add several rounds