
Access control vulnerability in Ownable implementation
Today, let’s get a little technical and discuss an access control vulnerability surrounding a classic smart contract trait.
Vulnerable code
The following code snippet is a simplified version of a smart contract that implements the Ownable trait from OpenZeppelin. The OwnableWrapper
contract is a wrapper around the Ownable
trait, which adds an setOwner
function that sets the owner address upon contract creation.
pragma solidity ^0.8.4;
import "@openzeppelin/contracts/access/Ownable.sol";
abstract contract OwnableWrapper is Ownable {
error AlreadyInitialized();
bool private _alreadyInit;
// Called when EIP-1167 clones a contract
function setOwner(address owner_) public {
if (_alreadyInit || owner() != address(0)) {
revert AlreadyInitialized();
}
_transferOwnership(owner_);
}
}
Issue description
The OwnableWrapper
trait builds upon OpenZeppelin’s Ownable
trait, which provides the onlyOwner
modifier. This modifier restricts function calls to the owner of the contract and includes functions like transferOwnership
and renounceOwnership
.
A critical issue arises with the setOwner
function, which is publicly callable. The guard checks in place are:
- The owner address must be zero.
- The contract should not be already initialized.
However, there’s a missing piece: _alreadyInit
isn’t being set to true
within this function, which effectively leaves only the owner() != address(0)
condition to guard the function.
Issue exploitation
You might wonder in which case the owner address would be zero after initialization. If we delve into the Ownable trait, specifically the renounceOwnership
function, we see that it sets the owner to address(0)
.
This is a significant red flag 🚨 because if the owner decides to revoke their rights and renounce ownership, the setOwner
function becomes callable again, opening up the possibility for anyone to claim ownership of the contract.
Remediation
The fix for this vulnerability is straightforward: set the _alreadyInit
flag to true
after the initial owner has been set.
Conclusion
In summary, this oversight in the access control mechanism could lead to unintended consequences where ownership could be claimed by an unintended party if the original owner renounces their ownership. Always ensure state changes are properly handled when dealing with ownership permissions in smart contracts.