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

Missing checks for `address(0)` when assigning values to address state variables

KimSeokJin

Description:
There are multiple instances in the code where addresses are assigned to state variables without checking if the address is zero (address(0)). This can lead to unintended behavior if these addresses are set to zero.

Impact:
Assigning a zero address can result in loss of functionality, as certain actions might require a valid, non-zero address to operate correctly.

Check for address(0) when assigning values to address state variables.

  • Found in src/abstracts/Adminable.sol Line: 37

    // Effect: update the admin.

  • Found in src/abstracts/SablierV2Lockup.sol Line: 54

    constructor(address initialAdmin, ISablierV2NFTDescriptor initialNFTDescriptor) {

  • Found in src/abstracts/SablierV2Lockup.sol Line: 310

    try ISablierV2Recipient(recipient).onLockupStreamRenounced(streamId) { } catch { }

Recommended Mitigation:
Add checks to ensure that the address being assigned is not zero.
Adminable.sol [Line: 37]

function transferAdmin(address newAdmin) public virtual override onlyAdmin {
+ // Check: the new admin address is not zero.
+ if (newAdmin == address(0)) {
+ revert Errors.InvalidAddress();
+ }
​
// Effect: update the admin.
admin = newAdmin;
​
// Log the transfer of the admin.
emit IAdminable.TransferAdmin({ oldAdmin: msg.sender, newAdmin: newAdmin });
}

SablierV2Lockup.sol (Constructor)

constructor(address initialAdmin, ISablierV2NFTDescriptor initialNFTDescriptor) {
+ // Check: the initial admin address is not zero.
+ if (initialAdmin == address(0)) {
+ revert Errors.InvalidAddress();
+}
admin = initialAdmin;
nftDescriptor = initialNFTDescriptor;
emit TransferAdmin({ oldAdmin: address(0), newAdmin: initialAdmin });
}

SablierV2Lockup.sol (Renouncement Hook)

function renounce(uint256 streamId) external override noDelegateCall notNull(streamId) updateMetadata(streamId) {
// Check: the stream is not cold.
Lockup.Status status = _statusOf(streamId);
if (status == Lockup.Status.DEPLETED) {
revert Errors.SablierV2Lockup_StreamDepleted(streamId);
} else if (status == Lockup.Status.CANCELED) {
revert Errors.SablierV2Lockup_StreamCanceled(streamId);
} else if (status == Lockup.Status.SETTLED) {
revert Errors.SablierV2Lockup_StreamSettled(streamId);
}
​
// Check: `msg.sender` is the stream's sender.
if (!_isCallerStreamSender(streamId)) {
revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
}
​
// Checks and Effects: renounce the stream.
_renounce(streamId);
​
// Log the renouncement.
emit ISablierV2Lockup.RenounceLockupStream(streamId);
​
// Interaction: if the recipient is a contract, try to invoke the renounce hook on the recipient without
// reverting if the hook is not implemented, and also without bubbling up any potential revert.
+ if (recipient != address(0) && recipient.code.length > 0) {
try ISablierV2Recipient(recipient).onLockupStreamRenounced(streamId) { } catch { }
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Info/Gas/Invalid as per Docs

https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

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