Ragnark_323 - Unbounded loop in `collectProtocol` function can leads to DOS

[sherlock-admin]

Ragnark_323

high

Unbounded loop in collectProtocol function can leads to DOS

Summary

unbounded loop in collectProtocol function can lead to Denail of Service

Vulnerability Detail

the collectProtocol function allows owner to collect protocol fees for multiple tokens and transfer them to a specified recipient.the loop inside this function iterate through array of tokens and calculate amount i.e protocol fees and if amount>0 then it transfer the amount to recipient address i.e the address of the recipient who will receive the collected fees and it tranfer's the amount by call's the IERC20's safeTransfer function .with all this happening in the loop and costing gas and executing gas costly function like safeTransfer() for all tokens in this loop can leads to dos due to exceeding the block size gas limit.

Impact

there can be many tokens that the loop have to go through and calculate the protocal fees and tranfering the protocol fees to recipient address in the loop by using gas costly executions like safeTransfer() can lead to fail of execution due to exceeding block size gas limit

Code Snippet

LiquidityBorrowingManager.sol#ln184

/**
     * @notice This function allows the owner to collect protocol fees for multiple tokens
     * and transfer them to a specified recipient.
     * @dev Only the contract owner can call this function.
     * @param recipient The address of the recipient who will receive the collected fees.
     * @param tokens An array of addresses representing the tokens for which fees will be collected.
     */
    function collectProtocol(address recipient, address[] calldata tokens) external onlyOwner {
        uint256[] memory amounts = new uint256[](tokens.length);
        for (uint256 i; i < tokens.length; ) {
            address token = tokens[i];
            uint256 amount = platformsFeesInfo[token] / Constants.COLLATERAL_BALANCE_PRECISION;
            if (amount > 0) {
                platformsFeesInfo[token] = 0;
                amounts[i] = amount;
                Vault(VAULT_ADDRESS).transferToken(token, recipient, amount);
            }
            unchecked {
                ++i;
            }
        }

        emit CollectProtocol(recipient, tokens, amounts);
    }

Tool used

VS code

Recommendation

avoide all the actions executed in a single transaction, especially when transfer's are executed as part of a loop.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid. We don't have an unbounded loop here. In the extreme case the admin will call the function again with a smaller array of tokens