Summary
compare to previous version, we can config other people's bdv on the down side, in the mode of Anti-lambda Convert. The problem is that we seems don't
have a limit on the kind of the convert, which means anyone can change other people's bdv to any kind of convert, which is not what we want.
Vulnerability Details
According to the sponoser describe about Anti-Lambda Convert:
- Currently, bdv is an asynchronous to the current bdv of the asset.If a user deposited an LP token at a higher price than now, they currently retain the inflated bdv. This can lead to beanstalk overpaying in stalk/seeds for that certain deposit. An Anti-Lamda Convert is a convert type that allows people to update other people's bdv on the downside. In a pvp enivornment like the evm, you'd expect people to update large positions with a huge BDV. Anti-lamda Converts can only be done on a per-deposit basis, as the ability to merge deposits should not be allowed.
The porblem is , according to the code ,seems that convert type is not limited. Wwe can change other people's bdv, in other modes depite Anti-Lamda Convert, could canuse unexpected result.
function convert(
bytes calldata convertData,
int96[] memory stems,
uint256[] memory amounts
)
external
payable
nonReentrant
returns (int96 toStem, uint256 fromAmount, uint256 toAmount, uint256 fromBdv, uint256 toBdv)
{
uint256 grownStalk;
LibConvert.convertParams memory cp = LibConvert.convert(convertData);
if (cp.decreaseBDV) {require(stems.length == 1 && amounts.length == 1, "Convert: DecreaseBDV only supports updating one deposit.");}
require(cp.fromAmount > 0, "Convert: From amount is 0.");
@> if(cp.account == address(0)) cp.account = msg.sender;
LibSilo._mow(cp.account, cp.fromToken);
if (cp.fromToken != cp.toToken) LibSilo._mow(cp.account, cp.toToken);
(grownStalk, fromBdv) = _withdrawTokens(
cp.fromToken,
stems,
amounts,
cp.fromAmount,
cp.account
);
uint256 newBdv = LibTokenSilo.beanDenominatedValue(cp.toToken, cp.toAmount);
@> toBdv = (newBdv > fromBdv || cp.decreaseBDV) ? newBdv : fromBdv;
toStem = _depositTokensForConvert(cp.toToken, cp.toAmount, toBdv, grownStalk, cp.account);
toAmount = cp.toAmount;
fromAmount = cp.fromAmount;
emit Convert(cp.account, cp.fromToken, cp.toToken, cp.fromAmount, cp.toAmount);
}
Impact
other type of convert can be used to change other people's bdv, which is not what we want,at least, not same as sponsor's description.
Tools Used
manual
Recommendation
Limit the convert type to Anti-Lamda Convert only if account is not msg.sender.
