Markets will incorrectly be invalidated

[hats-bug-reporter[bot]]

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

Description: Description:

The resolveMultiScalarMarket() function uses the allZeroesOrInvalid flag to check if all market payouts are zero or the market is invalid. The problem is that the logic inside the loop incorrectly updates this flag when payouts contain both zeros and non-zero values. For example, with payouts [0, val>0, 0, val>0], the flag gets set to true at the end, wrongly marking the market as invalid even though it's not.

Attack Scenario:

The resolveMultiScalarMarket() function uses the allZeroesOrInvalid to ensure that not all values in the market are zero or invalid. However, this will not work the way it is implemented.

bool allZeroesOrInvalid = true;

uint256 maxPayout = 2 ** (256 / 2) - 1;

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;
}

if (allZeroesOrInvalid) {
    // invalid result.
    payouts[numOutcomes] = 1;
}

The issue is that we will end up with an invalid market if allZeroesOrInvalid if there the last value is greater than 0 and the one before was zero. You can see in this example

payouts = [0, val>0, 0, val>0]

Loop #1:
allZeroesOrInvalid = 1 && 0 == 0 => 0 

Loop #2:
allZeroesOrInvalid = 0 && val>0 == 0 => 1 

Loop #3:
allZeroesOrInvalid = 1 && val>0 == 0 => 0 

Loop #4:
allZeroesOrInvalid = 0 && 0 == 0 => 1 

In this case, at the end of the loop, allZeroesOrInvalid will be true, and the market will be invalidated, although this is actually not true as neither all values are zero nor was the market invalid.

[J4X-98]

Quick addition here. This occurs if the last value is 0, not if the last is greater than 0. I mixed something up in a hurry there. Root cause of this being incorrectly implemented still stays the same.

[greenlucid]

https://docs.soliditylang.org/en/latest/cheatsheet.html

equality is checked before AND, so

allZeroesOrInvalid = 0 && 0 == 0 => 1 

is just 0, not 1

[clesaege]

payouts = [0, val>0, 0, val>0] Taking your loop as an example, you misevaluated #2

Loop #1: allZeroesOrInvalid = true && 0 == 0 => true

Loop #2: allZeroesOrInvalid = true && val>0 == 0 => false

Loop #3: allZeroesOrInvalid = false && val>0 == 0 => false

Loop #4: allZeroesOrInvalid = false && 0 == 0 => false