code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

Chainlink data could get unintended stale price #134

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/oracles/ChainlinkOracleProvider.sol#L55-L58

Vulnerability details

Impact

Though it checks updatedAt with stalePriceDelay, it still gets an unintended stale price without checking roundId.

Proof of Concept

In getPriceUSD function of ChainlinkOracleProvider.sol:

        (, int256 answer, , uint256 updatedAt, ) = AggregatorV2V3Interface(feed).latestRoundData();

        require(block.timestamp <= updatedAt + stalePriceDelay, Error.STALE_PRICE);
        require(answer >= 0, Error.NEGATIVE_PRICE);

latestRoundData() also returns roundId and answeredInRound, it's not safe to only check updateAt.

Tools Used

vim, ethers.js

Recommended Mitigation Steps

Also check roundId when getting price:

+        (uint80 roundId, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = AggregatorV2V3Interface(feed).latestRoundData();

         require(block.timestamp <= updatedAt + stalePriceDelay, Error.STALE_PRICE);
         require(answer >= 0, Error.NEGATIVE_PRICE);
+        require(answeredInRound >= roundID, Error.STALE_PRICE);
chase-manning commented 2 years ago

Duplicate of #17