Flow

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

Critical Privileges Transferred Without Address Validation in Single-Step Process

Relevant GitHub Links

Summary:

The Admin role, which holds critical privileges within the protocol, can be transferred to another address in a single step through the Adminable::transferAdmin function. However, this function lacks a validation check for a zero address (address(0)), posing a significant risk. If mistakenly set to the zero address, critical admin-only functions such as collectProtocolRevenue would be inaccessible, permanently locking protocol revenue.

Relevant Code:
Adminable::transferAdmin

function transferAdmin(address newAdmin) public virtual override onlyAdmin {
// Effect: update the admin.
admin = newAdmin;
// Log the transfer of the admin.
emit IAdminable.TransferAdmin({ oldAdmin: msg.sender, newAdmin: newAdmin });
}

Impact

Transferring Admin privileges without checking for address(0) introduces two main security and functionality risks:

  1. Mistaken Assignment to Zero Address: If Adminable::transferAdmin sets admin to the zero address by accident or due to a clipboard replacement attack, critical privileges would effectively be lost. This includes the ability to execute functions like collectProtocolRevenue, which are vital to protocol operations. Locked revenue would remain inaccessible indefinitely, impacting protocol sustainability.

  2. Single-Step Transfer Risk: As the transfer occurs in a single step without requiring confirmation, there is an increased chance of transferring Admin privileges to an unintended address. This could happen due to errors or malicious interference, risking control over essential protocol functions.

Tools Used

  • Manual Review

Recommended Mitigation Steps

  1. Add a check in Adminable::transferAdmin to ensure newAdmin is not address(0). This prevents accidental or malicious assignment of the zero address and safeguards admin functionality. If the protocol intends to allow an admin removal feature by setting admin to address(0) to operate without an admin, it is still advisable to implement the two-step confirmation outlined in point two. This extra layer ensures that setting the admin to zero is an intentional and verified action, mitigating accidental removal risks.

  2. Implement a two-step admin transfer process, where the new admin must explicitly accept the role before the transfer is finalized. This additional confirmation reduces the risk of errors and improves control over critical privileges.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
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.