Flow

Summary

In SablierFlowBase.sol, an unchecked subtraction in the collectProtocolRevenue function can cause an underflow in the aggregateBalance of a token when the admin attempts to collect protocol revenue. Which could cause future calls to the recover function to revert due to underflow, effectively bricking the ability to recover surplus tokens.

Vulnerability Details

The admin collects protocol revenue of a token through the collectProtocolRevenuefunction in SablierFlowBase.sol. There is an unchecked block in this function to subtract the aggregateBalanceof a token with the protocol revenue of the token.

The _withdraw function in SablierFlow.solis used by user to withdraw deposit from a stream. In this function the aggregateBalanceof a token is subtracted from the withdraw amount minus the protocol fee. The protocol fee is added to the protocolRevenueof that token.

POC

Consider this scenario:

1. User 1 create a stream with 100 USDC of deposits, the recipient is User 1.

2. User 1 withdraws 100 USDC from the stream, some amount is deducted from the withdraw amount due to protocol fee and added to protocol revenue for that token. Protocol fee is 1% , henceprotocolRevenueis now 1 USDC.

3. User 1 create a stream again with 100 USDC of deposits, the recipient is User 1.

4. Admin wants to collect the protocol revenue but User 1 frontrun the admin and withdraws all 100 USDC from the stream. protocolRevenueis now 2 USDC.

5. The aggregateBalanceis 0 USDC is subtracted from the protocolRevenue which is 2 USDC.

6. Admin successfully collects the protocol revenue but now the aggregateBalance of that token is type(uint256).max.

7. When admin needs to recover some surplus of a token through the recoverfunction it will most likely always revert because the token balance of the contract is subtracted from the aggregateBalanceof that token which is now a very large value. This will cause underflow and revert the function call.

Impact

The recoverfunction in SablierFlowBase.solis bricked for a certain token as a result surplus of that token cannot be recovered.

Note, that the aggregateBalancewill possibly be back to normal once another stream with that token is created and the stream receive deposit which will caused unchecked overflow for the aggregateBalanceand possibly the value will be back to 0 or more. However, the more protocol revenue collected the more tokens required to return the aggregate balance back to normal.

Tools Used

Manual review

Recommendations

Add this check in the collectProtocolRevenuefunction: