Lack of price freshness validation in `LRTOracle.sol#getAssetPrice()` and `ChainlinkPriceOracle.sol#getAssetPrice()` allows a stale price to be used

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/oracles/ChainlinkPriceOracle.sol#L38 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L46

Vulnerability details

ChainlinkPriceOracle should use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough to be used. LRTOracle should implement price freshness validation for any oracle used in the protocol.

Impact

A stale price can cause the malfunction of the LRTDepositPool.sol#depositAsset() function:

When a user calls the LRTDepositPool.sol#depositAsset() function, if the price of the asset has dropped, and the oracle price feed is stale, an outdated price will be used and the user will get more rsETH tokens than he should.

Proof of Concept

In the current implementation of ChainlinkPriceOracle.sol#getAssetPrice(), there is no price staleness check. This could lead to stale prices being used.

If the market price of the asset drops very quickly ("flash crashes"), and Chainlink's feed does not get updated in time, the smart contract will continue to believe the asset is worth more than the market value.

A user can take advantage of the oracle price feed staleness in the following scenario:

1. The user calls the LRTDepositPool.sol#depositAsset() function. The asset to stake price has dropped, but the oracle price feed has not updated the answer yet.

2. _mintRsETH() is called inside the depositAsset() function

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L141 uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);

3. The LRTDepositPool.sol#_mintRsETH() function calculates the rsETH amount to mint based on asset amount and asset exchange rates from the oracle.

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L152 (rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);

4. _mintRsETH() calls getRsETHAmountToMint() to view the amount of rsETH to mint

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L109 rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

5. LRTOracle.sol lacks price freshness validation in the getAssetPrice() function

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L45-L47

    function getAssetPrice(address asset) public view onlySupportedAsset(asset) returns (uint256) {
        return IPriceFetcher(assetPriceOracle[asset]).getAssetPrice(asset);
    }

ChainlinkPriceOracle.sol#getAssetPrice() also lacks price freshness validation

https://github.com/code-423n4/2023-11-kelp/blob/main/src/oracles/ChainlinkPriceOracle.sol#L37-L39

    function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
        return AggregatorInterface(assetPriceFeed[asset]).latestAnswer();
    }

Due to the oracle price feed staleness, the protocol gets an outdated answer (price higher than the actual price)

6. The calculation of rsETH tokens minted utilizes the outdated price, resulting in the issuance of a greater number of rsETH tokens to the user than what would be allocated using an up-to-date price.

Tools Used

Manual Review

Recommended Mitigation Steps

1. Consider adding a price freshness check in ChainlinkPriceOracle.sol#getAssetPrice()

    function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
        AggregatorInterface priceFeed = AggregatorInterface(assetPriceFeed[asset]);
        (, int256 answer, , uint256 updatedAt, ) = priceFeed.latestRoundData();

        // Check if the price data is stale
        require(block.timestamp - updatedAt < MAX_STALE_INTERVAL, "Price data is stale");

        // Ensure the answer is not negative
        require(answer > 0, "Invalid price data");

        return uint256(answer);
    }

MAX_STALE_INTERVAL can be based on the heartbeat of the feed. The heartbeat refers to the maximum expected time interval between updates to the price data.

Update ChainlinkPriceOracle.sol#AggregatorInterface to support the latestRoundData() method.

interface AggregatorInterface {
        function latestRoundData()
        external
        view
        returns (
            uint80 roundId,
            int256 answer,
            uint256 startedAt,
            uint256 updatedAt,
            uint80 answeredInRound
        );
}

2. If the protocol needs to use a different oracle, a similar price freshness check should be implemented in LTROracle.sol

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #32

[c4-pre-sort]

raymondfam marked the issue as not a duplicate

[c4-pre-sort]

raymondfam marked the issue as duplicate of #843

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid