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

L-1 Missing checks for `address(0)` when assigning a new admin

topg

Summary

Missing checks for address(0) when assigning a new admin.
The Adminable::transferAdmin function is intended to set the public Adminable::admin to the newAdmin, However it does not check if newAdmin is the zero address.

Vulnerability Details

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

Impact

The zero address is not a valid user address in Ethereum. If the admin is set to the zero address, no one will be able to perform admin-only functions since no one can have the zero address as their address.

Proof of Concept

The below test shows how we can assign the zero address to be the new admin.

  1. First we set up the contract.

function setUp() public {
admin = makeAddr("admin");
vm.startPrank(admin);
adminable = new AdminableMock(admin);
vm.stopPrank();
}

  1. We create our test.

function testTransferAdmin_ZeroAddress() public {
address zeroAddrs = address(0);
vm.startPrank(admin);
console.log("old admin: ", adminable.admin());
adminable.transferAdmin(zeroAddress);
assertEq(adminable.admin(), zeroAddress);
console.log("new admin: ", adminable.admin());
}

  1. We run our test in the terminal.

forge test --mt testTransferAdmin_ZeroAddress -vvv

  1. result.

Ran 1 test for test/integration/concrete/admin/admin.t.sol:AdminableTest
[PASS] testTransferAdmin_ZeroAddress() (gas: 21670)
Logs:
old admin: 0xaA10a84CE7d9AE517a52c6d5cA153b369Af99ecF
new admin: 0x0000000000000000000000000000000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.69ms (100.67µs CPU time)

Tools Used

Foundry, Aderyn

Recommendations

Add the following check to the Adminable::transferAdmin function.

/// @inheritdoc IAdminable
function transferAdmin(address newAdmin) public virtual override onlyAdmin {
+ require(newAdmin != address(0), "new admin == zero adress!");
// Effect: update the admin.
admin = newAdmin;
​
// Log the transfer of the admin.
emit IAdminable.TransferAdmin({ oldAdmin: msg.sender, newAdmin: newAdmin });
}
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.