Submitted by kirk-baird, also found by defsec, hickuphh3, and leastwood
Certain ERC20 tokens may change users balances over time (positively or negatively) or charge a fee when a transfer is called (FoT tokens). The accounting of these tokens is not handled by RoyaltyVault.sol or Splitter.sol and may result in tokens being stuck in Splitter or overstating the balance of a user
Thus, for FoT tokens if all users tried to claim from the Splitter there would be insufficient funds and the last user could not withdraw their tokens.
The function RoyaltyVault.sendToSplitter() will transfer splitterShare tokens to the Splitter and then call incrementWindow(splitterShare) which tells the contract to split splitterShare between each of the users.
Since the Splitter may receive less than splitterShare tokens if there is a fee on transfer the Splitter will overstate the amount split and each user can claim more than their value (except the last user who claims nothing as the contract will have insufficient funds to transfer them the full amount).
Furthermore, if the token rebase their value of the tokens down while they are sitting in the Splitter the same issue will occur. If the tokens rebase their value up then this will not be accounted for in the protocol.
It is recommend documenting clearly that rebasing token should not be used in the protocol.
Alternatively, if it is a requirement to handle rebasing tokens balance checks should be done before and after the transfer to ensure accurate accounting. Note: this makes the contract vulnerable to reentrancy and so a reentrancy guard must be placed over the function sendToSplitter().
sofianeOuafir (Joyn) confirmed
