Flow

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

Lack of Validation for Zero Address in transferAdmin Function

Summary

A critical vulnerability exists in the SablierFlow smart contract where the transferAdmin function lacks validation to prevent setting the admin address to the zero address (0x0000000000000000000000000000000000000000). This oversight allows the contract to become ungovernable, as administrative privileges can be inadvertently or maliciously transferred to an invalid address. Without a valid admin, essential administrative functions cannot be performed, potentially locking the contract in an unusable state.

Vulnerability Details

Affected Function: transferAdmin

In the SablierFlow contract, the transferAdmin function is responsible for transferring administrative privileges to a new address. The function currently does not validate the newAdmin parameter, allowing it to be set to the zero address (address(0)). Setting the admin to the zero address effectively removes any administrative control over the contract, as no entity can possess the zero address.

Issue Explanation:

  • No Validation for Zero Address: The function lacks a check to ensure newAdmin is not the zero address.

  • Loss of Governance: Once the admin is set to address(0), no further administrative actions can be taken, including transferring the admin role to a valid address.

  • Irreversible State: The contract becomes permanently ungovernable unless there's an alternative mechanism to regain administrative control.

Proof of Concept (PoC):

The following PoC demonstrates the vulnerability with detailed logs and comments for clarity, to run this test, create a file called SablierFlowAdminZeroAddressPOC.t.sol in /tests directory

Then run it using this command:

forge test --mt testTransferAdminToZeroAddress -vvvv
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22;
import "forge-std/src/Test.sol";
import "../src/SablierFlow.sol";
import "../src/interfaces/IFlowNFTDescriptor.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract SablierFlowAdminZeroAddressPOC is Test {
SablierFlow sablierFlow;
address initialAdmin = address(0x123); // Initial admin address
address descriptor = address(0x456); // Mock descriptor address
function setUp() public {
// Deploy the SablierFlow contract with the initial admin
vm.prank(initialAdmin);
sablierFlow = new SablierFlow(initialAdmin, IFlowNFTDescriptor(descriptor));
console.log("SablierFlow contract deployed with admin:", initialAdmin);
}
function testTransferAdminToZeroAddress() public {
// Start acting as the initial admin
vm.startPrank(initialAdmin);
console.log("Prank started as initial admin:", initialAdmin);
// Transfer admin role to the zero address
sablierFlow.transferAdmin(address(0));
console.log("Transferred admin role to zero address");
// Assert that the admin is now the zero address
address currentAdmin = sablierFlow.admin();
console.log("Current admin address:", currentAdmin);
assertEq(currentAdmin, address(0), "Admin address should be zero");
// Attempt to perform an admin-only function (should fail)
vm.expectRevert();
sablierFlow.transferAdmin(address(0x789));
console.log("Attempted to transfer admin role from zero address");
// Stop acting as initial admin
vm.stopPrank();
console.log("Prank stopped as initial admin");
}
}

Explanation:

  • Setup Phase:

    • Deploys the SablierFlow contract with initialAdmin as the admin.

    • Logs the deployment and initial admin address.

  • Test Phase (testTransferAdminToZeroAddress):

    • Starts acting as the initialAdmin.

    • Calls transferAdmin with address(0) to transfer admin privileges to the zero address.

    • Logs the transfer action.

    • Retrieves the current admin address and asserts it is address(0).

    • Attempts to transfer admin privileges again, which should revert because no one can act from the zero address.

    • Uses vm.expectRevert() to expect a revert on the failed admin action.

    • Logs each step for clarity.

Impact

  • Loss of Administrative Control: The contract becomes unmanageable as no further admin actions can be performed.

  • Security Risks: Critical functions that require admin privileges cannot be executed, potentially exposing the contract to unaddressed vulnerabilities.

  • Operational Disruption: Users relying on administrative actions (e.g., fee adjustments, contract upgrades) are affected due to the inability to perform such operations.

  • Irreversible State: Unless a backdoor or alternative mechanism exists (which is a security risk itself), the contract remains in an unusable state permanently.

Tools Used

  • Foundry: A fast, portable, and modular toolkit for Ethereum application development.

  • Solidity Compiler (solc): Version 0.8.22 used for compiling smart contracts.

  • Forge Std Library: Provides utilities for testing, including console.log for detailed output.

  • Console Logs (console.log): Used extensively in the PoC to trace execution flow and internal state.

Recommendations

Implement Validation Check in transferAdmin

Add a validation check to ensure that the newAdmin address is not the zero address. This prevents transferring administrative privileges to an invalid address.

Recommended Code Modification:

function transferAdmin(address newAdmin) public virtual override onlyAdmin {
require(newAdmin != address(0), "New admin address cannot be zero");
// Effect: update the admin.
admin = newAdmin;
// Log the transfer of the admin.
emit IAdminable.TransferAdmin({ oldAdmin: msg.sender, newAdmin: newAdmin });
}

Additional Recommendations

  • Input Validation Across Functions:

    • Ensure that all functions accepting addresses validate against the zero address where appropriate.

  • Unit Testing:

    • Add tests to confirm that functions revert when provided with invalid inputs, including the zero address.

  • Code Review and Auditing:

    • Perform thorough code reviews to identify similar oversights in other functions.

    • Consider third-party security audits to enhance contract security.

  • Documentation Updates:

    • Update the contract documentation to specify that the admin cannot be set to the zero address.

  • Emergency Recovery Mechanisms:

    • Implement a failsafe or emergency mechanism that allows recovery in case of critical failures (use with caution due to potential security implications).

Test Confirmation

After implementing the validation check, re-run the tests to confirm the fix:

forge test --match-path tests/SablierFlowAdminZeroAddressPOC.t.sol -vvvv

Expected Outcome:

  • The test testTransferAdminToZeroAddress should not pass, confirming that attempting to set the admin to the zero address results in a revert.

  • The assertion that the admin address remains unchanged when provided with the zero address should hold true.


By implementing these recommendations, the SablierFlow contract will prevent accidental or malicious transfer of administrative privileges to the zero address, maintaining governance integrity and operational functionality.

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.