BDVFacet is subject to Curve LP oracle manipulation via read-only reentrancy
Report ID
#20625
Target
https://github.com/BeanstalkFarms/Beanstalk/pull/337
Report type
Smart Contract
Impacts
Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Has PoC?
Yes
Summary
The function getXP() in LibMetaCurve.sol call to get_virtual_price() which can return a deflated price when it is reentered after calling the Curve pool's remove_liquidity function. This allows an attacker to liquidate accounts that use the pool.
Bug Description
get_virtual_price gives the value of an LP token relative to the pool stable asset by dividing the total value of the pool by the totalSupply() of LP tokens:
In the Curve pool function remove_liquidity when ETH is withdrawn, raw_call() allows the caller to reenter after the coin balances have been updated, but before the LP tokens are burned, so during the callback a reentrant call to get_virtual_price() will return a deflated value.
Impact
An attacker can exploit this by deploying a contract that does this:
get a large amount of ETH through a flashloan add_liquidity with the ETH borrowed Call remove_liquidity: during the callback raw_call() the Curve Oracle LP price is deflated due to large difference between Pool value and LP total supply.
function getXP(
uint256[2] memory balances,
uint256 padding
) internal view returns (uint256[2] memory) {
return LibCurve.getXP(
balances,
padding,
C.curve3Pool().get_virtual_price()
);
}LibMetaCurve.getXP() returns wrong value which is then used in LibBeanMetaCurve.bdv()
The value of price will be erroneous from the computation of getPrice('wrong xp')
uint256 price = LibCurve.getPrice(xp, a, D, RATE_MULTIPLIER);and therefore the value of curveValue,
uint256 curveValue = xp[1].mul(amount).div(totalSupply).div(price);and the return value of bdv()
return beanValue.add(curveValue);Please note that the value of virtualPrice can also be manipulated as it also calls the function get_virtual_price() from curveMetapool().
uint256 virtualPrice = C.curveMetapool().get_virtual_price();The returned value of LibBeanMetaCurve.bdv() is then used in BDVFacet.unripeLPToBDV()
function unripeLPToBDV(uint256 amount) public view returns (uint256) {
amount = LibUnripe.unripeToUnderlying(C.UNRIPE_LP, amount);
amount = LibBeanMetaCurve.bdv(amount);
return amount;
}the return value is then used in the following BDVFacet.bdv() function:
The LP token price is less than the actual price so collateral will be underestimated.
Attacker profits from the liquidations of any account that use this pool.
Execution after raw_call() in remove_liquidity resumes, LP tokens are burned
Repay loan + fee
Note that the function get_virtual_price() is also called in LibIncentive.getRates() twhich can also be manipulated.
function getRates() private view returns (uint256[2] memory) {
// Decimals will always be 6 because we can only mint beans
// 10**(36-decimals)
return [1e30, C.curve3Pool().get_virtual_price()];
}Funds at Risk
A previous proposal in 'Beanstalk Bug Bounty' allows to estimate the value of funds at risk, expecially: '*There's no liquid market for the BEAN3CRV LP token itself, so the current BDV is used to estimate the value. **There's no liquid market for Unripe assets, so the Chop Penalty is used to estimate the value of these tokens.'
The funds at risk in this case is similar as the previous one (BIR-3): 'The number of Beans at risk roughly equates to the value that could have been removed from the BEAN:3CRV liquidity pool.'
I don't have the exact number of tokens as of today but I think the estimated value of funds at risk are in the same range (around $3m), I will let the team to do the math with the right numbers.
Risk Breakdown
Difficulty to Exploit: Medium Weakness: CVSS2 Score:
Recommendation
A solution implemented by many protocols is to add a call to a pool non reentrant function inside the oracle, like this:
function getXP(
uint256[2] memory balances,
uint256 padding
) internal view returns (uint256[2] memory) {
+ uint256[2] calldata amounts;
+ ICurvePool(token).remove_liquidity(0, amounts);
return LibCurve.getXP(
balances,
padding,
C.curve3Pool().get_virtual_price()
);
}Note that the call to remove_liquidity will succeed but won't remove any liquidity if the pool is not reentered. It will revert if the pool is reentered because of the @nonreentrant('lock') decorator on remove_liquidity.
Example of implementation: https://github.com/makerdao/curve-lp-oracle/blob/master/src/CurveLPOracle.sol#L230
References
https://chainsecurity.com/heartbreaks-curve-lp-oracles/
https://forum.balancer.fi/t/reentrancy-vulnerability-scope-expanded/4345
Proof of concept
Below a simulation showing how to achieve the Virtual Price Manipulation. Because of the price manipulation, an attacker can trigger unfair liquidations to his advantage.
BIC Response
The code snippet references this line in Curve: raw_call(msg.sender, b"", value=value). This logic is only used in certain Curve pools, none of which are the metapool template used for the BEAN3CRV pool: https://github.com/search?q=repo%3Acurvefi%2Fcurve-contract%20raw_call(msg.sender%2C%20b%22%22%2C%20value%3Dvalue)&type=code.
Given this, it does not appear to be possible for this vulnerability to surface in standard Curve metapools. The POC also does not call Beanstalk at any point.
Due to these reasons, we are re-closing the submission and no reward will be issued.