Vulnerability Details
This function loops through stems until it withdraws amount maxTokens. As you can see it skips germinating stems. Problem is that it doesn't reset amount of germination stem.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/Convert/LibConvert.sol#L466-L469
function _withdrawTokens(
address token,
int96[] memory stems,
uint256[] memory amounts,
uint256 maxTokens
) internal returns (uint256, uint256) {
...
{
a.bdvsRemoved = new uint256[](stems.length);
a.stalksRemoved = new uint256[](stems.length);
a.depositIds = new uint256[](stems.length);
LibGerminate.GermStem memory germStem = LibGerminate.getGerminatingStem(token);
while ((i < stems.length) && (a.active.tokens < maxTokens)) {
...
@> if (germStem.germinatingStem <= stems[i]) {
i++;
continue;
}
...
}
@> for (i; i < stems.length; ++i) amounts[i] = 0;
@> emit RemoveDeposits(user, token, stems, amounts, a.active.tokens, a.bdvsRemoved);
@> emit LibSilo.TransferBatch(user, user, address(0), a.depositIds, amounts);
}
...
}
Suppose a scenario:
User submits stems = [germinatingStem, germinatingStem, oldStem].
First 2 values will be skipped in cycle, and last works correctly.
for cycle won't clear amounts of those germinating stems because they were looped.
As a result LibSilo.TransferBatch will be emitted with arbitrary amount and 0 tokenId.
Impact
Events RemoveDeposits and TransferBatch will be emitted with incorrect values.
Tools Used
Manual Review
Recommendations
Reset amount before skipping germination stem, and set depositId in advance:
function _withdrawTokens(
address token,
int96[] memory stems,
uint256[] memory amounts,
uint256 maxTokens
) internal returns (uint256, uint256) {
...
// a bracket is included here to avoid the "stack too deep" error.
{
a.bdvsRemoved = new uint256[](stems.length);
a.stalksRemoved = new uint256[](stems.length);
a.depositIds = new uint256[](stems.length);
// get germinating stem and stemTip for the token
LibGerminate.GermStem memory germStem = LibGerminate.getGerminatingStem(token);
while ((i < stems.length) && (a.active.tokens < maxTokens)) {
+ a.depositIds[i] = uint256(LibBytes.packAddressAndStem(token, stems[i]));
...
if (germStem.germinatingStem <= stems[i]) {
i++;
+ amounts[i] = 0;
continue;
}
...
- a.depositIds[i] = uint256(LibBytes.packAddressAndStem(token, stems[i]));
i++;
}
for (i; i < stems.length; ++i) amounts[i] = 0;
emit RemoveDeposits(user, token, stems, amounts, a.active.tokens, a.bdvsRemoved);
emit LibSilo.TransferBatch(user, user, address(0), a.depositIds, amounts);
}
...
}
