sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

Bauchibred - BlueBerryBank.borrow() is vulnerable to DOS attack, can easily be done when bank total debt is low. #83

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Bauchibred

medium

BlueBerryBank.borrow() is vulnerable to DOS attack, can easily be done when bank total debt is low.

Summary

If the bank total debt is low, a malicious user can launch a DOS attack to Borrow(). All that's needed to do is to pay off the whole debt for the bank DIRECTLY through the underlying bank.ctoken (compound) protocol. As a result, it will cause a divide-by-zero revert for the borrow() function. The borrow()will revert for that token forever. This DOS can be launched to freeze all the banks (A valid argument here is that this can only be done for all the banks with small amount of total debt cost wise from the attacker's end, but technically I think this can be done for any bank, whether the bank has a small total debt or not)

Vulnerability Detail

Below's how a malicious user can launch a DOS attack to Borrow() when bank total debt is low.

Let's say there exist a bank for token A and it has a very low total debt now. For example, right after the protocol is launched. Let bank.totalShare = 100 and bank.totalDebt = 100.

The attacker, Bob, can also front-run other borrowers and become the first borrower to set up the above state: bank.totalShare = 100 and bank.totalDebt = 100.

Attacker Bob then repays all the debt for the bank, not via BlueBerryBank, but directly through the underlying borrowing protocol, bank.ctoken, settting the BlueBerryBank's address as the payee, and Bob's wallet address as the payer. As a result, the bank's total debt becomes zero. That is bank.totalShare = 100 and bank.totalDebt = 0.

Another user Alice tries to call borrow() to borrow token A. however, L704 will always revert due to a divide-by-zero error since bank.totalDebt = 0. Therefore, nobody can use borrow() to borrow token A anymore.

Note that this same attack can be launched for banks for any other tokens in BlueBerryBank.sol

Impact

A DOS attack can be launched for any bank in the BlueBerryBank. The attack will be more affordable when the debt is low, for example, when the protocol is just launched. Nobody can use borrow() for that token anymore after the attack.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/BlueBerryBank.sol#L675-L713

Tool used

Manual review

Recommendation

When we see bank.totalShare !=0 and bank.totalDebt = 0, we can use 1000 as the denominator. That is, we artificially introduce a small total debt (1000 wei is nothing for most tokens) when there is no debt but there are shares.

Change function to:

/// @dev Borrow tokens from given bank. Must only be called from spell while under execution.
    /// @param token The token to borrow from the bank.
    /// @param amount The amount of tokens to borrow.
    /// @return borrowedAmount Returns the borrowed amount
    function borrow(
        address token,
        uint256 amount
    )
        external
        override
        inExec
        poke(token)
        onlyWhitelistedToken(token)
        returns (uint256 borrowedAmount)
    {
        if (!isBorrowAllowed()) revert Errors.BORROW_NOT_ALLOWED();
        Bank storage bank = banks[token];
        Position storage pos = positions[POSITION_ID];
        if (pos.debtToken != address(0)) {
            // already have some debts, allow same debt token
            if (pos.debtToken != token) revert Errors.INCORRECT_DEBT(token);
        } else {
            pos.debtToken = token;
        }

        uint256 totalShare = bank.totalShare;
        uint256 totalDebt = _borrowBalanceStored(token);
+     if(totalDebt == 0) totalDebt = 1000;  // @audit avoid divide-by-error revert
        uint256 share = totalShare == 0
            ? amount
            : (amount * totalShare).divCeil(totalDebt);
        if (share == 0) revert Errors.BORROW_ZERO_SHARE(amount);
        bank.totalShare += share;
        pos.debtShare += share;

        borrowedAmount = _doBorrow(token, amount);
        IERC20Upgradeable(token).safeTransfer(msg.sender, borrowedAmount);

        emit Borrow(POSITION_ID, msg.sender, token, amount, share);
    }