Skip to content
This repository has been archived by the owner on Nov 5, 2023. It is now read-only.

cducrest-brainbot - ChainlinkAdapterOracle can still give stale data #43

Closed
sherlock-admin opened this issue Apr 30, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Apr 30, 2023

cducrest-brainbot

medium

ChainlinkAdapterOracle can still give stale data

Summary

The implemented fix to issue sherlock-audit/2023-02-blueberry-judging#94 is not sufficient. The ChainlinkAdapterOracle can still give stale data.

Vulnerability Detail

The fix is the following:

    function getPrice(address token_) external view override returns (uint256) {
        // remap token if possible
        address token = remappedTokens[token_];
        if (token == address(0)) token = token_;

        uint256 maxDelayTime = timeGaps[token];
        if (maxDelayTime == 0) revert Errors.NO_MAX_DELAY(token_);

        // Get token-USD price
        uint256 decimals = registry.decimals(token, USD);
        (, int256 answer, , uint256 updatedAt, ) = registry.latestRoundData(
            token,
            USD
        );
        if (updatedAt < block.timestamp - maxDelayTime)
            revert Errors.PRICE_OUTDATED(token_);
+       if (answer <= 0) revert Errors.PRICE_NEGATIVE(token_); 

        return
            (answer.toUint256() * Constants.PRICE_PRECISION) / 10 ** decimals;
    }

There is no check regarding answeredInRound and roundId, so the price data could be carried over.

Impact

Correct price is detrimental to the protocol as a whole. It is required to guarantee proper borrow limits, liquidation conditions, etc ...

An incorrect price may result in loss of funds for users e.g. when they are liquidated due to an incorrect price while they should not have.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/oracle/ChainlinkAdapterOracle.sol#L77-L97

Tool used

Manual Review

Reference

code-423n4/2022-04-backd-findings#17
https://docs.chain.link/data-feeds/historical-data

Recommendation

Add require(answeredInRound >= roundID, "Stale price"); as an additional check:

    function getPrice(address token_) external view override returns (uint256) {
        // remap token if possible
        address token = remappedTokens[token_];
        if (token == address(0)) token = token_;

        uint256 maxDelayTime = timeGaps[token];
        if (maxDelayTime == 0) revert Errors.NO_MAX_DELAY(token_);

        // Get token-USD price
        uint256 decimals = registry.decimals(token, USD);
-        (, int256 answer, , uint256 updatedAt, ) = registry.latestRoundData(
+        (uint80 roundID, int256 answer, uint256 updatedAt, uint80 answeredInRound) = registry.latestRoundData(
            token,
            USD
        );
        if (updatedAt < block.timestamp - maxDelayTime)
            revert Errors.PRICE_OUTDATED(token_);
       if (answer <= 0) revert Errors.PRICE_NEGATIVE(token_); 
+      if(answeredInRound < roundID) revert Errors.PRICE_OUTDATED(token_);

        return
            (answer.toUint256() * Constants.PRICE_PRECISION) / 10 ** decimals;
    }

Duplicate of #118

@github-actions github-actions bot closed this as completed May 3, 2023
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label May 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant