Beanstalk Part 3

Summary

The antiLambdaConvert function in the LibConvertData.sol library is made to decode conversion parameters from a byte array, including a boolean flag decreaseBDV that indicates whether the bean denominated value (BDV) should be decreased. Albeit, the function hardcodes this boolean to true, which does not allow for dynamic behavior based on the input data. This inflexibility can lead to incorrect application logic where the decrease of BDV might not be intended.

Vulnerability details

Here is the sequence of function calls leading to the issue:

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/beanstalk/silo/ConvertFacet.sol#L79

1. ConvertFacet.convert function is called:

   LibConvert.convert(convertData);

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/Convert/LibConvert.sol#L65-L67

2. Inside LibConvert.convert, based on the conversion kind, it calls LibLambdaConvert.antiConvert:

   LibLambdaConvert.antiConvert(convertData);

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/Convert/LibLambdaConvert.sol#L52

3. LibLambdaConvert.antiConvert then calls LibConvertData.antiLambdaConvert:

   LibConvertData.antiLambdaConvert(convertData);

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/Convert/LibConvertData.sol#L79-L80

4. In LibConvertData.antiLambdaConvert, the boolean decreaseBDV is hardcoded:

   function antiLambdaConvert(bytes memory self)

   internal

   pure

   returns (uint256 amount, address token, address account, bool decreaseBDV)

   {

   (, amount, token, account) = abi.decode(self, (ConvertKind, uint256, address , address));

   decreaseBDV = true; // Hardcoded, does not decode from `self`

   }

However, the NatSpec comment for antiLambdaConvert suggests that there should be a boolean to indicate whether to decrease BDV, implying flexibility (true or false). However, the implementation hardcodes this value to true, which contradicts the suggested flexibility in the NatSpec comment.

Here is the Natspec:

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/Convert/LibConvertData.sol#L71-L73

Impact

The hardcoded boolean value restricts the flexibility of conversion operations, potentially leading to unintended financial or state changes in the smart contract system. This could affect the correctness of deposit updates and other relevant calculations or conversions. As already shown above, here is the chain of calls affected:

• ConvertFacet.convert in ConvertFacet.sol calls LibConvert.convert in LibConvert.sol which calls LibLambdaConvert.antiConvert in LibLambdaConvert.sol which eventually calls LibConvertData.antiLambdaConvert in LibConvertData.sol where this bug originates from.

Tools Used

Manual review

Recommended Mitigation Steps:

Modify the abi.decode line in LibConvertData.antiLambdaConvert to include the boolean value from the encoded data: