Use reasonable type for payouts in reportPayouts

[rmeissner]

Currently the payouts in the reportPayouts are uints which allow for potential issues in the claiming process when the oracle chooses very high values.

This issues was discovered as part of our bug bounty programm by pdobacz.

Bounty Submission

Given some resonable assumptions about the way the oracle contract executes reportPayouts, there might be a way to lock up the collateral associated with a condition. The potential exploit would be as follows:

Suppose that some proper oracle contract is functioning correctly and is properly verified by the Conditional Tokens Market participants. However, an oracleProxy contract, which is expected to be necessary to integrate oracle with ConditionalTokens.sol API allows for a, correct but peculiar, execution of the reportPayouts function.

It allows executing reportPayouts (https://github.com/gnosis/conditional-tokens-contracts/blob/master/contracts/ConditionalTokens.sol#L78) with payouts which represent the outcome of some condition correctly, insofar as relations between values in payouts go, i.e. the payoutNumerators are correct relatively. For example, for an [A, B] condition, if A happened, oracleProxy will be allowed by the oracle to reportPayouts with payouts equal [1, 0], [1000, 0], [any_arbitrarily_large_number, 0], etc.

reportPayouts rightfully covers the case where payouts would hold values which would overflow, if summed to become payoutDenominator (https://github.com/gnosis/conditional-tokens-contracts/blob/master/contracts/ConditionalTokens.sol#L89). However, if payouts holds values like [2^240, 0], while still admissible by the oracle contract, they might cause collateral tokens to be effectively locked in the instance of the ConditionalTokens.sol.

This is due to the fact, that in redeemPositions, we multiply payoutStake.mul(payoutNumerator) (https://github.com/gnosis/conditional-tokens-contracts/blob/master/contracts/ConditionalTokens.sol#L242). For payoutNumerator equal 2^240, only positions with miniscule payoutStake (2^16) would be able to correctly redeem, others exceeding that would cause a revert.

It might be argued, that it is outside of the scope of ConditionalTokens.sol responsibility, but instead a responsibility of the participants of this particular questionId, and the oracle/oracleProxy contracts. However, it might not be immediately evident, that the oracleProxy can be used in a way which allows for the lock up to happen on ConditionalTokens.sol end. Keeping the absolute values of the payouts vector small, seems out of the scope of responsibility of oracle/oracleProxy.

I think ConditionalTokens.sol, should protect the internal coherence of its state, by not admitting data, which might lead to data corruption and indefinite lock-up. For example, in the above [2^240, 0] scenario, it will never be possible to submit a correct vector [1, 0], after the large one has been submitted.

The fix here would be simple, to explicitly require payouts vector to have values smaller than a safe threshold like 2^64, in the reportPayouts function (https://github.com/gnosis/conditional-tokens-contracts/blob/master/contracts/ConditionalTokens.sol#L78). This would allow for usual values for payoutStake (assuming 10^18 being the usual token denomination) to always comfortably fit under the overflow threshold in redeemPositions.