Potential Off-by-One Error in resolveCategoricalMarket

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0x44a639d513f1d398c1639c98b9774cb402e44030609eddf7fb108f04e0283b2a
Severity: medium

Description:

Details

The RealityProxy contract resolves prediction markets by fetching results from Realitio and reporting them to the Conditional Tokens contract. The resolveCategoricalMarket function specifically handles the resolution of categorical markets, where the outcome is a single choice from a list of options.

In resolveCategoricalMarket, the code checks if the answer from Realitio is invalid using the condition answer >= numOutcomes. However, numOutcomes represents the number of valid outcomes, excluding the invalid outcome. Therefore, the correct condition to identify an invalid answer should be answer > numOutcomes.

Code Snippet

// ... inside resolveCategoricalMarket ...

if (answer == uint256(INVALID_RESULT) || answer >= numOutcomes) { // Incorrect condition
    // the last outcome is INVALID_RESULT.
    payouts[numOutcomes] = 1;
} else {
    payouts[answer] = 1;
}

Impact

This off-by-one error could lead to misclassifying a valid outcome as invalid. If the answer is exactly equal to numOutcomes, it represents the last valid outcome. However, the incorrect condition would treat it as an invalid outcome, potentially leading to incorrect payouts and disputes.

Scenario

• A categorical market is created with three valid outcomes (0, 1, 2).
• The correct answer from Realitio is 2.
• In resolveCategoricalMarket, numOutcomes is 3.
• The condition answer >= numOutcomes evaluates to true (2 >= 3), incorrectly identifying the answer as invalid.
• The market resolves to the invalid outcome, even though the answer was a valid outcome.

Fix

Change the condition to answer > numOutcomes:

// ... inside resolveCategoricalMarket ...

if (answer == uint256(INVALID_RESULT) || answer > numOutcomes) { // Correct condition
    // the last outcome is INVALID_RESULT.
    payouts[numOutcomes] = 1;
} else {
    payouts[answer] = 1;
}

Test

it("should correctly resolve a categorical market with an answer equal to the last valid outcome", async function () {
    const numOutcomes = 3; // Create a market with 3 valid outcomes

    // Create a categorical market with the specified number of outcomes
    const currentBlockTime = await time.latest();
    await marketFactory.createCategoricalMarket({
        ...categoricalMarketParams,
        outcomes: Array(numOutcomes).fill("Outcome").map((_, i) => i.toString()), // Generate outcomes dynamically
        openingTime: currentBlockTime + OPENING_TS,
    });

    // ... (rest of the test setup from your existing resolveCategoricalMarket test) ...

    // Submit an answer equal to the last valid outcome
    const answer = numOutcomes - 1;
    await realitio.submitAnswer(
        (await market.questionsIds())[0],
        ethers.toBeHex(BigInt(answer), 32),
        0,
        { value: ethers.parseEther(MIN_BOND) }
    );

    // ... (rest of the test logic) ...

    // Check that the payouts array has 1 at the correct index (answer)
    const expectedPayouts = Array(numOutcomes + 1).fill(0).map((_, i) => (i === answer ? 1 : 0));
    expect(eventPayouts.map(Number)).to.deep.equal(expectedPayouts);
});

This test creates a categorical market with a specific number of outcomes. It then submits an answer equal to the last valid outcome and checks that the market resolves correctly to that outcome, ensuring that the off-by-one error is fixed.

[clesaege]

Numbering starts at 0. So if there are numOutcomes non invalid outcomes, the last valid outcome is numOutcomes-1. So the first invalid outcome is numOutcomes.