rETH/ETH chainlink oracle has too long of heartbeat and deviation threshold which can cause unfair minting

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The reliance on the rETH/ETH Chainlink Oracle within the ChainlinkPriceOracle to evaluate the current RsETH token price may lead to inaccurate minting amounts.

Proof of Concept

ChainlinkPriceOracle uses the rETH/ETH chainlink oracle to calculate the current price of the RsETH token. This token valuation is used to determine the amount of RsETH to mint. This is problematic since rETH/ETH has a 24 hour heartbeat and a 2% deviation threshold. This deviation in price could easily cause loss of funds to the user.

    function getRsETHAmountToMint(
        address asset,
        uint256 amount
    )
        public
        view
        override
        returns (uint256 rsethAmountToMint)
    {
        // setup oracle contract
        address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE);
        ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress);

        // calculate rseth amount to mint based on asset amount and asset exchange rate
        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
    }

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

getAssetPrice()uses the rETH/ETH oracle to determine the price which as stated above has a 24 hour hearbeat and 2% deviation threshold, this means that the price can move up to 2% or 24 hours before a price update is triggered. The result is that the on-chain price could be much different than the true rETH price.

This price is used when determining how much RsETH to mint to the user. Since the oracle can be up to 2% different from the true price.

        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

Higher True Price: If the actual price exceeds the feed, minters receive less RsETH than they are entitled to.

Lower True Price: Conversely, if the actual price is lower than the feed, the minter receives more RsETH than they should, leading to unfairness for previous stakers and instant profit for the minter.

https://data.chain.link/ethereum/mainnet/crypto-eth/reth-eth

Tools Used

Manual Review

Recommended Mitigation Steps

Chainlink currently does not support a different rETH data feed.
Consider using other price oracle as alternative option.

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 #609

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory:
Invalid

[Henrychang26]

Thanks for judging @fatherGoose1,

Could you explain the thought process for judging this specific issue and #148?
Both having to do with price feed from Chainlink Oracle.

As stated by the sponsor,

We are okay with using 24 hour stale price as long as it is within price range (2% as specified by chainlink).

Commented by @fatherGoose1

Agree with sponsor. stETH/ETH exchange rates are generally extremely stable

If the price feeds are generally stable, I don't believe slippage will be a significant issue.
I would also like to add that price manipulation attack is possible for this specific instance, as long as its under than 2% within the last 24 hours. Then following is true:

Higher True Price: If the actual price exceeds the feed, minters receive less RsETH than they are entitled to.

Lower True Price: Conversely, if the actual price is lower than the feed, the minter receives more RsETH than they should, leading to unfairness for previous stakers and instant profit for the minter.

Would really appreciate some clarity on the distinction between the 2 issues! Thank again!

[0xnirlin]

I agree with the points raised by @Henrychang26 and as suggested in my report, given the high price of Eth and its derivatives depending upon the amount being deposited loss within the 2% bound can be still pretty significant.

Also regarding the stable exchange rate, just have a look at the stETH exchange rate, in the last 24 hours it fluctuated by .5% and also spikes in stEth exchange rate are common, look at the following chart
https://coinmarketcap.com/currencies/steth/steth/eth/

[c4-judge]

fatherGoose1 marked the issue as duplicate of #584

[c4-judge]

fatherGoose1 marked the issue as satisfactory

[c4-judge]

fatherGoose1 changed the severity to 3 (High Risk)