code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

wrong price reported by `getAssetPrice` and `getRSETHPrice` because of wrong use of chainlink oracle #730

Closed c4-submissions closed 10 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L52-L79 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L45-L47 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/oracles/ChainlinkPriceOracle.sol#L37-L39

Vulnerability details

Vulnerability details:

Details:

the chainlink oracle provides the price of an asset through functions like : latestAnswer which is deprecated, and latestRoundData. these prices are multiplied by a decimal to preserve precision. the decimals function in the price feed tells you the decimals, and you should divide by the decimals before you use the price. the decimals for stETH, cbETH and rETH are 18. so the price reported in latestAnswer is going to be multiplied by 1e18. the price given by chainlink is used without dividing by 1e18 in LRTOracle in the function getAssetPrice, and also in getRSETHPrice. so the price reported by these functions will be inflated by 1e18, which is a problem if somebody wants to get the price of RSETH from the protocol that mints RSETH he will get a price inflated by 1e18. which may also impact it's ability to trade on second markets, expecially that now it has no redeem functionality.

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

Impact:

because this vulnerability impacts external protocols from integrating with kelp protocol because of the inflated price reported, and also impacts users it should be medium severity

see POC

Proof of Concept:

Tools Used:

vscode

Recommended Mitigation Steps:

divide by the decimals of the price feed.

Assessed type

Oracle

c4-pre-sort commented 11 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #32

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as not a duplicate

c4-pre-sort commented 11 months ago

raymondfam marked the issue as primary issue

raymondfam commented 11 months ago

Inadequate proof alleging the price reported in latestAnswer is going to be multiplied by 1e18. No supporting links provided.

c4-judge commented 10 months ago

fatherGoose1 marked the issue as unsatisfactory: Insufficient proof