Flow

Sablier
FoundryDeFi
20,000 USDC
View results
Submission Details
Severity: medium
Invalid

Insufficient validation in the `calculateAmountsFromFee` function thus causing an underflow revert.

The calculateAmountsFromFee function in the Helpers library can encounter an underflow error when small values of totalAmount are input. This occurs because the calculated feeAmount, derived from the specified fee percentage, may round to a value equal to or exceeding totalAmount. When feeAmount is subtracted from totalAmount to calculate netAmount, this causes a revert due to underflow. This flaw can disrupt operations dependent on calculateAmountsFromFee, as seen in the _withdraw function, potentially impacting withdrawal functionality.

https://github.com/Cyfrin/2024-10-sablier/blob/8a2eac7a916080f2022527408b004578b21c51d0/src/libraries/Helpers.sol#L13
https://github.com/Cyfrin/2024-10-sablier/blob/8a2eac7a916080f2022527408b004578b21c51d0/src/SablierFlow.sol#L848

Impact:

If the underflow occurs, the _withdraw function may revert, denying users the ability to withdraw small amounts from their streams. This can impair user experience, especially in cases involving low stream balances, though it does not lead to a direct loss of funds.

Proof of Concept:

Used an existing test suite in the withdraw.t.sol under the concrete folder.

function test_WhenCallerRecipient()
external
whenNoDelegateCall
givenNotNull
whenAmountNotZero
whenWithdrawalAddressNotZero
whenWithdrawalAddressNotOwner
{
// It should withdraw.
_test_Withdraw({
streamId: defaultStreamId,
to: users.eve,
depositAmount: DEPOSIT_AMOUNT_6D,
protocolFeeAmount: 2e18,
withdrawAmount: 1e18
});
}

Here you can see that the protocolFeeAmount > withdrawAmount.

When the following function is executed using forge test --mt test_WhenCallerRecipient -vvvvv.

These are the following error logs:

Failing tests:
Encountered 1 failing test in tests/integration/concrete/withdraw/withdraw.t.sol:Withdraw_Integration_Concrete_Test
[FAIL: panic: arithmetic underflow or overflow (0x11)] test_WhenCallerRecipient() (gas: 147701)

Meaning that an underflow revert was reached.

Recommended Mitigation:

Introduce a check in calculateAmountsFromFee to verify that feeAmount does not exceed totalAmount. If feeAmount equals or surpasses totalAmount, netAmount should be set to zero, or a specific error should be returned, indicating the amount is too small for fee deduction.

So in the Helpers.sol place this changes in the calculateAmountsFromFee function in the Helpers lib.

function calculateAmountsFromFee(
uint128 totalAmount,
UD60x18 fee
)
internal
pure
returns (uint128 feeAmount, uint128 netAmount)
{
feeAmount = ud(totalAmount).mul(fee).intoUint128();
- netAmount = totalAmount - feeAmount;
+ if (feeAmount >= totalAmount) {
+ netAmount = 0;
+ } else {
+ netAmount = totalAmount - feeAmount;
+ }
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.