Loss of split amount due to rounding in low-decimal tokens

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Splits.sol#L161

Vulnerability details

Impact

Split receivers can receive 0 tokens, or a lower amount of tokens than expected, due to rounding. The bug may be intentionally abused by senders to reduce the amounts received by split receivers.

Proof of Concept

A user can set splits: a list of user IDs, and their weights, who wil receive portions of funds received by the user. The amount of funds that a split receiver can receive is determined by their weight, and the total weight equals to 1,000,000:

uint128 currSplitAmt =
    uint128((uint160(collectableAmt) * splitsWeight) / _TOTAL_SPLITS_WEIGHT - splitAmt);

Thus, funds are split into millionths shares. However, tokens with the number of decimals less than 6 cannot be divided by a million, which results in the currSplitAmt being 0. One example of such tokens is GUSD, which has 2 decimals. The minimal amount of GUSD that is divisible by 1,000,000 is 10,000–amounts smaller that that will result in 0 split amount when splitsWeight is 1: (10000e2 * 1) / 1e6 = 1. However, there's no check for a minimal divisible amount. On the other hand, to split 1 GUSD, splitsWeight must be not smaller than 10,000: (1e2 * 10000) / 1e6 = 1. But there's no check for a minimal weight when setting splits.

This bug can be abused by senders to reduce the amount they send to split receivers. Consider this scenario:

1. A project has a referral system, which allows project's user to receive a share of project's income. The project uses the GUSD token for accounting.
2. The project has set up splits, with multiple users receiving 100 / 1,000,000 of service's income.
3. As soon as project's balance is getting to 99 GUSD (or 199 GUSD, or 299 USD, etc.), the project triggers splitting.
4. Due to rounding, each user receives 0 GUSD (or 100 GUSD, or 200 GUSD, etc.).
5. Since splitting moves funds from the splittable balance to the collectable balance, splitting the same funds again is not possible, despite the split receivers have received 0 tokens.

The description of the contest mentions that malicious ERC20 contracts are out of scope. However, tokens with decimals other than 18 are not malicious and not alternative implementations: they are valid ERC20 tokens, as EIP-20 doesn't set a strict requirement of the number of decimals. The GUSD token, for example, is a valid ERC-20 token with the market cap of more than $570,000,000. The tokens is a stablecoin created by Gemini, a centralized exchange.

Tools Used

Manual review

Recommended Mitigation Steps

One good and universal solutions seems to allow users to fully define the configuration of splits, including the total weight. This will free the protocol from the need to support tokens with different decimals and will give users more flexibility when creating splits. And it's only users who will be responsible for setting weights that result in 0 amounts due to rounding.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #182

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

L

[c4-judge]

GalloDaSballo marked the issue as grade-c