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

13 stars 11 forks source link

Calls to Oracles don't check for stale prices #136

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/oracles/ChainlinkPriceOracle.sol#L34-L39 https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L116-L144 https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L146-L157 https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L91-L110

Vulnerability details

Impact

Oracle price feeds can become stale due to a variety of reasons. Using a stale price will result in incorrect calculations in the amount of rsETH to mint.

Proof of Concept

When users deposit assets in the LRTDepositPool the calculation of rsethAmountToMint can have wrong values because it uses outdated oracle prices, which results in receiving too much or less rsETH for the user. rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

Tools Used

Manual Review

Recommended Mitigation Steps

latestAnswer() is deprecated, use latestRoundData() instead a add a clear threshold when the prices should be updated (ex. each 2 minutes).

From Chainlink documentation

  (
    /*uint80 roundID*/,
    int256 answer,
    uint256 startedAt,
    /*uint256 updatedAt*/,
    /*uint80 answeredInRound*/
  ) = sequencerUptimeFeed.latestRoundData();

  // Answer == 0: Sequencer is up
  // Answer == 1: Sequencer is down
  bool isSequencerUp = answer == 0;
  if (!isSequencerUp) {
    revert SequencerDown();
  }

  // Make sure the grace period has passed after the
  // sequencer is back up.
  uint256 timeSinceUp = block.timestamp - startedAt;
  if (timeSinceUp <= GRACE_PERIOD_TIME) {
    revert GracePeriodNotOver();
  }

  (
    /*uint80 roundID*/,
    int data,
    /*uint startedAt*/,
    /*uint timeStamp*/,
    /*uint80 answeredInRound*/
   ) = dataFeed.latestRoundData();

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 not a duplicate

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #843

c4-judge commented 11 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid