tool: add detector for multiple new reinitializers

[QiuhaoLi]

When upgrading a proxy's implementation with reinitializer, developers could errorly write multiple new reinitializers. If the new reinitializer with a higher version is called first, the reinitializer with a lower version can't be called later (you can't call a reinitializer with a lower version) and may leave uninitalized state variables.

This could lead to severe results in the real world. For example, the Ronin Network prepared two upgrades for their gateway contract and wrote two reinitializers (initializeV3() and initializeV4()). However, the developer calls the initializeV4() first and leaves the state variables _operatorWeight and _totalOperatorWeight uninitialized, which should be set in the initializeV3(). So attackers can bypass the vote weight and withdraw assets directly [1] [2] [3].

The best practice for this issue is never to introduce more than one reinitializer in one updated implementation. This PR will add a detector to the slither-check-upgradeability to check if multiple new reinitializes are introduced and report a low-severity warning. For example:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.24;
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
contract Counter is Initializable {
    uint256 public x;

    function initialize(uint256 _x) public initializer {
        x = _x;
    }

    function initializeV2(uint256 _x) public reinitializer(2) {
        x = _x;
    }

    function changeX() public {
        x++;
    }
}

contract CounterV2 is Initializable {
    uint256 public x;
    uint256 public y;
    uint256 public z;

    function initialize(uint256 _x) public initializer {
        x = _x;
    }

    function initializeV2(uint256 _x) public reinitializer(2) {
        x = _x;
    }

    function initializeV3(uint256 _y) public reinitializer(3) {
        y = _y;
    }

    function initializeV4(uint256 _z) public reinitializer(4) {
        z = _z;
    }

    function changeX() public {
        x = x + y + z;
    }
}

qiuhao@laptop:~/tmp/foundry_test/upgrade$ slither-check-upgradeability src/Counter.sol Counter --new-contract-name CounterV2
...
INFO:Slither:
CounterV2.initializeV3(uint256) (src/Counter.sol#33-35) multiple new reinitializers.
CounterV2.initializeV4(uint256) (src/Counter.sol#37-39) multiple new reinitializers.
Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#multiple-new-reinitializers
qiuhao@laptop:~/tmp/foundry_test/upgrade$ 

[0xalpharush]

Can we add what should be done instead? I think they should be combined into a single re-initializer per upgrade based off the description of the issue, right?

[QiuhaoLi]

Yes, I added a suggestion, thanks.

[0xalpharush]

Suggested change

	info = [f, " multiple new reinitializers.\n"]
	info = [f, " multiple new reinitializers which should be combined into one per upgrade.\n"]

Same general idea as https://github.com/crytic/slither/pull/2536/files#r1722628467

[0xalpharush]

Can you run make reformat && make lint and fix the lint warnings please?

[0xalpharush]

I am trying to fix the CI failure in #2537

[0xalpharush]

Thanks @QiuhaoLi, these are great additions!

[QiuhaoLi]

@0xalpharush Thanks for the review :)