Beanstalk: The Finale

Summary

getPercentDifference function of the LibOracleHelpers.sol can give different answers for two same x & y value depending upon the order which is not safe to use.

Vulnerability Details

getPercentDifference takes in two inputs x & y , namely chainlink price & uniswap twap price in that order & if the percentage difference between them is >= 1% than 0 is returned by the oracle using it namely LibWstethEthOracle.sol . But the helper function does not work as intended.
Let’s understand with a example -
Price from feed Y (authentic) = 3000 $ & price from feed X(+1% manipulation) = 3030 $

= 0.01e18 = 0.01e18 so oracle correctly returns 0
So everything works fine but if vice versa happens i.e

Price from feed X (authentic) = 3000 $ & price from feed Y(+1% manipulation) = 3030 $

= 0.0099e18 < 0.01e18 so the trade goes through
the equation will turn out as above but here you will see the value is smaller than 0.01e18 even though the same price manipulation is present and it will pass through the MAX_DIFFERENCE check in LibWstethEthOracle.sol breaking a core functionality and allowing some small price manipulations.
Not to mention the 2nd scenario is easier to achieve for attacker as it requires small manipulation in TWAP & not in chainlink.
Here’s a POC to make things clearer -

So i hope the root cause is clear now i.e only unidirectional calculation of price change . need to do bi direction X to Y and Y to X not just X to Y (as in current code) which leads to inconsistent behaviour and random faliure of wstETH oracle or not failing when it should.

Code snippet-
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/e67707cc081dea948461a24b93be93563f457113/protocol/contracts/libraries/Oracle/LibOracleHelpers.sol#L18-L28

Impact

MAX_DIFFERENCE check fails executing trade at a slightly manipulated price.
Such small deviations will lead to huge arbitrage and loss of value for users.

Tools Used

Manual Review

Recommendations

I recommend deviation check method used by Gamma strategies (though they got rekt but it was due to a admin input error) . i.e find the relative difference of X to Y & Y to X and send the one that is maximum to avoid any inconsistancies in deviation check. (POC also has this to show how the recommended changes work to prevent POC scenario)

Though the issue has been previously reported, its mitigation provided does not prevent the attacker & is incorrect
If the price is manipulated, The provided mitigation bases of percentDifference only one way still , i.e basing on which value is greater but attacker can control that. Hence percentDifference must be calculated two ways always and then there percentage should be compared
instead of comparing the prices first & then just calculating 1 percentage diff.