Some markets won't be able to be resolved, because there isn't a limit for the max amount of outcomes, which can lead to DoS.

[hats-bug-reporter[bot]]

Github username: @MrValioBg Twitter username: valuevalk Submission hash (on-chain): 0x8847d43049d1ca5f010b86da5da10a89cfdecc4c2ff5ce5216291f33c7239ae8 Severity: high

Description:

Summary

It is possible that a market with too many possible outcomes cannot be resolved, due to high gas costs.

Vulnerability Details

We have the options to create different markets via MarketFactory.sol, however there isnt any enforcement on the max amount of outcomes, we just enforce the minimum amount, for example in createMultiScalarMarket:

    function createMultiScalarMarket(CreateMarketParams calldata params) external returns (address) {
@>>     require(params.outcomes.length >= 2, "Outcomes count must be 2 or more");

This is also valid for the other create functions.

The problem is that when we come to resolve the result of 1 market, we use RealityProxy where we essentially loop through all the outcomes, for example in resolveMultiScalarMarket:

    function resolveMultiScalarMarket(
        bytes32 questionId,
        bytes32[] memory questionsIds,
        uint256 numOutcomes
    ) internal {
        uint256[] memory payouts = new uint256[](numOutcomes + 1);
        bool allZeroesOrInvalid = true;

        /*
         * We set maxPayout to a sufficiently large number for most possible outcomes that also avoids overflows in the following places:
         * https://github.com/gnosis/conditional-tokens-contracts/blob/master/contracts/ConditionalTokens.sol#L89
         * https://github.com/gnosis/conditional-tokens-contracts/blob/master/contracts/ConditionalTokens.sol#L242
         */
        uint256 maxPayout = 2 ** (256 / 2) - 1;

        //@audit-issue potential blocking here.
@>>     for (uint256 i = 0; i < numOutcomes; i++) {
            payouts[i] = uint256(realitio.resultForOnceSettled(questionsIds[i]));

            if (payouts[i] == uint256(INVALID_RESULT)) {
                payouts[i] = 0;
            } else if (payouts[i] > maxPayout) {
                payouts[i] = maxPayout;
            }

            allZeroesOrInvalid = allZeroesOrInvalid && payouts[i] == 0;
        }

Impact

The problem is that too much looping will result in OOG errors, as its even possible to not be able to include the transaction in any block if the numOutcomes becomes too high. When we have too many outcomes set, this will result in DoS in this method and consequently payouts will never be made.

Recommendation

Enforce a reasonable limit on maximum possible outcomes when creating a market, to avoid reaching such a condition.

[clesaege]

The maximum possible number of outcomes is enforced by the conditional token framework.