Beanstalk Part 3

Summary

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibUnripe.sol#L134-L142
https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibUnripe.sol#L58-L67
https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibFertilizer.sol#L304-L326

The switchUnderlyingToken function lacks a crucial check to ensure that the balanceOfUnderlying is zero before proceeding to switch the underlying token. This check is mentioned in the documentation but not enforced in the code.

Vulnerability Details

`/**
 * @dev Switches the underlying token of an unripe token.
 * Should only be called if `s.u[unripeToken].balanceOfUnderlying == 0`.
 */
function switchUnderlyingToken(address unripeToken, address newUnderlyingToken) internal {
    AppStorage storage s = LibAppStorage.diamondStorage();
    s.u[unripeToken].underlyingToken = newUnderlyingToken;
    emit SwitchUnderlyingToken(unripeToken, newUnderlyingToken);
}`

`/**
 * @notice Decrements the underlying balance of an Unripe Token.
 * @param token The address of the Unripe Token.
 * @param amount The amount of the of the Unripe Token to be removed from storage reserves
 */
function decrementUnderlying(address token, uint256 amount) internal {
    AppStorage storage s = LibAppStorage.diamondStorage();
    s.u[token].balanceOfUnderlying = s.u[token].balanceOfUnderlying.sub(amount);
    emit ChangeUnderlying(token, -int256(amount));
}`

`function beginBarnRaiseMigration(address well) internal {
    AppStorage storage s = LibAppStorage.diamondStorage();
    require(well.isWell(), "Fertilizer: Not a Whitelisted Well.");


    // The Barn Raise only supports 2 token Wells where 1 token is Bean and the
    // other is supported by the Lib Usd Oracle.
    IERC20[] memory tokens = IWell(well).tokens();
    require(tokens.length == 2, "Fertilizer: Well must have 2 tokens.");
    require(
        tokens[0] == C.bean() || tokens[1] == C.bean(),
        "Fertilizer: Well must have BEAN."
    );
    // Check that Lib Usd Oracle supports the non-Bean token in the Well.
    LibUsdOracle.getTokenPrice(address(tokens[tokens[0] == C.bean() ? 1 : 0]));


    uint256 balanceOfUnderlying = s.u[C.UNRIPE_LP].balanceOfUnderlying;
    IERC20(s.u[C.UNRIPE_LP].underlyingToken).safeTransfer(
        LibDiamond.diamondStorage().contractOwner,
        balanceOfUnderlying
    );
    LibUnripe.decrementUnderlying(C.UNRIPE_LP, balanceOfUnderlying);
    LibUnripe.switchUnderlyingToken(C.UNRIPE_LP, well);
}`

As can be seen from above, the functions that should check for the balanceOfUnderlying is zero does not do that. this can lead to a scenario where tokens are locked, underlying switched and users losing their funds.

Impact

The switchUnderlyingToken function lacks a crucial check to ensure that the balanceOfUnderlying is zero before proceeding to switch the underlying token. This check is mentioned in the documentation but not enforced in the code.

Tools Used

Manual Review

Recommendations

+ function switchUnderlyingToken(address unripeToken, address newUnderlyingToken) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
require(s.u[unripeToken].balanceOfUnderlying == 0, "Underlying balance must be zero to switch tokens");
s.u[unripeToken].underlyingToken = newUnderlyingToken;
emit SwitchUnderlyingToken(unripeToken, newUnderlyingToken);
}