Summary
In SablierFlowBase contract contains an issue where its _update function allows NFT transfers to the zero address (burning), despite documentation explicitly stating this should be prevented. This could lead to accidental loss of stream NFTs.
Vulnerability Details
https://github.com/Cyfrin/2024-10-sablier/blob/963bf61b9d8ffe3eb06cbcf1c53f0ab88dbf0eb0/src/abstracts/SablierFlowBase.sol#L312-L330
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 function's documentation (line 304) states:
https://github.com/Cyfrin/2024-10-sablier/blob/963bf61b9d8ffe3eb06cbcf1c53f0ab88dbf0eb0/src/abstracts/SablierFlowBase.sol#L304
but, the implementation does not include any checks to prevent transfers to the zero address.
Impact
This issue allows stream NFTs to be accidentally or intentionally burned, leading to loss of stream access.
function testZeroAddressTransferBug() public {
vm.prank(admin);
uint256 streamId = sablier.createTestStream(user1);
vm.prank(user1);
vm.expectEmit(true, true, true, true);
emit Transfer(user1, address(0), streamId);
address previousOwner = sablier.exposed_update(address(0), streamId, user1);
assertEq(previousOwner, user1, "Previous owner should be user1");
vm.expectRevert();
sablier.ownerOf(streamId);
}
Test Results:
[PASS] testZeroAddressTransferBug() (gas: 87305)
Logs:
BUG DETECTED: _update function allows transfer to zero address despite documentation
Severity: Medium
Impact: Stream NFTs can be accidentally burned
Location: SablierFlowBase._update()
Expected: Transfers to zero address should be prevented
Actual: _update() allows transfers to zero address
Tools Used
Foundry Framework
Manual Code Review
Custom Test Suite
Recommendations
Add an explicit check in the _update function to prevent transfers to the zero address:
function _update(
address to,
uint256 streamId,
address auth
)
internal
override
updateMetadata(streamId)
returns (address)
{
address from = _ownerOf(streamId);
if (to == address(0)) {
revert Errors.SablierFlowBase_InvalidRecipient(to);
}
if (from != address(0) && !_streams[streamId].isTransferable) {
revert Errors.SablierFlowBase_NotTransferable(streamId);
}
return super._update(to, streamId, auth);
}
