Sablier
Sablier
DeFiFoundry
53,440 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Excessive Gas Consumption in `withdrawMultiple` Function in `SablierV2Lockup` contract

kind0devpro

Summary

The withdrawMultiple function in the SablierV2Lockup contract presents a potential vulnerability due to its high gas consumption. This function, which loops over arrays of streamId and amount, can become very expensive to execute if the arrays are large, possibly leading to failed transactions if the gas limit is exceeded.

Vulnerability Details

The withdrawMultiple function iterates through the provided arrays of streamId, recipients, and amounts, calling the withdraw function for each element. This linear increase in operations can result in high gas usage, especially with large input arrays. If the total gas required exceeds the block gas limit, the transaction will fail.

function withdrawMultiple(
uint256[] calldata streamIds,
uint128[] calldata amounts
)
public
override
noDelegateCall
notNull(streamIds)
updateMetadata(streamIds)
{
....
// Iterate over the provided array of stream IDs, and withdraw from each stream to the recipient.
for (uint256 i = 0; i < streamIdsCount; ++i) {
// Checks, Effects and Interactions: check the parameters and make the withdrawal.
withdraw({ streamId: streamIds[i], to: _ownerOf(streamIds[i]), amount: amounts[i] });
}
}

Proof of Concept

Adding this code to the withdrawMultiple.t.sol file

function test_WithdrawMultipleCostToMuchGas()
external
whenNotDelegateCalled
whenArrayCountsAreEqual
whenArrayCountsNotZero
givenNoNull
givenNoDepletedStream
whenToNonZeroAddress
whenNoAmountZero
whenNoAmountOverdraws
{
vm.txGasPrice(1);
// Simulate the passage of time.
vm.warp({ newTimestamp: earlyStopTime });
​
uint256 testIds = 25;
uint128 testAmount = 25;
​
testStreamIds = new uint256[](testIds);
for(uint256 i = 0; i < testIds; i++){
testStreamIds[i] = createDefaultStream();
}
​
// Run the test with the caller provided in {whenCallerAuthorizedAllStreams}.
resetPrank({ msgSender: caller });
​
// Expect the withdrawals to be made.
testAmounts = new uint128[](testAmount);
for(uint256 i = 0; i < testAmount; i++){
testAmounts[i] = defaults.WITHDRAW_AMOUNT();
}
​
for(uint256 i = 0; i < testAmount; i++){
expectCallToTransfer({ to: users.recipient, value: testAmounts[i] });
}
​
uint256 gasStart = gasleft();
// Make the withdrawals.
lockup.withdrawMultiple({ streamIds: testStreamIds, amounts: testAmounts });
uint256 gasEnd = gasleft();
​
uint256 gasUsedFirst = (gasStart - gasEnd) * tx.gasprice;
console.log("gas used streams withdrawal:", gasUsedFirst);
​
assert(true);
}

Changing the parameters of uint256 testIds = 25 and uint128 testAmount = 25, with 25 and 50 we can see the gas consumption of the withdrawMultiple function.
Test for 25 streams and 50 streams here are the results:

// WithdrawMultiple_LockupLinear_Integration_Concrete_Test
// [PASS] test_WithdrawMultipleCostToMuchGas() (gas: 9644352)
// Logs: ** 25 streams **
// gas used streams withdrawal: 37_0830
// Logs: ** 50 streams **
// gas used streams withdrawal: 73_0195
​
// WithdrawMultiple_LockupDynamic_Integration_Concrete_Test
// [PASS] test_WithdrawMultipleCostToMuchGas() (gas: 12668531)
// Logs: ** 25 streams **
// gas used streams withdrawal: 75_3356
// Logs: ** 50 streams **
// gas used streams withdrawal: 149_7624
​
// WithdrawMultiple_LockupTranched_Integration_Concrete_Test
// [PASS] test_WithdrawMultipleCostToMuchGas() (gas: 13205610)
// Logs: ** 25 streams **
// gas used streams withdrawal: 43_8026
// Logs: ** 50 streams **
// gas used streams withdrawal: 86_5353
​

Impact

High gas consumption due to the withdrawMultiple function's design can lead to:

  1. Transaction Failures: Users attempting to withdraw from multiple streams might experience failed transactions if the gas required exceeds the block limit.

  2. High Transaction Costs: Even if the transaction does not fail, users may incur high gas fees, making the function impractical for large arrays.

  3. Reduced Usability: The potential for high costs and failures can discourage users from using this function, reducing the overall utility of the contract.

Tools Used

  • Manual Code Review: Analyzed the contract's code to identify the looping mechanism and its implications on gas usage.

  • Gas Estimation Tools: foundry gas.

Recommendations

  1. Limit Batch Size: Restrict the number of withdrawals processed in a single transaction to prevent excessive gas usage. For example, set a maximum batch size:

function withdrawMultiple(
uint256[] calldata streamIds,
uint128[] calldata amounts
)
public
override
noDelegateCall
notNull(streamIds)
updateMetadata(streamIds)
{
​
+ uint256 maxBatchSize = 10; // Set an appropriate limit
+ require(streamIds.length <= maxBatchSize, "Exceeds maximum batch size");
....
​
// Iterate over the provided array of stream IDs, and withdraw from each stream to the recipient.
for (uint256 i = 0; i < streamIdsCount; ++i) {
// Checks, Effects and Interactions: check the parameters and make the withdrawal.
withdraw({ streamId: streamIds[i], to: _ownerOf(streamIds[i]), amount: amounts[i] });
}
}

  1. Optimize Withdraw Function: Ensure that the withdraw function is as gas-efficient as possible by minimizing state changes and external calls:

  2. Alternative Solutions: Explore off-chain computation and batching techniques where feasible to reduce on-chain operations and gas costs.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Info/Gas/Invalid as per Docs

https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

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