DoS in _retrieve() function if the withdrawal from the Sablier is zero

Summary

When the _retrieve() function is called, if the withdrawal amount from the Sablier is zero, the _retrieve() function will always be reverted.

Vulnerability Detail

The _retrieve() function is used to retrieve and distribute TELCOIN to council members based on the stream from _target, in this example I assume the _target was Sablier stream.

function _retrieve() internal {
    // Get the initial TELCOIN balance of the contract
    uint256 initialBalance = TELCOIN.balanceOf(address(this));
    // Execute the withdrawal from the _target, which might be a Sablier stream or another protocol
    _stream.execute(
        _target,
        abi.encodeWithSelector(
            ISablierV2ProxyTarget.withdrawMax.selector,
            _target,
            _id,
            address(this)
        )
    );

    // Get the new balance after the withdrawal
    uint256 currentBalance = TELCOIN.balanceOf(address(this));
    // Calculate the amount of TELCOIN that was withdrawn during this operation
    uint256 finalBalance = (currentBalance - initialBalance) +
        runningBalance;
    // Distribute the TELCOIN equally among all council members
    uint256 individualBalance = finalBalance / totalSupply();
    // Update the running balance which keeps track of any TELCOIN that can't be evenly distributed
    runningBalance = finalBalance % totalSupply();

    // Add the individual balance to each council member's balance
    for (uint i = 0; i < balances.length; i++) {
        balances[i] += individualBalance;
    }
}

At lines 271-279, this function will execute the withdrawal from the Sablier stream by calling the withdrawMax() function.

By invoking the _retrieve() function, the maximum withdrawable amount is withdrawn from the stream. Consequently, there is no remaining withdrawal amount for the CouncilMember contract, leading to the next withdrawal from this contract having a maximum amount of zero according to the Sablier documentation:(https://docs.sablier.com/contracts/v2/reference/core/abstracts/abstract.SablierV2Lockup#withdrawmax)

the withdrawMax() is used to withdraw the maximum withdrawable amount from the stream to the provided address to.

function withdrawMax(uint256 streamId, address to) external override {
    withdraw({ streamId: streamId, to: to, amount: _withdrawableAmountOf(streamId) });
}

https://github.com/sablier-labs/v2-core/blob/b0016437ef3cc8606e1100965dd911d7e658b40b/src/abstracts/SablierV2Lockup.sol#L297-L299

However, if the _retrieve() function is called again, it calls withdrawMax() with zero amounts, causing the function to revert according to the Sablier documentation: (https://docs.sablier.com/contracts/v2/reference/core/abstracts/abstract.SablierV2Lockup#withdraw)

amount must be greater than zero and must not exceed the withdrawable amount.

    // Checks: the withdraw amount is not zero.
    if (amount == 0) {
        revert Errors.SablierV2Lockup_WithdrawAmountZero(streamId);
    }

https://github.com/sablier-labs/v2-core/blob/b0016437ef3cc8606e1100965dd911d7e658b40b/src/abstracts/SablierV2Lockup.sol#L269-L272

As a result, the _retrieve() function will revert during the second invocation when attempting to withdraw zero amounts.

Additionally, the _retrieve() function is highly likely to be called since most functions in the CouncilMember contract execute this function before their operation.

Impact

If the withdraw amount of CouncilMember contract is zero, most of function in CouncilMember contract that call retrieve() function will failed.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L270-L279

Tool used

Manual Review

Recommendation

Skipping the withdraw from Sabiler if the withdraw amount is zero.

Duplicate of #47

sherlock-audit / 2024-01-telcoin-judging

Ignite - DoS in _retrieve() function if the withdrawal from the Sablier is zero #51