Summary
The functions to chop and deposit unripe assets into the silo should implement a slippage protection
Relevant GitHub Links:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/silo/SiloFacet/SiloFacet.sol#L43-L59
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/barn/UnripeFacet.sol#L79-L100
Vulnerability Details
When a user has unripe assets and want to convert them to ripe assets he has to call the chop function. This function does the following:
function chop(
address unripeToken,
uint256 amount,
LibTransfer.From fromMode,
LibTransfer.To toMode
) external payable fundsSafu noSupplyChange nonReentrant returns (uint256) {
uint256 supply = IBean(unripeToken).totalSupply();
amount = LibTransfer.burnToken(IBean(unripeToken), amount, LibTractor._user(), fromMode);
(address underlyingToken, uint256 underlyingAmount) = LibChop.chop(
unripeToken,
amount,
supply
);
require(underlyingAmount > 0, "Chop: no underlying");
IERC20(underlyingToken).sendToken(underlyingAmount, LibTractor._user(), toMode);
emit Chop(LibTractor._user(), unripeToken, amount, underlyingAmount);
return underlyingAmount;
}
function LibChop.chop(
address unripeToken,
uint256 amount,
uint256 supply
) internal returns (address underlyingToken, uint256 underlyingAmount) {
AppStorage storage s = LibAppStorage.diamondStorage();
underlyingAmount = LibUnripe._getPenalizedUnderlying(unripeToken, amount, supply);
LibUnripe.decrementUnderlying(unripeToken, underlyingAmount);
underlyingToken = s.sys.silo.unripeSettings[unripeToken].underlyingToken;
}
function _getPenalizedUnderlying(
address unripeToken,
uint256 amount,
uint256 supply
) internal view returns (uint256 redeem) {
require(isUnripe(unripeToken), "not vesting");
uint256 sharesBeingRedeemed = getRecapPaidPercentAmount(amount);
redeem = _getUnderlying(unripeToken, sharesBeingRedeemed, supply);
}
function getRecapPaidPercentAmount(
uint256 amount
) internal view returns (uint256 penalizedAmount) {
AppStorage storage s = LibAppStorage.diamondStorage();
return s.sys.fert.fertilizedIndex.mul(amount).div(s.sys.fert.unfertilizedIndex);
}
function _getUnderlying(
address unripeToken,
uint256 amount,
uint256 supply
) internal view returns (uint256 redeem) {
AppStorage storage s = LibAppStorage.diamondStorage();
redeem = s.sys.silo.unripeSettings[unripeToken].balanceOfUnderlying.mul(amount).div(supply);
}
As we can see, the amount of ripe assets that the user will get in exchange for his unripe assets is as follows:
amountOutRipeAsset = balanceOfUnderlying * fertilizedIndex * amountInUnripeAsset / unfertilizedIndex / totalSupply
As we can see there are a lot of variables that can change at any moment and by any user, so if an unripe token holder expects x amount of ripe assets in exchange of his amount, if someone either chop assets before him or a sunrise function is called and fertilizer indexes change he can get less amount of these ripe assets that he expected. If the function had a slippage protection, if he was going to get less amount of ripe assets, the transaction would revert.
The same think happens when depositing unripe assets into the silo, it needs to compute the value of the deposited assets via the BDV functions from the BDVFacet.
function unripeLPToBDV(uint256 amount) public view returns (uint256) {
amount = LibUnripe.unripeToUnderlying(
C.UNRIPE_LP,
amount,
IBean(C.UNRIPE_LP).totalSupply()
);
amount = LibWellBdv.bdv(LibBarnRaise.getBarnRaiseWell(), amount);
return amount;
}
function unripeBeanToBDV(uint256 amount) public view returns (uint256) {
return
LibUnripe.unripeToUnderlying(C.UNRIPE_BEAN, amount, IBean(C.UNRIPE_BEAN).totalSupply());
}
To compute the value in terms of Beans of the UnripeBeans and UnripeLPs it uses the same formula:
value = balanceOfUnderlying * amount / totalSupply
In the case of LP once it has this value if checks the equivalence of an LP token with a bean using the well.
This formula uses the balanceOfUnderlying and the totalSupply of the unripe asset which basically can change by buying fertilizer or chopping assets. That means that a user can deposit some unripe assets into the silo and if someone chops before him he can get less bdv for his unripe assets than he expected. That's why this function should implement a slippage protection too because it uses variables that are directly manipulable by users. At least in the silo these 2 functions should implement a slippage protection because other assets use a twap so it should not be manipulable.
Impact
Medium, users can get less value than he can expect at the beggining
Tools Used
Manual review
Recommendations
Implement a slippage protection:
function chop(
address unripeToken,
uint256 amount,
+ uint256 minAmountOut,
LibTransfer.From fromMode,
LibTransfer.To toMode
) external payable fundsSafu noSupplyChange nonReentrant returns (uint256) {
// burn the token from the user address
uint256 supply = IBean(unripeToken).totalSupply();
amount = LibTransfer.burnToken(IBean(unripeToken), amount, LibTractor._user(), fromMode);
// get ripe address and ripe amount
(address underlyingToken, uint256 underlyingAmount) = LibChop.chop(
unripeToken,
amount,
supply
);
+ require(underlyingAmount >= minAmountOut, "Slippage protection");
// send the corresponding amount of ripe token to the user address
require(underlyingAmount > 0, "Chop: no underlying");
IERC20(underlyingToken).sendToken(underlyingAmount, LibTractor._user(), toMode);
// emit the event
emit Chop(LibTractor._user(), unripeToken, amount, underlyingAmount);
return underlyingAmount;
}
function deposit(
address token,
uint256 _amount,
+ uint256 minBDV,
LibTransfer.From mode
)
external
payable
fundsSafu
noSupplyChange
noOutFlow
nonReentrant
mowSender(token)
returns (uint256 amount, uint256 _bdv, int96 stem)
{
amount = LibTransfer.receiveToken(IERC20(token), _amount, LibTractor._user(), mode);
- (_bdv, stem) = _deposit(LibTractor._user(), token, amount);
+ (_bdv, stem) = _deposit(LibTractor._user(), token, amount, minBDV);
}
function _deposit(
address account,
address token,
uint256 amount,
+ uint256 minBDV
) internal returns (uint256 stalk, int96 stem) {
GerminationSide side;
(stalk, side) = LibTokenSilo.deposit(
account,
token,
stem = LibTokenSilo.stemTipForToken(token),
amount,
+ minBDV
);
LibSilo.mintGerminatingStalk(account, uint128(stalk), side);
}
// function that fetches bdv
function deposit(
address account,
address token,
int96 stem,
uint256 amount,
+ uint256 minBDV
) internal returns (uint256 stalk, GerminationSide) {
uint256 bdv = beanDenominatedValue(token, amount);
+ require(bdv >= minBDV, "Slippage protection");
return depositWithBDV(account, token, stem, amount, bdv);
}
