init() will fail because the well that should be whitelisted has no liquidityLibWstethEthOracle::getWstethEthPrice returns wrong wstETH/ETH price in some conditions impacting system operationsLibFertilizer:remainingRecapitalization() less fertilizers is mintable than is needed to fully recapitalize the lost LP token value wstETH:ETH price using Chainlinklookback parameter when invoking the getWstethUsdPrice() in the getTokenPrice functioninit() will fail because the well that should be whitelisted has no liquiditySubmitted by BARW.
According to the team “the initial liquidity for the new Bean-wstEth well will be added to the well once the Eth from the old well is converted to wstEth”. The problem is that it will not be possible to initiate the migration to the new well because the init() function will revert if the new well does not have any liquidity in it.
When calling InitMigrateUnripeBeanEthToBeanSteth:init() to initiate the migration from the Bean-ETH well to the Bean-wstETH well, the new well is whitelisted by calling LibWhitelist:whitelistToken(). During this function call the function LibWellBdv:bdv is entered:
function bdv(
address well,
uint amount
) internal view returns (uint _bdv) {
uint beanIndex = LibWell.getBeanIndexFromWell(well);
// For now, assume Beanstalk should always use the first pump and given that the Well has been whitelisted, it should be assumed
// that the first Pump has been verified when the Well was whitelisted.
Call[] memory pumps = IWell(well).pumps();
uint[] memory reserves = IInstantaneousPump(pumps[0].target).readInstantaneousReserves(well, pumps[0].data);
// If the Bean reserve is beneath the minimum balance, the oracle should be considered as off.
require(reserves[beanIndex] >= C.WELL_MINIMUM_BEAN_BALANCE, "Silo: Well Bean balance below min");
Call memory wellFunction = IWell(well).wellFunction();
uint lpTokenSupplyBefore = IWellFunction(wellFunction.target).calcLpTokenSupply(reserves, wellFunction.data);
reserves[beanIndex] = reserves[beanIndex].sub(BEAN_UNIT); // remove one Bean
uint deltaLPTokenSupply = lpTokenSupplyBefore.sub(
IWellFunction(wellFunction.target).calcLpTokenSupply(reserves, wellFunction.data)
);
_bdv = amount.mul(BEAN_UNIT).div(deltaLPTokenSupply);
}
This function gets the reserves of the new Bean-wstETH well that should be whitelisted and checks if the bean reserves of the well are >= WELL_MINIMUM_BEAN_BALANCE which is set to 1000 beans:
uint[] memory reserves = IInstantaneousPump(pumps[0].target).readInstantaneousReserves(well, pumps[0].data);
// If the Bean reserve is beneath the minimum balance, the oracle should be considered as off.
require(reserves[beanIndex] >= C.WELL_MINIMUM_BEAN_BALANCE, "Silo: Well Bean balance
uint256 internal constant WELL_MINIMUM_BEAN_BALANCE = 1000_000_000; // 1,000 Beans
The issue arises from the fact that at the time the well is whitelisted it does not have any liquidity in it because according to the development team “the initial liquidity will be added to the well once the Eth is converted to wstEth”. This means the function will revert and therefore it will not be possible to initiate the migration.
Do not whitelist the well when calling InitMigrateUnripeBeanEthToBeanSteth:init() but add a second function InitMigrateUnripeBeanEthToBeanSteth:finish() where the LP tokens for the new well are added and the new well which now has enough liquidity is whitelisted.
LibWstethEthOracle::getWstethEthPrice returns wrong wstETH/ETH price in some conditions impacting system operationsSubmitted by KiteWeb3, holydevoti0n, Bauchibred, zhuying, 0xBeastBoy, bladesec, 0xTheBlackPanther. Selected submission by: KiteWeb3.
The LibWstethEthOracle::getWstethEthPrice function is designed in the system to compute the wstETH/ETH price . On the top of the LibWstethEthOracle contract a detailed NatSpec describes the price computation logic. Reported here for clarity: "It then computes a wstETH:ETH price by taking the minimum of (3) and either the average of (1) and (2) if (1) and (2) are within MAX_DIFFERENCE from each other or (1)."
According to the NatSpec, the contract should compute the wstETH:ETH price by taking the minimum of the the redemption value or the average of the Chainlink and Uniswap oracle prices if their percent difference is within a specified threshold (MAX_DIFFERENCE) or the Chainlink oracle price if the percent difference exceeds this threshold. However, the actual implementation does not handle the scenario where the percent difference exceeds MAX_DIFFERENCE. Consequently, users interacts with the system, such as minting fertilizer tokens, using inaccurate price data.
The LibWstethEthOracle::getWstethEthPrice lacks explicit handling for scenarios where the percent difference between the Chainlink and Uniswap oracle prices is greater then MAX_DIFFERENCE. This omission leads to situations where the contract does not default to the Chainlink price as intended, affecting the accuracy and reliability of the wstETH:ETH price computation.
/**
* @title Wsteth Eth Oracle Library
* @author brendan
* @notice Computes the wstETH:ETH price.
* @dev
* The oracle reads from 4 data sources:
* a. wstETH:stETH Redemption Rate: (0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0)
* b. stETH:ETH Chainlink Oracle: (0x86392dC19c0b719886221c78AB11eb8Cf5c52812)
* c. wstETH:ETH Uniswap Pool: (0x109830a1AAaD605BbF02a9dFA7B0B92EC2FB7dAa)
* d. stETH:ETH Redemption: (1:1)
*
* It then computes the wstETH:ETH price in 3 ways:
* 1. wstETH -> ETH via Chainlink: a * b
* 2. wstETH -> ETH via wstETH:ETH Uniswap Pool: c * 1
* 3. wstETH -> ETH via stETH redemption: a * d
*
@> * It then computes a wstETH:ETH price by taking the minimum of (3) and either the average of (1) and (2)
@> * if (1) and (2) are within `MAX_DIFFERENCE` from each other or (1).
**/
function getWstethEthPrice(uint256 lookback) internal view returns (uint256 wstethEthPrice) {
uint256 chainlinkPrice = lookback == 0 ?
LibChainlinkOracle.getPrice(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT) :
LibChainlinkOracle.getTwap(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT, lookback);
// Check if the chainlink price is broken or frozen.
if (chainlinkPrice == 0) return 0;
uint256 stethPerWsteth = IWsteth(C.WSTETH).stEthPerToken();
chainlinkPrice = chainlinkPrice.mul(stethPerWsteth).div(CHAINLINK_DENOMINATOR);
// Uniswap V3 only supports a uint32 lookback.
if (lookback > type(uint32).max) return 0;
uint256 uniswapPrice = LibUniswapOracle.getTwap(
lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES :
uint32(lookback),
WSTETH_ETH_UNIV3_01_POOL, C.WSTETH, C.WETH, ONE
);
// Check if the uniswapPrice oracle fails.
if (uniswapPrice == 0) return 0;
@> if (LibOracleHelpers.getPercentDifference(chainlinkPrice, uniswapPrice) < MAX_DIFFERENCE) {
@> wstethEthPrice = chainlinkPrice.add(uniswapPrice).div(AVERAGE_DENOMINATOR);
@> if (wstethEthPrice > stethPerWsteth) wstethEthPrice = stethPerWsteth;
@> wstethEthPrice = wstethEthPrice.div(PRECISION_DENOMINATOR);
}
}
}
The absence of the missing return of the Chainlink oracle price in scenarios of significant price discrepancy between the Chainlink and Uniswap oracles (LibOracleHelpers.getPercentDifference(chainlinkPrice, uniswapPrice) > MAX_DIFFERENCE) can lead to a scenario where the contract uses an average price that does not accurately reflect market conditions. The smart contract will operate with an inaccurate wstETH:ETH price, impacting operations dependent on this price. This could result in financial losses for users and undermine the integrity of the system.
For example, in the beanstalk system, the FertilizerFacet::mintFertilizer function relies on the LibWstethEthOracle::getWstethEthPrice to fetch the wstETH:ETH price from. This price is crucial for calculating the amount of Fertilizer tokens that can be acquired with the provided tokenAmountIn. However, if this function returns an inaccurate price, it would not reflect the actual price of the asset. Consequently, users could continue to mint fertilizer tokens using this inaccurate price data, leading to transactions occurring at incorrect prices.
Manual review
Modify the LibWstethEthOracle::getWstethEthPrice function to include explicit logic for handling the case where the percent difference between the Chainlink and Uniswap prices is greater then MAX_DIFFERENCE.
if (LibOracleHelpers.getPercentDifference(chainlinkPrice, uniswapPrice) < MAX_DIFFERENCE) {
wstethEthPrice = chainlinkPrice.add(uniswapPrice).div(AVERAGE_DENOMINATOR);
+ } else {
+ wstethEthPrice = chainlinkPrice;
+ }
if (wstethEthPrice > stethPerWsteth) wstethEthPrice = stethPerWsteth;
wstethEthPrice = wstethEthPrice.div(PRECISION_DENOMINATOR);
- }
Submitted by Bauchibred, bladesec. Selected submission by: Bauchibred.
The protocol exhibits a disparity in the calculation of price differences due to the current implementation in LibOracleHelpers::getPercentDifference(). This affects LibWstethEthOracle::getWstethEthPrice(), since the present implementation causes unintended returns of 0 for price queries. The discrepancy arises from the flawed calculation logic as explained in the report, causing the protocol to incorrectly assume price manipulation in certain valid scenarios. This flaw undermines the reliability of price queries and cwould result in denial-of-service unintended implementation
First take a look at https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibOracleHelpers.sol#L25-L32
function getPercentDifference(
uint x,
uint y
) internal pure returns (uint256 percentDifference) {
percentDifference = x.mul(ONE).div(y);
percentDifference = x > y ? percentDifference - ONE : ONE - percentDifference; // SafeMath unnecessary due to conditional check
}
}
This function is used to get the percent difference between two values, keep in mind that "values" in this context are "prices" provided by different oracle sources, protocol hardcodes the numerator and denominator of getting the percentage as x and y respectively x.mul(ONE).div(y), but evidently we can see that the percentage of the price difference is finalized depending on if x > y or not, causing the equation to be flawed, cause percentDifference - ONE & ONE - percentDifference would yield different results in the context for two same x and y price.
To deduce the differences in the formular, we can see that
x > y then the percentDifference is gotten as ( ( (1e18 * X ) / Y ) - 1e18), howeverx < y then the percentDifference is gotten as (1e18 - ( ( 1e18 * X ) / Y ) )Using generic values of 5 and 6... and assuming 1e18 as 1000 just for POC sake, i.e instances of 1e18 * X is now 1000X.
For the first case (X = 5, Y = 6), we get a percentDifference of 166,67 ~ 16,7% For the second case (X = 6, Y = 5), we get a percentDifference of 200 = 20%
Now, take a look at the only instance where this check is implemented in scope, https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol#L68-L98
function getWstethEthPrice(uint256 lookback) internal view returns (uint256 wstethEthPrice) {
uint256 chainlinkPrice = lookback == 0 ?
LibChainlinkOracle.getPrice(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT) :
LibChainlinkOracle.getTwap(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT, lookback);
// Check if the chainlink price is broken or frozen.
if (chainlinkPrice == 0) return 0;
uint256 stethPerWsteth = IWsteth(C.WSTETH).stEthPerToken();
chainlinkPrice = chainlinkPrice.mul(stethPerWsteth).div(CHAINLINK_DENOMINATOR);
// Uniswap V3 only supports a uint32 lookback.
if (lookback > type(uint32).max) return 0;
uint256 uniswapPrice = LibUniswapOracle.getTwap(
lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES :
uint32(lookback),
WSTETH_ETH_UNIV3_01_POOL, C.WSTETH, C.WETH, ONE
);
// Check if the uniswapPrice oracle fails.
if (uniswapPrice == 0) return 0;
//@audit `getPercentDifference` is queried with `MAX_DIFFERENCE`
if (LibOracleHelpers.getPercentDifference(chainlinkPrice, uniswapPrice) < MAX_DIFFERENCE) {
wstethEthPrice = chainlinkPrice.add(uniswapPrice).div(AVERAGE_DENOMINATOR);
if (wstethEthPrice > stethPerWsteth) wstethEthPrice = stethPerWsteth;
wstethEthPrice = wstethEthPrice.div(PRECISION_DENOMINATOR);
}
}
This function is used to get the wstEth price, but the MAX_DIFFERENCE check is applied here and in a case where the price discrepancy is more than MAX_DIFFERENCE this function returns 0.
The above explained case means that the current implementation of getPercentDifference would cause protocol assuming the price has been manipulated for the same difference just dependent on if the chainlink price is higher/lower than the uniswap returned price, causing 0 to be returned in some valid cases and considering the fact that the MAX_DIFFERENCE has been set very low, this would frequently occur, to use a real life case a sa POC, as at the time of writing this report, the value of wstEth in comparison to eth is 1,16323, so assuming the queried price X from chainlink returns 1,17490, and uniswap price Y returns 1,16323, the getPercentDifference() check fails, since the difference in percentage is = 1,0032409755594 * 1e18 i.e > MAX_DIFFERENCE, however if we swap which provider provides which, i.e now chainlink X returns 1,16323 and uniswap price Y returns 1,17490, the getPercentDifference() check passes since the difference in percentage would be = 0.99327602349136 * 1e18 i.e < MAX_DIFFERENCE proving the disparity and this calculation wstethEthPrice = chainlinkPrice.add(uniswapPrice).div(AVERAGE_DENOMINATOR); would be the same for the two cases irrespective of (x > y)? i.e in our highlighted case above for both instances the calculation would equal ((1,17490, + 1,16323 ) / 2) = 1,169065 and should be considered valid in both cases.
Desynchronization of price queries and this directly affects all instances in protocol where LibWstethEthOracle.sol's getWstethEthPrice() is called, i.e in two same cases where the provided prices are the same, the attempt to query the price correctly works with one where as it doesn't with the other, leading to unintended return of 0 for price essentially a DOS to LibWstethEthOracle.sol's getWstethEthPrice() since protocol would now assume that the price has been manipulated, keep in mind that in real sense this calculation wstethEthPrice = chainlinkPrice.add(uniswapPrice).div(AVERAGE_DENOMINATOR); would always return the same value for both cases and as such the price should be valid and accepted in both cases.
Manual review
Make the check to always be in sync, this can be by ensuring that the divisor is always the lower/greater price and then maybe reimplementing the value of MAX_DIFFERENCE to take this into consideration, the former alone is sufficient, i.e reimplement https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibOracleHelpers.sol#L25-L32 to:
function getPercentDifference(
uint x,
uint y
) internal pure returns (uint256 percentDifference) {
if (x == y) {
percentDifference = 0;
} else if (x < y) {
percentDifference = x.mul(ONE).div(y);
percentDifference = ONE - percentDifference;
} else {
percentDifference = y.mul(ONE).div(x);
percentDifference = ONE - percentDifference;
}
return percentDifference;
}
Here we are always using the bigger price as the denominator, thereby making sure that in whichever of the two cases explained in report, i.e if x > y or not a fixed percentDifference is provided and this can then be accurately checked against protocol's set MAX_DIFFERENCE value.
LibFertilizer:remainingRecapitalization() less fertilizers is mintable than is needed to fully recapitalize the lost LP token valueSubmitted by BARW.
There is a division before multiplication at LibFertilizer:remainingRecapitalisation(), which will cause the protocol to enable the mint of less fertilizer than intended. This will result in less value recapitalized than lost and therefor loss of value for the holders of UNRIPE_LP tokens.
When calling LibFertilizer::remainingRecapitalization() the amount of totalDollars this calculated that still need to be recapitalized:
uint256 totalDollars = C.dollarPerUnripeLP().mul(C.unripeLP().totalSupply()).div(DECIMALS);
For this the function C.dollarPerUnripeLP is used invoking a division operation before the following multiplication, leading to precision loss:
function dollarPerUnripeLP() internal pure returns (uint256) {
return 1e12/UNRIPE_LP_PER_DOLLAR;
}
Add this file to test folder and run forge test -vv --mt test_precision_loss
// SPDX-License-Identifier: MIT
pragma solidity =0.7.6;
import {SafeMath} from "openzeppelin/utils/math/SafeMath.sol";
import "forge-std/Test.sol";
import "forge-std/console.sol";
contract TestPrecisionLoss is Test {
using SafeMath for uint256;
function setUp() public {}
function test_precision_loss() external {
uint256 UNRIPE_LP_TOTAL_SUPPLY = 96033202232306;
uint128 DECIMALS = 1e6;
uint256 UNRIPE_LP_PER_DOLLAR = 1884592; // 145_113_507_403_282 / 77_000_000
uint256 dollarPerUnripeLP = 1e12 / UNRIPE_LP_TOTAL_SUPPLY;
uint256 totalDollars1 = (1e12 / UNRIPE_LP_PER_DOLLAR)
.mul(UNRIPE_LP_TOTAL_SUPPLY)
.div(DECIMALS);
console.log("totalDollars1: ", totalDollars1);
uint256 totalDollars2 = (1e12 * UNRIPE_LP_TOTAL_SUPPLY)
.div(UNRIPE_LP_PER_DOLLAR)
.div(DECIMALS);
console.log("totalDollars2: ", totalDollars2);
console.log("the diff: ", totalDollars2 - totalDollars1);
}
}
The output of the test is the following:
Running 1 test for test/testPrecisionLoss.sol:TestPrecisionLoss
[PASS] test_precision_loss() (gas: 5691)
Logs:
totalDollars1: 50956945702101
totalDollars2: 50957025304313
the diff: 79602212
remainingRecapitalization() will always return lesser value than it should be. This will lead to less fertilizer available for mint and therefore less USD value that will get recapitalized for the UNRIPE_LP holders. Here is the related code in FertilizerFacet::mintFertilizer():
uint128 remaining = uint128(LibFertilizer.remainingRecapitalization().div(1e6));// remaining <= 77_000_000 so downcasting is safe.
require(fertilizerAmountOut <= remaining, "Fertilizer: Not enough remaining.");
Foundry
--uint256 totalDollars = C.dollarPerUnripeLP().mul(C.unripeLP().totalSupply()).div(DECIMALS);
++uint256 totalDollars = (1e12).mul(C.unripeLP().totalSupply()).div(C.unripeLPPerDollara()).div(DECIMALS);
Submitted by holydevoti0n.
The protocol will migrate the Bean:WETH Well LP to Bean:wsETH Well LP after initializing the bip migration:
InitMigrateUnripeBeanEthToBeanSteth -> LibFertilizer -> beginBarnRaiseMigration -> switchUnderlyingToken
LibUnripe.switchUnderlyingToken(C.UNRIPE_LP, well);
This will change the underlying token of C.UNRIPE_LP to the new Bean:wsETH pool.
Whenever getBarnRaiseWell uses a fallback underlyingToken, the correct pool to be returned should be the new one added Bean:wsETH not Bean:WETH. But currently, the Bean:WETH pool is used.
As the getBarnRaiseWell is used in several areas of the protocol like:
LibConvert)Whenever the fallback underlyingToken is used it will completely break the protocol logic as Bean:WETH is not the current underlying token after the migration.
Add the following test inside BeanEthToBeanWstethMigration.test.js -> 'Initializes migration'
describe('When the fallback unlderyingToken is used', async function () {
it('should return valid fallback token', async function () {
await this.beanstalk.connect(owner).switchUnderlyingToken(UNRIPE_LP, ethers.constants.AddressZero)
expect(await this.beanstalk.getBarnRaiseToken()).to.be.equal(WSTETH)
})
})
Output:
21 passing (19s)
1 failing
1) Bean:Eth to Bean:Wsteth Migration
Initializes migration
When the fallback unlderyingToken is used
should return valid fallback token:
AssertionError: expected '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C7…' to equal '0x7f39C581F595B53c5cb19bD0b3f8dA6c935…'
+ expected - actual
-0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2
+0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0
Hardhard & Manual Review
Add the C.BEAN_WSTETH_WELL as a fallback for the underlying token on LibBarnRaise. Also, ensure C.BEAN_WSTETH_WELL will have the correct address.
return
s.u[C.UNRIPE_LP].underlyingToken == address(0)
- ? C.BEAN_ETH_WELL
+ ? C.BEAN_WSTETH_WELL
: s.u[C.UNRIPE_LP].underlyingToken;
wstETH:ETH price using ChainlinkSubmitted by holydevoti0n, InAllHonesty, 0xBeastBoy. Selected submission by: InAllHonesty.
The chosen stETH:ETH Chainlink Oracle has a huge heartbeat, which exposes the protocol to unnecessary risk that could be easily mitigated by chosing another path of computing the same price with two different Chainlink Oracles that have a better heartbeat.
According to the handy comments in LibWstethEthOracle.sol the price ofwstETH:ETH is computed as follows:
The oracle reads from 4 data sources:
a. wstETH:stETH Redemption Rate
b. stETH:ETH Chainlink Oracle
c. wstETH:ETH Uniswap Pool
d. stETH:ETH Redemption (1:1)
It then computes the wstETH:ETH price in 3 ways:
1. wstETH -> ETH via Chainlink: a * b
2. wstETH -> ETH via wstETH:ETH Uniswap Pool: c * 1
3. wstETH -> ETH via stETH redemption: a * d
Looking at the feed details on Chainlink's Price Feed page we can see the following details:
Pair: STETH / ETH
Deviation 0.5%
Heartbeat 86400s
Decimals 18
On the same page we find the following:
Pair: STETH / USD
Deviation 1%
Heartbeat 3600s
Decimals 8
Pair: ETH / USD
Deviation 0.5%
Heartbeat 3600s
Decimals 8
Changing the way the Chainlink price is computed from a * b to a * (STETH / USD) * 1/(ETH / USD)(adjusted for decimals) would yield an overall heartbeat of 3600s (1 hour) vs the existing one of 86400s (1 day).
A similar finding is available here.
Moreover there's a strong chance this was the intention of the developer given that the RFC doesn't specify the stETH:ETH Chainlink Oracle but specifies the stETH:USD Chainlink Oracle and a different method of computing the wstETH:ETH price.
Protocol could use inaccurate prices, or at least could benefit from a more accurate price feed in case the proposed changed is implemented.
Likelihood: Low to Extremely Low
Impact: The consumption of stale prices is usually Medium-High depending on how bad the consumed price is.
Overall I consider the severity Low.
Manual review
Implement the a * (STETH / USD) * 1/(ETH / USD)(adjusted for decimals) instead of wstETH -> ETH via Chainlink: a * b used currently.
Submitted by IvanFitro, bladesec. Selected submission by: IvanFitro.
getBeanAmountOut() incorrectly uses C.UNRIPE_BEAN.totalSupply() instead of C.UNRIPE_LP.totalSupply() to calculate the BEAN amount, resulting in an incorrect calculation.
getBeanAmountOut() calculates the amount of BEAN tokens a user would receive per BEAN:3CRV LP provided.
function getBeanAmountOut(uint256 amountIn)
internal
view
returns (uint256 bean)
{
uint256 lp = LibUnripe.unripeToUnderlying(
C.UNRIPE_LP,
amountIn,
@> IBean(C.UNRIPE_BEAN).totalSupply()
);
bean = LibWellConvert.getBeanAmountOut(LibBarnRaise.getBarnRaiseWell(), lp);
bean = LibUnripe
.underlyingToUnripe(C.UNRIPE_BEAN, bean)
.mul(LibUnripe.percentBeansRecapped())
.div(LibUnripe.percentLPRecapped());
}
As observed, the calculation in getBeanAmountOut() mistakenly utilizes IBean(C.UNRIPE_BEAN).totalSupply() to determine the LP. However, this line calculates the LP not the BEAN token amount. The correct implementation is using IBean(C.UNRIPE_LP).totalSupply() to next calcualte correctly the desired BEAN token quantity.
The BEAN token amount obtained is incorrect.
Manual review.
Change IBean(C.UNRIPE_BEAN).totalSupply() for IBean(C.UNRIPE_LP).totalSupply() .
function getBeanAmountOut(uint256 amountIn)
internal
view
returns (uint256 bean)
{
uint256 lp = LibUnripe.unripeToUnderlying(
C.UNRIPE_LP,
amountIn,
- IBean(C.UNRIPE_BEAN).totalSupply()
+ IBean(C.UNRIPE_LP).totalSupply()
);
bean = LibWellConvert.getBeanAmountOut(LibBarnRaise.getBarnRaiseWell(), lp);
bean = LibUnripe
.underlyingToUnripe(C.UNRIPE_BEAN, bean)
.mul(LibUnripe.percentBeansRecapped())
.div(LibUnripe.percentLPRecapped());
}
lookback parameter when invoking the getWstethUsdPrice() in the getTokenPrice functionSubmitted by mrMorningstar, 0xbug, bladesec, KupiaSec, ge6a. Selected submission by: 0xbug.
The getWstethUsdPrice() function is being called without using the lookback parameter if it's the WSTETH token.
The function uses a constant value of 0 for the lookback parameter when calling LibWstethUsdOracle.getWstethUsdPrice().
So it always returns the current spot price for wstETH.
function getTokenPrice(address token, uint256 lookback) internal view returns (uint256) {
if (token == C.WETH) {
uint256 ethUsdPrice = LibEthUsdOracle.getEthUsdPrice(lookback);
if (ethUsdPrice == 0) return 0;
return ethUsdPrice;
}
if (token == C.WSTETH) {
uint256 wstethUsdPrice = LibWstethUsdOracle.getWstethUsdPrice(0); // @audit missing lookback?
if (wstethUsdPrice == 0) return 0;
return wstethUsdPrice;
}
revert("Oracle: Token not supported.");
}
It's always returning the current price instead of TWAP for wstETH.
This could lead to inaccurate calculations in calling this getTokenPrice for wstETH.
Manual review
It's recommended to use the lookback parameter instead of 0.
uint256 wstethUsdPrice = LibWstethUsdOracle.getWstethUsdPrice(lookback);