`getMarketInfo` skipResults does not work

[code423n4]

Handle

cmichel

Vulnerability details

The RCFactory.getMarketInfo function uses the same counter _resultNumber for the result arrays' index. This counter is increased if _skipResults is set, and the arrays are therefore not indexed at zero.

if (_resultNumber < _skipResults) {
    // @audit increases the array index
    _resultNumber++;
} else {
    // @audit will start at a higher number if _skipResults is set
    _marketAddresses[_resultNumber] = _market;
    _ipfsHashes[_resultNumber] = ipfsHash[_market];
    _slugs[_resultNumber] = addressToSlug[_market];
    _potSizes[_resultNumber] = IRCMarket(_market)
        .totalRentCollected();
    _resultNumber++;
}

Imagine _skipResults = marketInfoResults to receive the second "page" of market infos. The function will just return an empty array of size marketInfoResults because of the while(_resultNumber < marketInfoResults) condition and increasing this same counter when skipping results.

Impact

The function does not return the correct market infos if _skipResults is used.

Recommended Mitigation Steps

The _resultNumber which is the index to the result arrays may not be increased when skipping results, instead a different counter should be used. The easiest way to fix this is by just decrementing the _skipResults variable itself. Change the if (_resultNumber < _skipResults) condition to:

if (IRCMarket(_market).state() == IRCMarket.States(_state)) {
    if (_skipResults > 0) {
        _skipResults--;
    } else {
        // same old
    }
}

[Splidge]

Duplicate of #14

[0xean]

going with the severity of the duplicate #14 on this issue. Based on the functions purpose the potential blast radius of the issue does seem small.