DeFiHardhat
21,000 USDC
View results
Submission Details
Severity: medium
Invalid

Hardcoded Boolean Value Leads to Inflexible Behavior

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

  1. 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

  1. 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

  1. 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

/// @notice Decoder for the antiLambdaConvert
/// @dev contains an additional address parameter for the account to update the deposit
/// and a bool to indicate whether to decrease the bdv

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:

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;
+ (, amount, token, account, decreaseBDV) = abi.decode(self, (ConvertKind, uint256, address, address, bool));
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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