# Not Validating Return Data e.g Chainlink: (lastestRoundData) {% hint style="info" %} [**Book an audit with Zokyo**](https://www.zokyo.io/) {% endhint %} ## [Use latestRoundData instead latestAnswer to run more validations](https://github.com/code-423n4/2021-07-wildcredit-findings/issues/75) #### 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: ```solidity 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](https://docs.chain.link/docs/faq/#how-can-i-check-if-the-answer-to-a-round-is-being-carried-over-from-a-previous-round). ## [Chainlink's `latestRoundData` might return stale or incorrect results](https://github.com/code-423n4/2021-12-perennial-findings/issues/24) #### Vulnerability Details **Code Location** [ChainlinkOracle.sol#L50-L60](https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/oracle/ChainlinkOracle.sol#L50-L60) ```solidity 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](https://docs.chain.link/docs/historical-price-data/#historical-rounds) * [Chainlink FAQ on Stale Data](https://docs.chain.link/docs/faq/#how-can-i-check-if-the-answer-to-a-round-is-being-carried-over-from-a-previous-round) **Recommendation** Introduce checks to verify that the data obtained is not stale. For instance: ```solidity 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](https://github.com/code-423n4/2021-12-yetifinance-findings/issues/91) 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](https://github.com/code-423n4/2021-12-yetifinance/blob/1da782328ce4067f9654c3594a34014b0329130a/packages/contracts/contracts/PriceFeed.sol#L578). The existing checks in the contract are as follows: ```solidity 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: ```solidity 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: ```solidity 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. ```solidity 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: ```solidity 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.