Description

ManagementAccount uses OpenZeppelin's AccessControl for permissions. Functions protected by the onlyOwner modifier check OWNER_ROLE via hasRole():

function _checkOwner() internal view {
!   if (!hasRole(OWNER_ROLE, msg.sender)) revert ManagementAccountErrors.NotOwner(msg.sender);
}

However, the owner() view function returns a cached _owner storage variable that is only set during initialization and never updated:

function initialize(address owner_, address factory_) external initializer {
    [...]
!   _owner = owner_;
    [...]
    _setupRole(DEFAULT_ADMIN_ROLE, owner_);
    _setupRole(OWNER_ROLE, owner_);
    [...]
}

function owner() external view returns (address) {
!   return _owner;
}

Since DEFAULT_ADMIN_ROLE can grant OWNER_ROLE to new addresses and revoke it from the original owner, the actual owner can change through role management. When this happens, _checkOwner() enforces the new role holder, but owner() continues returning the original address, creating a mismatch between the view function and actual access control.

Impact

Any off-chain or on-chain systems relying on the owner to identify the current account owner will receive incorrect information, potentially leading to authorization failures, incorrect UI displays, or operational issues.

Recommendations

Update owner() to derive the owner from OWNER_ROLE instead of the storage variable:

function owner() external view returns (address) {
-   return _owner;
+   // Get the first member of OWNER_ROLE
+   // Note: This assumes single owner; adjust if multiple owners are supported
+   return getRoleMember(OWNER_ROLE, 0);
}

Remediation

This issue has been acknowledged by Hyperbeat, and a fix was implemented in commit 811642af↗. The owner view function has now been removed.