✍️Not Validating Return Data e.g Chainlink: (lastestRoundData)

Book an audit with Zokyo

Use latestRoundData instead latestAnswer to run more validations

Vulnerability Details

Impact

The UniswapV3Oracle.sol retrieves the last WETH price through the latestAnswer method. While this method provides the last recorded value, it doesn't allow for checking the freshness or the validity of the data, which can be crucial for accurate and secure operations.

Enhanced Data Validation Approach

Utilizing the latestRoundData method instead offers the ability to conduct additional validations ensuring the data's accuracy and timeliness. Below is an example of how to implement these extra checks:

solidityCopy code(
    roundId,
    rawPrice,
    ,
    updateTime,
    answeredInRound
) = AggregatorV3Interface(XXXXX).latestRoundData();

require(rawPrice > 0, "Chainlink price <= 0");
require(updateTime != 0, "Incomplete round");
require(answeredInRound >= roundId, "Stale price");

These validation checks will help ensure that the price data used is not only non-zero but also derived from a complete round and is not stale, providing greater reliability and security in the price information used for operations.

Additional Resources

For more details on how to ascertain if a round’s answer is carried over from a previous round, please refer to the Chainlink documentation.

Chainlink's latestRoundData might return stale or incorrect results

Vulnerability Details

Code Location

ChainlinkOracle.sol#L50-L60

solidityCopy codefunction sync() public {
    (, int256 feedPrice, , uint256 timestamp, ) = feed.latestRoundData();
    Fixed18 price = Fixed18Lib.ratio(feedPrice, SafeCast.toInt256(_decimalOffset));

    if (priceAtVersion.length == 0 || timestamp > timestampAtVersion[currentVersion()] + minDelay) {
        priceAtVersion.push(price);
        timestampAtVersion.push(timestamp);

        emit Version(currentVersion(), timestamp, price);
    }
}

Issue

The sync function in ChainlinkOracle.sol employs latestRoundData without validating whether the returned data is stale, potentially leading to the usage of outdated prices. According to Chainlink’s documentation, it's imperative to check for stale data to ensure price accuracy and reliability.

• Historical Price Data
• Chainlink FAQ on Stale Data

Recommendation

Introduce checks to verify that the data obtained is not stale. For instance:

solidityCopy code(uint80 roundID, int256 feedPrice, , uint256 timestamp, uint80 answeredInRound) = feed.latestRoundData();
require(feedPrice > 0, "Chainlink price <= 0");
require(answeredInRound >= roundID, "Stale price");
require(timestamp != 0, "Round not complete");

Adding these checks will safeguard against the risks associated with stale data, enhancing the robustness and reliability of your price feeds.

SHOULD CHECK RETURN DATA FROM CHAINLINK AGGREGATORS

The latestRoundData function within PriceFeed.sol contract retrieves asset prices from a Chainlink aggregator. Nonetheless, the function lacks adequate checks on roundID, creating a vulnerability to stale prices, which might jeopardize funds. According to Chainlink's documentation, when no answer is reached, the function doesn’t throw an error but instead returns 0. This behavior leads to the PriceOracle receiving incorrect price data. Dependence on an external Chainlink oracle, serving as the source of index price data, comes with risks typically associated with third-party data reliance. If the oracle is not properly maintained, outdated or incorrect data might be used in the system's index price calculations, potentially causing liquidity issues.

Proof of Concept

Refer to the contract at the following GitHub link. The existing checks in the contract are as follows:

solidityCopy codeif (!_response.success) {return true;}
if (_response.roundId == 0) {return true;}
if (_response.timestamp == 0 || _response.timestamp > block.timestamp) {return true;}
if (_response.answer <= 0) {return true;}

Tools Used

• Manual Review

Recommended Mitigation Steps

It's advisable to implement additional checks on the returned data and to provide clear revert messages for stale prices or incomplete rounds. For instance:

solidityCopy code(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = ETH_CHAINLINK.latestRoundData();
require(price > 0, "Chainlink price <= 0"); 
require(answeredInRound >= roundID, "Stale price detected");
require(timeStamp != 0, "Incomplete round detected");

By adding these checks, you can enhance the contract's resilience against stale or inaccurate price data, improving overall security and reliability.

Lacking Validation Of Chainlink' Oracle Queries

Vulnerability Details

Impact

The TwapOracle.consult() function is currently devoid of crucial validations that ensure completion and validity of a round, leading to potential risks. Notably, it recklessly casts an int256 price to uint256 without preliminary value checks, thereby exposing the system to the risk of underflow. This flaw may yield unexpected results, further propagating issues throughout the protocol wherever this function is invoked.

Moreover, the GasThrottle.validateGas() modifier relies on Chainlink's latestAnswer() function, which unfortunately doesn’t incorporate necessary checks against stale data. For enhanced data integrity, it is advisable to use latestRoundData() as it inherently supports additional validation checks.

Recommended Mitigation Steps

For safeguarding against the identified vulnerabilities, initiate validation of the latestRoundData() output with the code snippet provided below:

solidityCopy code(
    uint80 roundID,
    int256 price,
    ,
    uint256 updateTime,
    uint80 answeredInRound
) = ETH_CHAINLINK.latestRoundData();
require(answeredInRound >= roundID, "Chainlink Price Stale");
require(price > 0, "Chainlink Malfunction");
require(updateTime != 0, "Incomplete round");

Implement these validation checks within both TwapOracle.consult() and GasThrottle.validateGas(). For the latter, ensure replacement of the latestAnswer() function with latestRoundData() to eliminate the risk of utilizing stale data.

Oracle data feed is insufficiently validated

Vulnerability Details

Impact

Stale prices pose a significant risk as they can result in incorrect quoteAmount return values. Without proper validation checks for the oracle data feed, the contract might use outdated or incorrect price data, leading to potentially erroneous or malfunctional operations within the protocol.

Proof of Concept

The _peek function in question lacks adequate validation checks for the data retrieved from the oracle. In its current implementation, it does not verify the freshness of the price data or the completeness of the data round, which is crucial for ensuring accurate and reliable price information.

solidityCopy codefunction _peek(
    bytes6 base,
    bytes6 quote,
    uint256 baseAmount
) private view returns (uint256 quoteAmount, uint256 updateTime) {
    ...

    (, int256 daiPrice, , , ) = DAI.latestRoundData();
    (, int256 usdcPrice, , , ) = USDC.latestRoundData();
    (, int256 usdtPrice, , , ) = USDT.latestRoundData();

    require(
        daiPrice > 0 && usdcPrice > 0 && usdtPrice > 0,
        "Chainlink pricefeed reporting 0"
    );

    ...
}

Tools Used

• Manual Review

Recommended Mitigation Steps

To remedy this vulnerability, it’s imperative to implement robust validation for the oracle data feed in the _peek function. Below is a revised version of the function with added validation checks for each price data retrieved:

solidityCopy codefunction _peek(
    bytes6 base,
    bytes6 quote,
    uint256 baseAmount
) private view returns (uint256 quoteAmount, uint256 updateTime) {
    ...
    (uint80 roundID, int256 daiPrice, , uint256 timestamp, uint80 answeredInRound) = DAI.latestRoundData();
    require(daiPrice > 0, "ChainLink: DAI price <= 0");
    require(answeredInRound >= roundID, "ChainLink: Stale DAI price");
    require(timestamp > 0, "ChainLink: DAI round not complete");

    (roundID, int256 usdcPrice, , timestamp, answeredInRound) = USDC.latestRoundData();
    require(usdcPrice > 0, "ChainLink: USDC price <= 0");
    require(answeredInRound >= roundID, "ChainLink: Stale USDC price");
    require(timestamp > 0, "ChainLink: USDC round not complete");

    (roundID, int256 usdtPrice, , timestamp, answeredInRound) = USDT.latestRoundData();
    require(usdtPrice > 0, "ChainLink: USDT price <= 0");
    require(answeredInRound >= roundID, "ChainLink: Stale USDT price");
    require(timestamp > 0, "ChainLink: USDT round not complete");
    ...
}

By incorporating these validation checks, you will ensure that the contract only uses valid, fresh, and complete price data, thereby mitigating the risk of using stale prices and improving the overall security and reliability of the contract's operations.

No sanity check on pricePerShare might lead to lost value

Overview:

The pricePerShare value is sourced either directly from an oracle or derived from ibBTC's core system. However, the current implementation lacks adequate safeguards and validation checks against potential bugs, exploits, or incorrect data originating from these sources.

Impact:

The pricePerShare is instrumental in determining the transfer amounts within the system. If there's an anomaly or discrepancy in its value— specifically, if pricePerShare is erroneously reported to be lower than its true market value— there's a tangible risk of substantial fund depletion from the associated Curve pool. In scenarios where the pricePerShare is undervalued, it may trigger an unintended and excessive outflow of wibbtc from the Curve pool, leading to significant financial losses and undermining the integrity and stability of the entire pool.

Recommended Mitigation Steps:

To preemptively address and mitigate this vulnerability:

1. Implement Validation Checks: Introduce rigorous validation checks to confirm the accuracy and integrity of the pricePerShare data retrieved from both the oracle and ibBTC's core. This includes cross-verifying the received data against reliable external sources and implementing fail-safes and alerts for any discrepancies detected.

2. Incorporate Safety Mechanisms: Design and embed safety mechanisms and contingency protocols to halt or limit transfers promptly if there's a suspected issue or anomaly with the pricePerShare value. This reactive measure can prevent fund drainage and provide an opportunity for manual intervention and assessment.