Submitted by axic, also found by gpersoon, pauliax, [Jmukesh](https://twitter.com/MukeshJeth), a_delamo, s1m0, cmichel, and shw_
Some major tokens went live before ERC20 was finalized, resulting in a discrepancy whether the transfer functions should (A) return a boolean or (B) revert/fail on error. The current best practice is that they should revert, but return true on success. However, not every token claiming ERC20-compatibility is doing this  some only return true/false; some revert, but do not return anything on success. This is a well known issue, heavily discussed since mid-2018.
Today many tools, including OpenZeppelin, offer a wrapper for safe ERC20 transfer:
RealityCards is not using such a wrapper, but instead tries to ensure successful transfers via the balancedBooks modifier:
This modifier is present on most functions, but is missing on topupMarketBalance:
In the case where an ERC20 token which is not reverting on failures is used, a malicious actor could call topupMarketBalance with a failing transfer, but also move the value of marketBalance above the actual holdings. After this, deposit, withdrawDeposit, payRent, payout, sponsor, etc. could be locked up and always failing with Books are unbalanced.
Anyone can call topupMarketBalance with some unrealistically large number, so that marketBalance does not overflow, but is above the actually helping balances. This is only possible if the underlying ERC20 used is not reverting on failures, but is returning false instead.
Recommended Steps:
Splidge (Reality Cards) confirmed:
Splidge (Reality Cards) resolved:
