`allZeroes` does not work correctly

[hats-bug-reporter[bot]]

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

Description: Description:

The resolveMultiCategoricalMarket() function uses the allZeroes flag to determine if all payouts are zero, in which case the market is invalidated. However, the logic fails when a non-zero payout is prior to last. In such cases, like with payouts [0, 1, 0], the flag is incorrectly reset to true at the end, causing the market to be wrongly marked as invalid, which results in the winners losing their rightful payouts.

Attack Scenario:

The resolveMultiCategoricalMarket() function uses the allZeroes bool to determine if all payouts were zero. In that case, the market is invalidated.

bool allZeroes = true;

for (uint256 i = 0; i < numOutcomes; i++) {
    payouts[i] = (answer >> i) & 1;
    allZeroes = allZeroes && payouts[i] == 0;
}

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

Unfortunately, the current functionality does not work if the value before the last value is not zero. You can look at this example where the payouts are [0,1,0], which I chose to keep it simple:

Loop #1
allZeroes = 1 && 0 == 0 => 1

Loop #1
allZeroes = 1 && 1 == 0 => 0

Loop #1
allZeroes = 0 && 0 == 0 => 1

So, after the loop, allZeroes will be 1 again, and the market will be set as invalid, although not all values were zero. This will invalidate the payouts and result in the winners losing their winnings.

[J4X-98]

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

[greenlucid]

If this is correct, this issue is also invalid:

https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/issues/14#issuecomment-2374464661

[clesaege]

Loop 1
allZeroes = true && 0 == 0 => true

Loop 2
allZeroes = true && 1 == 0 => false

Loop 2
allZeroes = false && 0 == 0 => false

I've made a gist demonstrating that as another hunter reported something similar.

If you still believe you are correct, please provide a test demonstrating it.