Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Using the `decimals()` selector for verifying a Chainlink oracle can not be exclusive to price feed contracts

draiakoo

Summary

Using the decimals() selector for verifying a Chainlink oracle can not be exclusive to price feed contracts

Relevant GitHub Links:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Silo/LibWhitelist.sol#L453

Vulnerability Details

When a token is whitelisted to the Silo, a function staticall is executed to proof that the oracle implementation is compatible with the type of oracle the contract expects. To do that there are 3 types of oracle:

/**
* @notice Verifies whether a gaugePointSelector at an external contract
* is valid for the gauge system.
*/
function verifyOracleImplementation(
address oracleImplementation,
bytes4 selector,
bytes1 encodeType
) internal view {
bool success;
// if the encode type is 0x01, verify using the chainlink implementation.
if (encodeType == bytes1(0x01)) {
(success, ) = oracleImplementation.staticcall(
abi.encodeWithSelector(IChainlinkAggregator.decimals.selector)
);
} else if (encodeType == bytes1(0x02)) {
// 0x0dfe1681 == token0() for uniswap pools.
(success, ) = oracleImplementation.staticcall(abi.encodeWithSelector(0x0dfe1681));
} else {
// verify you passed in a callable oracle selector
(success, ) = oracleImplementation.staticcall(abi.encodeWithSelector(selector, 0));
}
​
require(success, "Whitelist: Invalid Oracle Implementation");
}

We can see that the different types of oracles are Chainlink price feed, Uniswap pool and a custom oracle implementation.
In the case of Uniswap pool it calls the function token0() which is actually a pretty specific function to a Uniswap pool. However for the chainlink price feed it calls the function decimals(). The problem with this function is that is a pretty common function that a lot of contracts can implement, for example ERC20s. For this reason, if for example an address of an ERC20 is passed as the oracle implementation when whitelisting a token, the call success. However, when Beanstalk will try to fetch a price, the transaction will revert.
The optimal function to ensure that a contract is a chainlink oracle would be the latestRoundData() function because it is still a view function which can be accessed with a staticall and it is exclusive for chainlink price feeds.

Impact

Medium, a mistake can be made easily when whitelisting a token and passing an address of an ERC20 as an oracle implementation and it will not revert

Tools Used

Manual review

Recommendations

Use instead the latestRoundData() function selector to verify chainlink price feeds.

/**
* @notice Verifies whether a gaugePointSelector at an external contract
* is valid for the gauge system.
*/
function verifyOracleImplementation(
address oracleImplementation,
bytes4 selector,
bytes1 encodeType
) internal view {
bool success;
// if the encode type is 0x01, verify using the chainlink implementation.
if (encodeType == bytes1(0x01)) {
(success, ) = oracleImplementation.staticcall(
- abi.encodeWithSelector(IChainlinkAggregator.decimals.selector)
+ abi.encodeWithSelector(IChainlinkAggregator.latestRoundData.selector)
);
} else if (encodeType == bytes1(0x02)) {
// 0x0dfe1681 == token0() for uniswap pools.
(success, ) = oracleImplementation.staticcall(abi.encodeWithSelector(0x0dfe1681));
} else {
// verify you passed in a callable oracle selector
(success, ) = oracleImplementation.staticcall(abi.encodeWithSelector(selector, 0));
}
​
require(success, "Whitelist: Invalid Oracle Implementation");
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational/Gas

Invalid as per docs https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

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