IPegToken Manual Review Findings

IPT-01M: Improper Disable of Initializers

Type	Severity	Location
Standard Conformity		IPegToken.sol:L50

Description:

The PegTokenV1Storage contract is inheriting from both OpenZeppelin's initialization model as well as the 0xPhase system's custom proxy initialization model, however, it solely disables the 0xPhase model and does not disable the initializer of OpenZeppelin.

Impact:

As the Initializable::initializer and ProxyInitializable::initialize modifiers are put in use in the same function, the risk factor of this exhibit is minimal. However, future upgrades of the token may not properly disable the initializers and thus be susceptible to an exploitation of this flaw.

Example:

Copy
35abstract contract PegTokenV1Storage is
36  IPegToken,
37  Initializable,
38  ProxyInitializable,
39  ERC20Upgradeable,
40  ERC20BurnableUpgradeable,
41  ERC20SnapshotUpgradeable,
42  ERC20PermitUpgradeable,
43  AccessControl
44{
45  bytes32 public constant SNAPSHOT_ROLE = keccak256("SNAPSHOT_ROLE");
46  bytes32 public constant MANAGER_ROLE = keccak256("MANAGER_ROLE");
47
48  /// @notice Disables initialization on the target contract
49  constructor() {
50    _disableInitialization();
51  }
52
53  /// @notice Initializes the peg token contract on version 1
54  /// @param db_ The protocol DB
55  function initializePegTokenV1(
56    IDB db_,
57    string memory name_,
58    string memory symbol_
59  ) external initialize("v1") initializer {

Recommendation:

We advise the Initializable::initializer modifier to be introduced to the PegTokenV1Storage::constructor, ensuring that the contract properly disables initializations of the base implementation of both systems.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The Initializable::initializer modifier has been properly introduced to the contract's PegTokenV1Storage::constructor, alleviating this exhibit.