Flow

Vulnerability Details

https://github.com/Cyfrin/2024-10-sablier/blob/main/src/SablierFlow.sol#L564-#L621

https://github.com/Cyfrin/2024-10-sablier/blob/main/src/SablierFlow.sol#L792-#L794

In the _create()function, streamId is consecutive increase by one:

Along with allowing set senderaddress to arbitrary address, it opened attack vector when reorg happen:

• User A create stream with id = 50, set user B as sender

• Sometimes later, user B deposit token to that streamId

• Reorg happen, attacker do these actions in one transaction:

• 1, create streamId with same id = 50due to reorg with same sender but receiver is different

• 2, approve permission of that streamId for attacker's controlled address, then transfer it to original recipient

• Later, user B deposit to that streamId, attacker can withdraw part of them due to being approved before:

  function _withdraw(

  uint256 streamId,

  address to,

  uint128 amount

  )

  internal

  returns (uint128 withdrawnAmount, uint128 protocolFeeAmount)

  {

  . . . . . . .

  if (to != _ownerOf(streamId) && !_isCallerStreamRecipientOrApproved(streamId)) { // <---

  revert Errors.SablierFlow_WithdrawalAddressNotRecipient({ streamId: streamId, caller: msg.sender, to: to });

  }

  . . . . . . .

  }

if (to != _ownerOf(streamId) && !_isCallerStreamRecipientOrApproved(streamId)) {
revert Errors.SablierFlow_WithdrawalAddressNotRecipient({ streamId: streamId, caller: msg.sender, to: to });
}Impact

Victim's token can be stolen when reorg happen

Tools Used

Manual review

Recommendations

streamIdshould be randomly generated by using caller's address and other parameters.