code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. _split() will mostly fail/revert on small-weighted receivers due to underflow. This is because it calculates the split amount for a receiver in the following way, which will underflow for small-weighted receivers.

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

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Here is an example of how _split() will fail/revert on small-weighted receivers due to underflow:

1) Suppose the total balance is 100, and to be distributed to receivers A (50%), B (40%) and C(10%). Therefore, A will get 50, B will get 40, and C will get 10.

2) When it is time to calculate the amount for B, we have: collectableAmt = 100, _TOTAL_SPLITS_WEIGHT =1_000_000, splitsWeight = 400_000, and splitAmt = 50, As a resul,t we have 40-50, an underflow.

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

3) _split() will fail/revert on B, C will not be processed at all.

Tools Used

Remix

Recommended Mitigation Steps

We need to correct the way of calculating the split amount of a receiver as follows:

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

xmxanuel commented 1 year ago

This is a misunderstanding how the formular is working with:

splitWeight
collectableAmt

Testcase with your Example

works correctly

function testWeights() public {
    uint weightA = 500_000;
    uint weightB = 400_000;
    uint weightC = 100_100;

    uint _TOTAL_SPLITS_WEIGHT =1_000_000;

    uint collectableAmt = 100;
    uint splitAmt = 0;
    uint splitWeights = weightA;

    uint128 a =
            uint128((uint160(collectableAmt) * splitWeights) / _TOTAL_SPLITS_WEIGHT - splitAmt);
    assertEq(a, 50);
    splitAmt += a;
    splitWeights+= weightB;
    uint b =  uint128((uint160(collectableAmt) * splitWeights) / _TOTAL_SPLITS_WEIGHT - splitAmt);
    assertEq(b, 40);
    splitAmt += b;
    splitWeights+= weightC;
    uint c =  uint128((uint160(collectableAmt) * splitWeights) / _TOTAL_SPLITS_WEIGHT - splitAmt);
    assertEq(c, 10);
}