Unbounded loop may cause Dos by user-specified array-length.

Summary

Unbounded loop may cause Dos by user-specified loss.

Vulnerability Detail

TxBuilderExtension.execute() function takes user-specified array length and passed to executeInternal() function in this function calls TxBuilderExtension.borrow() here which calls IronBank.borrow() look like below :-

    function borrow(address from, address to, address market, uint256 amount)
        external
        nonReentrant
        isAuthorized(from)
    {
        DataTypes.Market storage m = markets[market];
        require(m.config.isListed, "not listed");
        require(!m.config.isBorrowPaused(), "borrow paused");
        require(m.totalCash >= amount, "insufficient cash");

        _accrueInterest(market, m);

        uint256 newTotalBorrow = m.totalBorrow + amount;
        uint256 newUserBorrowBalance;
        unchecked {
            // Overflow not possible: borrowBalance + amount is at most totalBorrow + amount, which is checked above.
            newUserBorrowBalance = _getBorrowBalance(m, from) + amount;
        }

        if (m.config.borrowCap != 0) {
            require(newTotalBorrow <= m.config.borrowCap, "borrow cap reached");
        }

        // Update storage.
        unchecked {
            m.totalCash -= amount;
        }
        m.totalBorrow = newTotalBorrow;
        m.userBorrows[from].borrowBalance = newUserBorrowBalance;
        m.userBorrows[from].borrowIndex = m.borrowIndex;

        // Enter the market.
        if (amount > 0) {
            _enterMarket(market, from);
        }

        IERC20(market).safeTransfer(to, amount);

        if (isCreditAccount(from)) {
            require(from == to, "credit account can only borrow to itself");
            require(creditLimits[from][market] >= newUserBorrowBalance, "insufficient credit limit");
        } else {
            _checkAccountLiquidity(from);
        }

in above function we can see it accessing state variable and changes the state variable per loop . And in line 384 call _entermaket() function look like below :-

    function _enterMarket(address market, address user) internal {
        if (enteredMarkets[user][market]) {
            // Skip if user has entered the market.
            return;
        }

        enteredMarkets[user][market] = true;
        allEnteredMarkets[user].push(market);

        emit MarketEntered(market, user);
    }

In above function which push new market to global array .When IronBank protocol have large amount of different markets and user submit same large amount markets to entered in same block transaction then this will cause DOS . Because unbounded loop which will reach limit of block gas limit.

Impact

actions array based on user-specified length if user submit large length of array the block.limit may reached and chance of DOS .

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/extensions/TxBuilderExtension.sol#L100

Tool used

Manual Review

Recommendation

Limit the size of Array of actions in execute() function

sherlock-audit / 2023-05-ironbank-judging

pavankv241 - Unbounded loop may cause Dos by user-specified array-length. #307