code-423n4 / 2022-02-jpyc-findings

1 stars 0 forks source link

All Contract Are Not Upgradeable contracts #50

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L40 https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L41 https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L43 https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L44

Vulnerability details

Impact

Ownable, Pausable, Rescuable and Blocklistable contracts are not inherited from Upgradeable contracts. The contracts are not upgradeable. For the using the upgradeable contract, The following steps should be followed.

- Instead of Ownable, we use the upgradable variant OwnableUpgradeable
- Replace the constructor with the initializer method, with the initializer modifier. Note that you need to - manually call the __{contract}_init(); method of every parent contract with appropriate parameters.

Proof of Concept

  1. Navigate to the following contract initializer.
contract FiatTokenV2 is
    Ownable,
    Pausable,
    Blocklistable,
    Rescuable,
    EIP3009,
    EIP2612,
    UUPSUpgradeable

Tools Used

Code Review

Recommended Mitigation Steps

Ensure that all necessary functions are inherited from the upgradeable contracts. The sample can be seen from below.

pragma solidity 0.8.12;

import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract MyToken is Initializable, ERC20Upgradeable, OwnableUpgradeable, UUPSUpgradeable {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() initializer {}

    function initialize() initializer public {
        __ERC20_init("MyToken", "MTK");
        __Ownable_init();
        __UUPSUpgradeable_init();
    }

    function _authorizeUpgrade(address newImplementation)
        internal
        onlyOwner
        override
    {}
}