0Kage - Interest component of underlying amount is not withdrawable using the withdrawLend function. Such amount is permanently locked in the BlueBerryBank contract

[github-actions]

0Kage

high

Interest component of underlying amount is not withdrawable using the withdrawLend function. Such amount is permanently locked in the BlueBerryBank contract

Summary

Soft vault shares are issued against interest bearing tokens issued by Compound protocol in exchange for underlying deposits. However, withdrawLend function caps the withdrawable amount to initial underlying deposited by user (pos.underlyingAmount). Capping underlying amount to initial underlying deposited would mean that a user can burn all his vault shares in withdrawLend function and only receive original underlying deposited.

Interest accrued component received from Soft vault (that rightfully belongs to the user) is no longer retrievable because the underlying vault shares are already burnt. Loss to the users is permanent as such interest amount sits permanently locked in Blueberry bank.

Vulnerability Detail

withdrawLend function in BlueBerryBank allows users to withdraw underlying amount from Hard or Soft vaults. Soft vault shares are backed by interest bearing cTokens issued by Compound Protocol

User can request underlying by specifying shareAmount. When user tries to send the maximum shareAmount to withdraw all the lent amount, notice that the amount withdrawable is limited to the pos.underlyingAmount (original deposit made by the user).

While this is the case, notice also that the full shareAmount is deducted from underlyingVaultShare. User cannot recover remaining funds because in the next call, user doesn't have any vault shares against his address. Interest accrued component on the underlying that was returned by SoftVault to BlueberryBank never makes it back to the original lender.

    wAmount = wAmount > pos.underlyingAmount
            ? pos.underlyingAmount
            : wAmount;

        pos.underlyingVaultShare -= shareAmount;
        pos.underlyingAmount -= wAmount;
        bank.totalLend -= wAmount;

Impact

Every time, user withdraws underlying from a Soft vault, interest component gets trapped in BlueBerry contract. Here is a scenario.

• Alice deposits 1000 USDC into SoftVault using the lend function of BlueberryBank at T=0
• USDC soft vault mints 1000 shares to Blueberry bank
• USDC soft vault deposits 1000 USDC into Compound & receives 1000 cUSDC
• Alice at T=60 days requests withdrawal against 1000 Soft vault shares
• Soft Vault burns 1000 soft vault shares and requests withdrawal from Compound against 1000 cTokens
• Soft vault receives 1050 USDC (50 USDC interest) and sends this to BlueberryBank
• Blueberry Bank caps the withdrawal amount to 1000 (original deposit)
• Blueberry Bank deducts 0.5% withdrawal fees and deposits 995 USDC back to user

In the whole process, Alice has lost access to 50 USDC.

Code Snippet

https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/BlueBerryBank.sol#L693

Tool used

Manual Review

Recommendation

Introduced a new variable to adjust positions & removed cap on withdraw amount.

Highlighted changes I recommend to withdrawLend with //******//.

function withdrawLend(address token, uint256 shareAmount)
        external
        override
        inExec
        poke(token)
    {
        Position storage pos = positions[POSITION_ID];
        Bank storage bank = banks[token];
        if (token != pos.underlyingToken) revert INVALID_UTOKEN(token);
        
        //*********-audit cap shareAmount to maximum value, pos.underlyingVaultShare*******
        if (shareAmount > pos.underlyingVaultShare) {
            shareAmount = pos.underlyingVaultShare;
        }

        // if (shareAmount == type(uint256).max) {
        //     shareAmount = pos.underlyingVaultShare;
        // }        

        uint256 wAmount;
        uint256 amountToOffset; //*********- audit added this to adjust position********
        if (address(ISoftVault(bank.softVault).uToken()) == token) {
            ISoftVault(bank.softVault).approve(
                bank.softVault,
                type(uint256).max
            );
            wAmount = ISoftVault(bank.softVault).withdraw(shareAmount);
        } else {
            wAmount = IHardVault(bank.hardVault).withdraw(token, shareAmount);
        }

        //*********- audit calculate amountToOffset********
        //*********-audit not capping wAmount anymore*******
        amountToOffset = wAmount > pos.underlyingAmount
            ? pos.underlyingAmount
            : wAmount;

        pos.underlyingVaultShare -= shareAmount;
     //*********-audit subtract amountToOffset instead of wAmount*******
        pos.underlyingAmount -= amountToOffset;
        bank.totalLend -= amountToOffset;

        wAmount = doCutWithdrawFee(token, wAmount);

        IERC20Upgradeable(token).safeTransfer(msg.sender, wAmount);
    }