ast3ros - Result from Chainlink oracle price is not checked for validity

[sherlock-admin]

ast3ros

medium

Result from Chainlink oracle price is not checked for validity

Summary

Chainlink oracle price is not checked for validity.

Vulnerability Detail

In Kept contract, the ETH price is retrieved from Chainlink oracle to calculate the keeper fee.

    (, int256 answer, , ,) = ethTokenOracleFeed().latestRoundData();

https://github.com/sherlock-audit/2023-07-perennial/blob/main/root/contracts/attribute/Kept.sol#L62

However, the price is not sufficiently validated. The updatedAt result is ignored and is not checked with a threshold to ensure the price is not too old.

Impact

The stale price will lead to incorrect keeper fee calculation.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/root/contracts/attribute/Kept.sol#L62

Tool used

Manual Review

Recommendation

Add a checking

        function _etherPrice() private view returns (UFixed18) {
-           (, int256 answer, , ,) = ethTokenOracleFeed().latestRoundData();
+           (uint80 roundID, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = ethTokenOracleFeed().latestRoundData();
+           require(answer > 0,"Negative Price");
+           require(block.timestamp <= updatedAt + stalePriceDelay, "Stale price");
            return UFixed18Lib.from(Fixed18Lib.ratio(answer, 1e8)); 
        }

Duplicate of #159

[sherlock-admin]

3 comment(s) were left on this issue during the judging contest.

141345 commented:

d

n33k commented:

unhandled stale price returned from latestRoundData()

darkart commented:

The same as 120