Potential issue in MarketFactory:createCategoricalMarket

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0xcb7713d9df6da3b43a630c31be7584235353d73b4162309ae92f516fa7808157
Severity: high

Description:
Description
There is a potential issue with the market creation and resolution flow in the createCategoricalMarket function. The primary concern arises when multiple markets are created with identical parameters, leading to a situation where the same questionId and conditionId are generated. This can result in market collisions where one market’s resolution and payout structure affects another, potentially causing incorrect resolutions, cross-market payout conflicts, and exploitable conditions.

Let me explain the full functional flow of the scenario which leads to reverts and duplication.

he createCategoricalMarket function initiates the market creation process for categorical markets. This function is responsible for configuring and deploying a new market with a specific set of outcomes. The function accepts a set of parameters encapsulated in the CreateMarketParams structure, including:

• marketName: The name of the market.
• outcomes[]: An array of possible outcomes (for example, “Team A,” “Team B”).
• category: The category of the market question.
• lang: The language of the question.
• lowerBound and upperBound: These are used only in scalar markets.
• Other metadata like parent market details (optional), min bond, opening time, etc.

This encodes the market question and its outcomes for interaction with Reality.eth.

The market's questionId and conditionId are generated based on the encoded question, the number of outcomes, and certain configuration settings. The questionId is a hash derived from the question parameters and the outcomes, and the conditionId is generated based on the questionId and the number of outcomes.
Code:

bytes32 questionId = keccak256(
    abi.encode(questionsIds, params.outcomes.length, config.templateId, params.lowerBound, params.upperBound)
);
bytes32 conditionId = prepareCondition(questionId, config.outcomeSlotCount);

Here, the conditionId and questionId are based on identical parameters, meaning two markets with the same settings will end up with the same questionId and conditionId.

The new market is deployed using the createMarket function, which clones the existing market template, sets up the market parameters (such as outcomes, bounds, and question details), and initializes the market with the generated conditionId and questionId.

Market instance = Market(market.clone());
instance.initialize(
    marketName,
    params.outcomes,
    params.lowerBound,
    params.upperBound,
    conditionalTokensParams,
    realityParams,
    realityProxy
);

The issue arises because the same conditionId and questionId are shared between markets with identical parameters, as explained in the next sections.

Once a market is created and the outcome is known (either by manual reporting or through the resolution mechanism), the resolve function in the RealityProxy contract is invoked. This function calls various market resolution methods depending on the type of market (categorical, scalar, etc.). The resolve function calls market.questionsIds() and market.templateId() to fetch the relevant question and outcome details. The resolveCategoricalMarket function is then invoked for categorical markets. In the resolveCategoricalMarket function, the result of the question is fetched from Reality.eth. Based on the result, the market's payout structure (payoutNumerators) is updated. If the answer from Reality.eth is invalid or out of range, the INVALID_RESULT outcome is used.

uint256 answer = uint256(realitio.resultForOnceSettled(questionsIds[0]));
uint256[] memory payouts = new uint256[](numOutcomes + 1);

if (answer == uint256(INVALID_RESULT) || answer >= numOutcomes) {
    payouts[numOutcomes] = 1;
} else {
    payouts[answer] = 1;
}

The market reports the outcome to the Conditional Tokens contract, updating the payoutNumerators for the questionId and conditionId.
Code:

conditionalTokens.reportPayouts(questionId, payouts);

Impact\

If multiple markets share the same questionId and conditionId, resolving one market will resolve all other markets with the same identifiers. This leads to potential unintended consequences, as outlined below.

If two markets share the same questionId and conditionId, the resolution of one market will directly affect the other. In practice, this means that resolving one market will resolve all markets with the same identifiers, regardless of their intended independence.

Example Scenario:

• Market 1: "Who will win the football match?" (Outcomes: "Team A", "Team B")
• Market 2: "Who will win the football match?" (Outcomes: "Team A", "Team B")

If both markets have the same outcomes, category, and language (or any other identical parameters), they will end up sharing the same questionId and conditionId. When Market 1 is resolved (e.g., "Team A" wins), Market 2 will also automatically be resolved with the same outcome, even if it was supposed to be independent.

The function reportPayouts in ConditionalTokens sets the payoutNumerators for a given conditionId. If multiple markets share the same conditionId, they will also share the same payout structure. This can lead to incorrect or duplicate payouts for users across different markets.

Example:

If Market 1 is resolved and the payout for "Team A" is set, users in Market 2 will receive payouts based on Market 1's resolution, even though Market 2 may have been intended to remain unresolved or independent.

Attackers could exploit this vulnerability by creating multiple identical markets. They could participate in all markets and manipulate the outcome of one, gaining payouts in all linked markets due to the shared questionId and conditionId.

Recommendation
Modify the questionId and conditionId generation logic to incorporate additional unique parameters, such as a market creation timestamp or unique market ID. This will ensure that even markets with identical settings are treated independently. Introduce validation to ensure that markets with identical parameters cannot be created. This would prevent accidental or malicious creation of duplicate markets.

[Tobi0x18]

To create second question with the same question_id of the first question is impossible in RealityETH-3.0.
As a result, to create 2 markets with the same question_id and conditionId is impossible.

File: contracts\src\interaction\reality\RealityETH-3.0.sol
335:     function _askQuestion(bytes32 question_id, bytes32 content_hash, address arbitrator, uint32 timeout, uint32 opening_ts, uint256 min_bond)  
336:         stateNotCreated(question_id) 

176:     modifier stateNotCreated(bytes32 question_id) {
177:         require(questions[question_id].timeout == 0, "question must not exist");
178:         _;
179:     }
180: 

[xyzseer]

There are several issues like this, all of them marked as invalid. It's not a bug, we are reusing reality questions and conditional token conditions on purpose.

You mention that This can lead to incorrect or duplicate payouts but you are not demonstrating it. If you think it's the case, a POC is needed.

[clesaege]

If multiple markets share the same questionId and conditionId, resolving one market will resolve all other markets with the same identifiers.

That is correct.

This leads to potential unintended consequences, as outlined below.

That isn't demonstrated by the report. If you have 2 markets with the same questionId and conditionId, that means they rely on the same question, so when the question is resolved, both markets should resolve.

Note that allowing multiple markets to share those is made to prevent someone from creating markets with stupid token names, blocking the creation of the legitimate market.