Beanstalk Part 2
Beanstalk
DeFiHardhat
35,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`LibUniswapOracle.consult()` does not handle failure as required

tigerfrake

Summary

According to comments , the LibUniswapOracle.consult() is:

A variation of {OracleLibrary.consult} that returns just the arithmetic mean tick and returns 0 on failure instead of reverting if {IUniswapV3Pool.observe} reverts.

However, the function itself does not live up to this as it does not return 0 on failure.

Vulnerability Details

  • https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol#L41-L62

function consult(address pool, uint32 secondsAgo)
internal
view
returns (bool success, int24 arithmeticMeanTick)
{
require(secondsAgo != 0, 'BP');
​
​
uint32[] memory secondsAgos = new uint32[](2);
secondsAgos[0] = secondsAgo;
secondsAgos[1] = 0;
​
​
try IUniswapV3Pool(pool).observe(secondsAgos) returns (
int56[] memory tickCumulatives,
uint160[] memory
) {
int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0];
arithmeticMeanTick = int24(tickCumulativesDelta / secondsAgo);
// Always round to negative infinity
if (tickCumulativesDelta < 0 && (tickCumulativesDelta % secondsAgo != 0)) arithmeticMeanTick--;
success = true;
} catch {}
}

If the observation is successful (i.e., the try block executes without error), the function calculates the arithmetic mean tick using the tick cumulatives obtained from the observation.

If the observation fails (i.e., the try block encounters an error), the function catches the error without reverting. This is indicated by the empty catch block {}.
In case of failure, the function does not provide any specific handling or return value, leaving success as false and arithmeticMeanTick uninitialized.

0 is only returned when the consult() function is used within another function with a success boolean set to it such as in getTwap():

  • https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol#L30-L34

function getTwap(uint32 lookback, address pool, address token1, address token2, uint128 oneToken) internal view returns (uint256 price) {
(bool success, int24 tick) = consult(pool, lookback);
if (!success) return 0;
price = OracleLibrary.getQuoteAtTick(tick, oneToken, token1, token2);
}

Therefore, the consult() as it is does not return 0 as the comments suggest on its own.

Impact

Without specific handling or return values for failure cases, the behavior of the function becomes ambiguous. It's unclear what to expect when an error occurs.

Tools Used

Manual Review

Recommendations

To ensure clarity and consistency in the consult function's behavior, modify it to return 0 when the observation fails.

function consult(address pool, uint32 secondsAgo)
internal
view
returns (int24 arithmeticMeanTick)
{
require(secondsAgo != 0, 'BP');
​
uint32[] memory secondsAgos = new uint32[](2);
secondsAgos[0] = secondsAgo;
secondsAgos[1] = 0;
​
try IUniswapV3Pool(pool).observe(secondsAgos) returns (
int56[] memory tickCumulatives,
uint160[] memory
) {
int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0];
arithmeticMeanTick = int24(tickCumulativesDelta / secondsAgo);
// Always round to negative infinity
if (tickCumulativesDelta < 0 && (tickCumulativesDelta % secondsAgo != 0)) arithmeticMeanTick--;
return arithmeticMeanTick;
} catch {
return 0; // @audit If observation fails, return 0
}
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Informational/Invalid

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.