Unchecked Percentage Allocation in Rebalance Function Allows Excessive Asset Distribution

Summary

The rebalance function in the smart contract does not properly validate the length of the percentages array against the number of supported money markets, potentially leading to incorrect fund distribution or function reverts.

Vulnerability Detail

The function attempts to rebalance tokens across multiple money markets based on provided percentages. However, it doesn't verify that the length of the percentages array matches the number of supported money markets minus one (as the last market receives the remainder). This can lead to two issues:

If percentages is shorter than expected, some markets will not receive funds. If percentages is longer than expected, the function will attempt to access non-existent money markets, causing a revert.

The function only checks for parity after filtering for supported markets:

if (percentagesLength + 1 != supportedMoneyMarketsSize) revert NotParity();

This check comes too late in the function execution and doesn't prevent the initial mismatch.

Impact

https://github.com/sherlock-audit/2024-06-union-finance-update-2/blob/main/union-v2-contracts/contracts/asset/AaveV3Adapter.sol#L1

uneven or incorrect distribution of funds across money markets

Code Snippet

    function rebalance(
        address tokenAddress,
        uint256[] calldata percentages
    ) external override onlyAdmin checkMarketSupported(tokenAddress) {
        IERC20Upgradeable token = IERC20Upgradeable(tokenAddress);
        uint256 moneyMarketsLength = moneyMarkets.length;
        uint256 percentagesLength = percentages.length;

        IMoneyMarketAdapter[] memory supportedMoneyMarkets = new IMoneyMarketAdapter[](moneyMarketsLength);
        uint256 supportedMoneyMarketsSize;

        // Loop through each money market and withdraw all the tokens
        for (uint256 i = 0; i < moneyMarketsLength; i++) {
            IMoneyMarketAdapter moneyMarket = moneyMarkets[i];
            if (!moneyMarket.supportsToken(tokenAddress)) continue;
            supportedMoneyMarkets[supportedMoneyMarketsSize] = moneyMarket;
            supportedMoneyMarketsSize++;
            moneyMarket.withdrawAll(tokenAddress, address(this));
        }

        if (percentagesLength + 1 != supportedMoneyMarketsSize) revert NotParity();

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

        for (uint256 i = 0; i < percentagesLength; i++) {
            IMoneyMarketAdapter moneyMarket = supportedMoneyMarkets[i];
            uint256 amountToDeposit = (tokenSupply * percentages[i]) / 10000;
            if (amountToDeposit == 0) continue;
            token.safeTransfer(address(moneyMarket), amountToDeposit);
            moneyMarket.deposit(tokenAddress);
        }

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

        IMoneyMarketAdapter lastMoneyMarket = supportedMoneyMarkets[supportedMoneyMarketsSize - 1];
        if (remainingTokens > 0) {
            token.safeTransfer(address(lastMoneyMarket), remainingTokens);
            lastMoneyMarket.deposit(tokenAddress);
        }

        emit LogRebalance(tokenAddress, percentages);
    }

Tool used

Manual Review

Recommendation

1..) Add an early check to validate the length of the percentages array:

function rebalance(
    address tokenAddress,
    uint256[] calldata percentages
) external override onlyAdmin checkMarketSupported(tokenAddress) {
    IERC20Upgradeable token = IERC20Upgradeable(tokenAddress);
    uint256 moneyMarketsLength = moneyMarkets.length;
    uint256 percentagesLength = percentages.length;

    require(percentagesLength == moneyMarketsLength - 1, "Invalid percentages length");

2.) Consider adding a check to ensure the sum of percentages does not exceed 10000:

uint256 totalPercentage = 0;
for (uint256 i = 0; i < percentagesLength; i++) {
    totalPercentage += percentages[i];
}
require(totalPercentage <= 10000, "Total percentage exceeds 100%");

sherlock-audit / 2024-06-union-finance-update-2-judging

Nyxaris - Unchecked Percentage Allocation in Rebalance Function Allows Excessive Asset Distribution #8