LeFy - Using Pausable instead of PausableUpgradeable in a UUPSUpgradeable contract can lead to issues

[sherlock-admin3]

LeFy

Medium

Using Pausable instead of PausableUpgradeable in a UUPSUpgradeable contract can lead to issues

Summary

Th Ethos contracts follow the OpenZeppelin's UUPSUpgradeable standard, but its using the non-upgradeable Pausable instead of PausableUpgradeable and this can lead to potential issues with pausing functionality.

Root Cause

All the Ethos contracts are UUPSUpgradeable and inherits the AccessControl.sol and this inherits the Pausable which shouldnt be used ideally along with upgradable contracts according to OpenZeppelin's standard as contracts with constructors shouldnt be used with upgradable contracts:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import { IPausable } from "../interfaces/IPausable.sol";
import { IContractAddressManager } from "../interfaces/IContractAddressManager.sol";
import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol";
import { AccessControlEnumerable } from "@openzeppelin/contracts/access/extensions/AccessControlEnumerable.sol";
import { SignatureControl } from "./SignatureControl.sol";
import { ETHOS_INTERACTION_CONTROL } from "./Constants.sol";

/**
 * @dev Contract module that allows children to restrict access to run functions
 * by service account only.
 */
abstract contract AccessControl is IPausable, Pausable, AccessControlEnumerable, SignatureControl {

[Repo Link](https://github.com/sherlock-audit/2024-10-ethos-network/blob/db37b9dc2b792e245eb683d8a956bcb7ef2f1a27/ethos/packages/contracts/contracts/utils/AccessControl.sol#L1C1-L15C100

According to OpenZeppelin's standard for writing upgradable contracts, " Due to a requirement of the proxy-based upgradeability system, no constructors can be used in upgradeable contracts." "Keep in mind that this restriction affects not only your contracts, but also the contracts you import from a library. This means you should not be using these contracts in your OpenZeppelin Upgrades project. Instead, make sure to use @openzeppelin/contracts-upgradeable, which is an official fork of OpenZeppelin Contracts that has been modified to use initializers instead of constructors" Link to OpenZeppelin doc Constructor Caveat

But since the Ethos contracts use Pausable which has a construtor , the code in the constructor wont get executed :

    constructor() {
        _paused = false;
    }

Repo Link

As a result, the pausable functionality might not be initialized properly and might cause unexpected issues, which can in turn lead to other issues for eg during upgrade the EthosProfile.sol might not get initialized properly as it calls internal functions during its initialization (_createProfile(owner);)which have the whenNotPaused modifier checks and that might fail.

  function initialize(
    address owner,
    address admin,
    address expectedSigner,
    address signatureVerifier,
    address contractAddressManagerAddr
  ) external initializer {
    __accessControl_init(
      owner,
      admin,
      expectedSigner,
      signatureVerifier,
      contractAddressManagerAddr
    );
    __UUPSUpgradeable_init();
    maxNumberOfInvites = 2048;
    maxNumberOfAddresses = 128;
    profileCount = 1; // no profiles with id 0
    _createProfile(owner);
    profiles[1].inviteInfo.available = 10;
    // only the origin can invite themself
    profiles[1].inviteInfo.invitedBy = 1;
  }

Repo Link

Impact

The pausable functionality might cause unexpected issues as Pausable is not compatible with upgradable contracts

Mitigation

Use PausableUpgradeable instead of Pausable