Hash collision will not allow for generation of similar markets

Github username: @J4X_98 Twitter username: J4X_Security Submission hash (on-chain): 0x44135b783d69a9e687774608b7a95fe80f37d846068dc807d9fcd19ee8603743 Severity: medium

Description: Description:

The createMultiScalarMarket() function creates a multi-scalar market using the CreateMarketParams struct, including several arbitrary user-provided strings. It encodes market questions by concatenating the questionStart, each outcome, and questionEnd strings using abi.encodePacked(), which can lead to hash collisions when distinct inputs produce the same encoded result. This causes the askRealityQuestion() function to potentially reuse a previously generated question ID if the concatenated string hash matches one from an existing market, even if the question data differs. Although the market name is correctly separated to avoid collisions, this issue with encoded questions can lead to unintended reuse of question IDs across markets.

Attack Scenario:

When a user wants to create a multi-scalar market, he calls createMultiScalarMarket(), providing the CreateMarketParams struct.

struct CreateMarketParams {
    string marketName;
    string[] outcomes;
    string questionStart;
    string questionEnd;
    string outcomeType;
    uint256 parentOutcome;
    address parentMarket;
    string category;
    string lang;
    uint256 lowerBound;
    uint256 upperBound;
    uint256 minBond;
    uint32 openingTime;
    string[] tokenNames;
}

This structure includes multiple strings, which can be of arbitrary size and are user-provided.

for (uint256 i = 0; i < params.outcomes.length; i++) {
    encodedQuestions[i] = encodeRealityQuestionWithoutOutcomes(
        string(abi.encodePacked(params.questionStart, params.outcomes[i], params.questionEnd)),
        params.category,
        params.lang
    );
}

As we can see, the encodedQuestions are filled out by calling the encodeRealityQuestionWithoutOutcomes() with some user-provided data. Here the bug occurs, we are using abi.encodePacked() to pack params.questionStart + params.outcomes[i] + params.questionEnd. The problem is that each of these is an arbitrary string of size. So even markets where all 3 are different from one another could end up with the exact string being parsed into the encodeRealityQuestionWithoutOutcomes() function as the question parameter here. encodeRealityQuestionWithOutcomes() will then encode this and save it into the encodeQuestions variable. Later, we can see that askRealityQuestion() gets called with the created string.

for (uint256 i = 0; i < config.encodedQuestions.length; i++) {
    questionsIds[i] =
        askRealityQuestion(config.encodedQuestions[i], config.templateId, params.openingTime, params.minBond);
}

The problem is that if an Id for this string was already generated, we will reuse the same Id.

function askRealityQuestion(
    string memory encodedQuestion,
    uint256 templateId,
    uint32 openingTime,
    uint256 minBond
) internal returns (bytes32) {
    bytes32 content_hash = keccak256(abi.encodePacked(templateId, openingTime, encodedQuestion));

    bytes32 question_id = keccak256(
        abi.encodePacked(
            content_hash, arbitrator, questionTimeout, minBond, address(realitio), address(this), uint256(0)
        )
    );

    if (realitio.getTimeout(question_id) != 0) {
        return question_id;
    }

    return realitio.askQuestionWithMinBond(
        templateId, encodedQuestion, arbitrator, questionTimeout, openingTime, 0, minBond
    );
}

So, in our case, although this question combination has not been asked yet, we will just use the ID from the already-asked one with the same hash. When we look at how the market name is created later on, we can see that it is correctly separated to prevent collisions.

return createMarket(
    params,
    string(abi.encodePacked(params.questionStart, "[", params.outcomeType, "]", params.questionEnd)),
    InternalMarketConfig({
        encodedQuestions: encodedQuestions,
        outcomeSlotCount: params.outcomes.length + 1, // additional outcome for Invalid Result.
        templateId: REALITY_UINT_TEMPLATE
    })
);

Recommendation:

The issue can be mitigated by separating the question start, outcome, and question end. This can be achieved either by using abiEncode or by separating them with something like colons.

hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

Hash collision will not allow for generation of similar markets #11