Sablier

Summary

The checkAndCalculateBrokerFee function uses assert instead of require to check that the total amount is greater than the broker fee amount. This practice is not recommended because assert is intended for internal consistency checks and invariants, while require is designed for input validation and conditions that depend on external factors. Using assert in this context can lead to higher gas consumption if the condition fails.

Vulnerability Details

Consider the following code snippet from the checkAndCalculateBrokerFee function:

Impact

The primary impact of using assert instead of require is related to gas consumption:

1. Higher Gas Costs on Failure: When an assert statement fails, it consumes all remaining gas, whereas a failed require statement refunds the remaining gas. This could result in higher gas costs for users if the condition fails unexpectedly.

2. Misleading Error Handling: assert is typically used for conditions that should never fail, such as internal invariants. Using assert for input validation can be misleading, suggesting that the failure is due to a bug in the code rather than invalid user input.

Tools Used

Manual review

Recommendations

Replace assert with require to properly handle the validation and ensure that the remaining gas is refunded if the condition fails:

Recommended code changes:

Description of Changes

1. Replaced the assert statement with a require statement to ensure that the total amount is greater than the broker fee amount.
   This change ensures that the transaction fails gracefully with a refund of remaining gas if the condition is not met.

2. Included a check to ensure that the broker fee amount does not exceed the maximum value for uint128.
   This addition helps prevent overflow issues and ensures the correctness of the broker fee calculation.