DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: low
Valid

Function `getBeanTokenPriceFromTwaReserves` will return prices with wrong decimals

Summary

getBeanTokenPriceFromTwaReserves will return wrong price with tokens that does not have 18 decimals

Relevant GitHub Links:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Well/LibWell.sol#L249-L257

Vulnerability Details

The getBeanTokenPriceFromTwaReserves is meant to compute the token price in terms of beans of a token paired with bean in a well. The implementation works as follows:

function getBeanTokenPriceFromTwaReserves(address well) internal view returns (uint256 price) {
AppStorage storage s = LibAppStorage.diamondStorage();
// s.sys.twaReserve[well] should be set prior to this function being called.
// 'price' is in terms of reserve0:reserve1.
if (s.sys.twaReserves[well].reserve0 == 0 || s.sys.twaReserves[well].reserve1 == 0) {
price = 0;
} else {
// fetch the bean index from the well in order to properly return the bean price.
if (getBeanIndexFromWell(well) == 0) {
price = uint256(s.sys.twaReserves[well].reserve0).mul(1e18).div(
s.sys.twaReserves[well].reserve1
);
} else {
price = uint256(s.sys.twaReserves[well].reserve1).mul(1e18).div(
s.sys.twaReserves[well].reserve0
);
}
}
}

As we can see, the computation that is done is reserveOfBeans / reservesOfToken. Because solidity does not support floating point computations if reservesOfToken is greater than reserveOfBeans would return 0. To avoid this scenario, the reserveOfBeans is multiplied by 1e18. This extra decimals are intended to be cancelled with the next division.

For example:
reserveOfBeans = 1000e6
reservesOfToken = 10e18

computedPrice = 1000e6 * 1e18 / 10e18 = 100e6

In this example, the price of the token are 100 beans.

This 1e18 is used because almost all ERC20 have 18 decimals of precision. However, not all of them use this amount of decimals and this can lead to wrong results. Let's take the example of the WBTC on Ethereum chain. This token has 8 decimals of precision. Let's consider the following scenario:

reserveOfBeans = 60000e6
reservesOfWBTC = 1e8

In this scenario 1 WBTC equals 60000 beans, hence the result of the function would need to be 60000e6. However the result of the computation is much higher:

computedPrice = 60000e6 * 1e18 / 1e8 = 600000000000000e6

This extra 10 decimals multiplied make the price much expensive and will return a wrong amount to the protocol.

This function is used when evaluating the price during a sunrise function. Where the well with the largest liquidity is fetch and computed the price.

function evalPrice(int256 deltaB, address well) internal view returns (uint256 caseId) {
AppStorage storage s = LibAppStorage.diamondStorage();
// p > 1
if (deltaB > 0) {
// Beanstalk will only use the largest liquidity well to compute the Bean price,
// and thus will skip the p > EXCESSIVE_PRICE_THRESHOLD check if the well oracle fails to
// compute a valid price this Season.
// deltaB > 0 implies that address(well) != address(0).
uint256 beanTknPrice = LibWell.getBeanTokenPriceFromTwaReserves(well);
if (beanTknPrice > 1) {
uint256 beanUsdPrice = uint256(1e30).div(
LibWell.getUsdTokenPriceForWell(well).mul(beanTknPrice)
);
if (beanUsdPrice > s.sys.seedGaugeSettings.excessivePriceThreshold) {
// p > excessivePriceThreshold
return caseId = 6;
}
}
caseId = 3;
}
// p < 1
}

This makes that if at some point the well with the largest liquidity has a token with more or less than 18 decimals it will return a wrong price and the caseId for the evaluation of the price will be wrong.

Impact

High

Tools Used

Manual review

Recommendations

Multiply the reserves of beans by the amount of decimals of the token:

function getBeanTokenPriceFromTwaReserves(address well) internal view returns (uint256 price) {
AppStorage storage s = LibAppStorage.diamondStorage();
// s.sys.twaReserve[well] should be set prior to this function being called.
// 'price' is in terms of reserve0:reserve1.
if (s.sys.twaReserves[well].reserve0 == 0 || s.sys.twaReserves[well].reserve1 == 0) {
price = 0;
} else {
// fetch the bean index from the well in order to properly return the bean price.
if (getBeanIndexFromWell(well) == 0) {
+ uint8 tokenDecimals = IWell(well).tokens()[1].decimals();
- price = uint256(s.sys.twaReserves[well].reserve0).mul(1e18).div(
+ price = uint256(s.sys.twaReserves[well].reserve0).mul(10 ** tokenDecimals).div(
s.sys.twaReserves[well].reserve1
);
} else {
+ uint8 tokenDecimals = IWell(well).tokens()[0].decimals();
- price = uint256(s.sys.twaReserves[well].reserve1).mul(1e18).div(
+ price = uint256(s.sys.twaReserves[well].reserve1).mul(10 ** tokenDecimals).div(
s.sys.twaReserves[well].reserve0
);
}
}
}
Updates

Lead Judging Commences

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

Appeal created

draiakoo Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`LibWell.getBeanTokenPriceFromTwaReserves()` incorrectly assumes that token has 18 decimals

Support

FAQs

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