rebalance function will fail due to invalid condition

[code423n4]

Handle

csanuragjain

Vulnerability details

Impact

User will be unable to rebalance the fund

Proof of Concept

1. Navigate to contract at https://github.com/code-423n4/2021-10-union/blob/main/contracts/asset/AssetManager.sol

2. Check the rebalance function

    function rebalance(address tokenAddress, uint256[] calldata percentages)
        external
        override
        checkMarketSupported(tokenAddress)
        onlyAdmin
    {
        IERC20Upgradeable token = IERC20Upgradeable(tokenAddress);
        require(percentages.length + 1 == moneyMarkets.length, "AssetManager: percentages error");

        for (uint256 i = 0; i < moneyMarkets.length; i++) {
            if (!moneyMarkets[i].supportsToken(tokenAddress)) {
                continue;
            }
            moneyMarkets[i].withdrawAll(tokenAddress, address(this));
        }

        uint256 tokenSupply = token.balanceOf(address(this));

        for (uint256 i = 0; i < percentages.length; i++) {
            if (!moneyMarkets[i].supportsToken(tokenAddress)) {
                continue;
            }
            uint256 amountToDeposit = (tokenSupply * percentages[i]) / 10000;
            if (amountToDeposit == 0) {
                continue;
            }
            token.safeTransfer(address(moneyMarkets[i]), amountToDeposit);
            moneyMarkets[i].deposit(tokenAddress);
        }

        uint256 remainingTokens = token.balanceOf(address(this));
        if (moneyMarkets[moneyMarkets.length - 1].supportsToken(tokenAddress) && remainingTokens > 0) {
            token.safeTransfer(address(moneyMarkets[moneyMarkets.length - 1]), remainingTokens);
            moneyMarkets[moneyMarkets.length - 1].deposit(tokenAddress);
        }

        require(token.balanceOf(address(this)) == 0, "AssetManager: there are remaining funds in the fund pool");

        emit LogRebalance(tokenAddress, percentages);
    }

3. The condition tries to see if percentages.length + 1 == moneyMarkets.length which is incorrect. It should be ideally percentages.length == moneyMarkets.length. If this correction is not made then below loop will fail when i=percentages.length since moneyMarkets[percentages.length] does not exist

for (uint256 i = 0; i < percentages.length; i++) {
            if (!moneyMarkets[i].supportsToken(tokenAddress)) {
                continue;
            }
            uint256 amountToDeposit = (tokenSupply * percentages[i]) / 10000;
            if (amountToDeposit == 0) {
                continue;
            }
            token.safeTransfer(address(moneyMarkets[i]), amountToDeposit);
            moneyMarkets[i].deposit(tokenAddress);
        }

Recommended Mitigation Steps

Change the condition to: require(percentages.length == moneyMarkets.length, "AssetManager: percentages error");

[GalloDaSballo]

Disagree with the finding

percentages.length + 1 == moneyMarkets.length means that percentages.length < moneyMarkets

And the index i never goes above those values

Invalid finding