Summary

The LibChainlinkOracle::getTokenPrice function has a parameter of lookback in order to determine how many seconds ago do we want to obtain the twap of a chainlink price feed. However, this is implemented in a wrong way

Relevant GitHub Links:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol#L39-L48

Vulnerability Details

When LibChainlinkOracle::getTokenPrice is called, it returns a different price calculation depending on the lookback parameter passed to the function:

In this case the ternary operator returns the function getPrice (instantaneous price) when lookback > 0 and getTwap when lookback == 0. As we can see, the conditional for returning the different price computation is wrong because it returns the twap price when lookback = 0, which is basically the instantaneous price and it returns the current price when the lookback parameter is greater than 0, when it should return the twap according to the amount of lookback passed.

The correct behaviour should be that when lookback is set to 0, it should return the instantaneous price "getPrice" and when it is greater than 0 it should return the "getTwap" function passing it the lookback parameter.

Impact

Medium, no matter what lookback will be, the instantaneous price from the chainlink oracle will be returned.

Chainlink is not manipulable but the functionality clearly does not work as intended and can return unexpected results.

Tools Used

Manual review

Recommendations