Flow

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

Flow NFT should not be burnable especially when a Flow has not been voided but this is not prevented

Summary

The Flow NFT architecture specifies that NFTs minted under this system should not be burnable, as burning an NFT would disrupt intended functionality and may result in the permanent loss of user funds. However, in the current implementation, there is no restriction on burning by transferring the NFT to the zero address. Consequently, if an NFT is marked as transferable, it can be inadvertently burned by the recipient, violating the intended behavior.

Vulnerability Details

1. **Burnability of Flow NFTs**:

- The `_update` function is intended to prevent transfers to the zero address to safeguard against accidental burns. However, the implementation currently lacks a restriction for transfers to the zero address, making the Flow NFT burnable.

- As a result, if the NFT is transferable, a recipient or approved third party may send the NFT to the zero address, effectively burning it, which is counter to the intended immutable nature of Flow NFTs.

2. **Code Analysis**:

/// @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(
@audit>> address to,
uint256 streamId,
address auth
)
internal
override
updateMetadata(streamId)
returns (address)
{
address from = _ownerOf(streamId);
@audit>no check for to > if (from != address(0) && !_streams[streamId].isTransferable) {
revert Errors.SablierFlowBase_NotTransferable(streamId);
}
return super._update(to, streamId, auth);
}
}

- The `_update` function currently checks the `isTransferable` flag and permits transfers if the NFT is transferable. However, it does not include a check to revert transactions where the `to` address is set to zero.

- This omission allows a transferrable Flow NFT to be sent to the zero address, thus being permanently lost without recovery, which directly contradicts the design.

Impact

The ability to burn Flow NFTs presents several risks and potential issues:

- **Loss of Funds**: The burnability of Flow NFTs may lead to the unintended destruction of assets, leading to irrecoverable loss of funds for users (Sender).

- **Broken Functionality**: Burning disrupts the Flow architecture’s operational structure, causing failures in processes that rely on active Flow NFTs.

Tools Used

Manual Review

Recommendations

1. **Restrict Transfers to Zero Address**:

- Modify the `_update` function to include a check that reverts any transactions where the `to` address is set to zero. This will ensure that recipients or approved parties cannot burn Flow NFTs by transferring them to the zero address.

Or a transfer to the to address 0 should be prevented as long as the NFT has not been voided.

function _update(
address to,
uint256 streamId,
address auth
)
internal
override
updateMetadata(streamId)
returns (address)
{
++ require(to != address(0), "Flow NFTs cannot be burned");
address from = _ownerOf(streamId);
if (from != address(0) && !_streams[streamId].isTransferable) {
revert Errors.SablierFlowBase_NotTransferable(streamId);
}
return super._update(to, streamId, auth);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 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.