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).

renounceOwnership

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.

remediation

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.