Beanstalk Part 1
Beanstalk
DeFiHardhatOracleProxyUpdates
100,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Missing validation on stems and amounts in `_removeDepositsFromAccount`

0xbeastboy

Summary

The issue in the LibSilo:_removeDepositsFromAccount function is that there is a lack of validation for the length of the stems and amounts arrays, which could potentially lead to unexpected behavior or errors if these arrays are not of equal length. Additionally, there is a missing check to ensure that the stems array has at least one element before proceeding with the loop.

Vulnerability Details

See the following code:

function _removeDepositsFromAccount(
address account,
address token,
int96[] calldata stems,
uint256[] calldata amounts
) internal returns (AssetsRemoved memory ar) {
AppStorage storage s = LibAppStorage.diamondStorage();
uint256[] memory bdvsRemoved = new uint256[](stems.length);
uint256[] memory removedDepositIDs = new uint256[](stems.length);
LibGerminate.GermStem memory germStem = LibGerminate.getGerminatingStem(token);
for (uint256 i; i < stems.length; ++i) {
LibGerminate.Germinate germState = LibGerminate._getGerminationState(stems[i], germStem);
uint256 crateBdv = LibTokenSilo.removeDepositFromAccount(account, token, stems[i], amounts[i]);
bdvsRemoved[i] = crateBdv;
removedDepositIDs[i] = LibBytes.packAddressAndStem(token, stems[i]);
uint256 crateStalk = stalkReward(stems[i], germStem.stemTip, crateBdv.toUint128());
​
// if the deposit is germinating, decrement germinating values,
// otherwise increment deposited values.
if (germState == LibGerminate.Germinate.NOT_GERMINATING) {
ar.active.bdv = ar.active.bdv.add(crateBdv);
ar.active.stalk = ar.active.stalk.add(crateStalk);
ar.active.tokens = ar.active.tokens.add(amounts[i]);
} else {
if (germState == LibGerminate.Germinate.ODD) {
ar.odd.bdv = ar.odd.bdv.add(crateBdv);
ar.odd.tokens = ar.odd.tokens.add(amounts[i]);
} else {
ar.even.bdv = ar.even.bdv.add(crateBdv);
ar.even.tokens = ar.even.tokens.add(amounts[i]);
}
// grown stalk from germinating deposits do not germinate,
// and thus must be added to the grown stalk.
ar.grownStalkFromGermDeposits = ar.grownStalkFromGermDeposits.add(crateStalk);
}
}
​
// add inital stalk deposit to all stalk removed.
{
uint256 stalkIssuedPerBdv = s.ss[token].stalkIssuedPerBdv;
if (ar.active.tokens > 0) {
ar.active.stalk = ar.active.stalk.add(ar.active.bdv.mul(stalkIssuedPerBdv));
}
​
if (ar.odd.tokens > 0) {
ar.odd.stalk = ar.odd.bdv.mul(stalkIssuedPerBdv);
}
​
if (ar.even.tokens > 0) {
ar.even.stalk = ar.even.bdv.mul(stalkIssuedPerBdv);
}
}
​
// "removing" deposits is equivalent to "burning" a batch of ERC1155 tokens.
emit TransferBatch(msg.sender, account, address(0), removedDepositIDs, amounts);
emit RemoveDeposits(
account, token, stems, amounts, ar.active.tokens.add(ar.odd.tokens).add(ar.even.tokens), bdvsRemoved
);
}

Impact

If the lengths of the stems and amounts arrays are not equal, it can lead to out-of-bounds access or incomplete processing of deposit removal operations. This could result in inconsistent state changes within the application and potentially cause unexpected behavior or errors.

Also without checking if the stems array has at least one element, the loop may iterate unnecessarily, which could waste gas and processing resources. It's important to optimize the function to avoid unnecessary computations.

Tools Used

Manual Review

Recommendations

To address these issues, you should add the following validations at the beginning of the function:

require(stems.length > 0, "EnrootFacet: The 'stems' array must have at least one element");
require(stems.length == amounts.length, "EnrootFacet: The lengths of 'stems' and 'amounts' arrays must be equal");

These validations ensure that the stems array has at least one element and that both arrays have the same length before proceeding with the removal of deposits. By adding these checks, you improve the robustness and reliability of the function, reducing the risk of unexpected behavior or errors.

Updates

Lead Judging Commences

giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational/Invalid

Array length mismatch

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.