hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

getMarkets() can be inaccessible as the number of the markets is unfixed #36

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x57796cb5bc251f4258e3d981eda97c011cac4b00b182fec383ce6ed3b7c2f8a1 Severity: low

Description: Description\

Currently getMarkets() fetches the total number of the markets and loops through it getting info for every one of them. The problem is that if the number of the markets becomes too huge, the function will face memory corruption and DoS as it will be too expensive to execute.

Attack Scenario\

Take a look at the current getMarkets() functionality:

MarketView.sol::158-180

function getMarkets(uint256 count, IMarketFactory marketFactory) external view returns (MarketInfo[] memory) {
        address[] memory allMarkets = marketFactory.allMarkets();

        MarketInfo[] memory marketsInfo = new MarketInfo[](count);

        if (allMarkets.length == 0) {
            return marketsInfo;
        }

        uint256 lastIndex = allMarkets.length - 1;
        uint256 startIndex = allMarkets.length > count ? allMarkets.length - count : 0;
        uint256 currentIndex = 0;

        for (uint256 j = lastIndex; j >= startIndex; j--) {
            marketsInfo[currentIndex++] = getMarket(marketFactory, Market(allMarkets[j]));

            if (j == 0) {
                break;
            }
        }

        return marketsInfo;
    }

As you can see here, the loop is only stopped if the lastIndex is reached and this value is equal to the allMarkets.length – 1. And as we are dealing with memory here, the function may become too expensive to execute as everything is copied to memory and face memory corruption. Therefore, getMarkets() function will be broken. `

Recommendation Limit the total number of the markets to avoid DoS.

clesaege commented 1 month ago

This is just a frontend helper, gas isn't an issue (and it's not in scope).