Flow
Sablier
FoundryDeFi
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Lack of Address Zero Check in `_update` Function for `to` Address

brene

Summary

The _update function lacks a check to ensure that the to address (the recipient address for a stream transfer) is not set to the zero address (address(0)). Although the update function comment suggests that transfers to the zero address are prohibited to prevent accidental burns, there is no corresponding validation for the to address. This could allow streams to be transferred to address(0), resulting in asset loss or locked funds.

Vulnerability Details

The _update function is intended to facilitate updates to the stream, including transfers, is responsible for initializing a new stream.

/// @notice Overrides the {ERC-721._update} function to check that the stream is transferable.
///
/// @dev The transferable flag is ignored if the current owner is 0, as the update in this case is a mint and
/// is allowed. Transfers to the zero address are not allowed, preventing accidental burns.
///
/// @param to The address of the new recipient of the stream.
/// @param streamId ID of the stream to update.
/// @param auth Optional parameter. If the value is not zero, the overridden implementation will check that
/// `auth` is either the recipient of the stream, or an approved third party.
///
/// @return The original recipient of the `streamId` before the update.
function _update(
address to,
uint256 streamId,
address auth
)
internal
override
updateMetadata(streamId)
returns (address)
{
address from = _ownerOf(streamId);
​
if (from != address(0) && !_streams[streamId].isTransferable) {
revert Errors.SablierFlowBase_NotTransferable(streamId);
}
​
return super._update(to, streamId, auth);
}

The update function comments specifies that “Transfers to the zero address are not allowed, preventing accidental burns.” However, while a check is implemented for from address, there is no similar check for to address. The omission of to != address(0) allows the possibility of transferring streams to the zero address, contrary to the intended behavior specified in the update function comment.

Impact

Allowing transfers to the zero address can result in the loss of funds, as no one would be able to access or reclaim streams sent there. This could lead to user asset loss and decreased trust in the protocol, especially if the protocol processes a significant number of transfers to zero address.

Tools Used

Manual Review

Recommendations

Add a validation check to ensure that the recipient address (to) is not set to address(0).

function _update(
address to,
uint256 streamId,
address auth
)
internal
override
updateMetadata(streamId)
returns (address)
{
address from = _ownerOf(streamId);
​
​
- if (from != address(0) && !_streams[streamId].isTransferable) {
+ if (from != address(0) && to != address(0) && !_streams[streamId].isTransferable) {
revert Errors.SablierV2Lockup_NotTransferable(streamId);
}
​
​
return super._update(to, streamId, auth);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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