Beanstalk Part 3
Beanstalk
DeFiHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`ConvertFacet.convert()` uses a wrong `BDV` when `decreaseBDV = true`.

bladesec

Summary

ConvertFacet.convert() uses a wrong BDV when decreaseBDV = true.

Vulnerability Details

In convert(), it always uses newBdv as a deposit BDV if cp.decreaseBDV = true.

// Withdraw the tokens from the deposit.
(grownStalk, fromBdv) = _withdrawTokens(
cp.fromToken,
stems,
amounts,
cp.fromAmount,
cp.account
);
​
// Calculate the bdv of the new deposit.
uint256 newBdv = LibTokenSilo.beanDenominatedValue(cp.toToken, cp.toAmount);
​
// If `decreaseBDV` flag is not enabled, set toBDV to the max of the two bdvs.
toBdv = (newBdv > fromBdv || cp.decreaseBDV) ? newBdv : fromBdv;
​
toStem = _depositTokensForConvert(cp.toToken, cp.toAmount, toBdv, grownStalk, cp.account);

But, according to the Improvements Summary, any user can decrease a deposit's BDV if the recorded BDV(fromBdv in the codebase) is greater than the current BDV(newBdv in the codebase).

Implement an Anti λ → λ Convert type that allows any user to decrease a Deposit's BDV if the Recorded BDV (the BDV stored on-chain with the Deposit) is greater than the Current BDV (the number of tokens in the Deposit × the current BDV of the token);

From my understanding of the summary, newBdv should be used only when fromBdv > newBdv if decreaseBDV = true but the current implementation always uses newBdv whick looks incorrect.

Impact

ConvertFacet.convert() might use a wrong deposit BDV when decreaseBDV = true.

Tools Used

Manual Review

Recommendations

convert() should use mininum value of newBdv and fromBdv if decreaseBDV = true.

Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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