tool: add detector for multiple new reinitializers

scripts/ci_test_upgradability.sh

@@ -195,6 +195,19 @@ then
     exit 255
 fi
 
+slither-check-upgradeability "$DIR_TESTS/contract_initialization.sol" Contract_reinitializer_V2 --new-contract-name Counter_reinitializer_V3_V4 > test_15.txt 2>&1
+DIFF=$(diff test_15.txt "$DIR_TESTS/test_15.txt")
+if [  "$DIFF" != "" ]
+then
+    echo "slither-check-upgradeability 14 failed"
+    cat test_15.txt
+    echo ""
+    cat "$DIR_TESTS/test_15.txt"
+    echo ""
+    echo "$DIFF"
+    exit 255
+fi
+
 rm test_1.txt
 rm test_2.txt
 rm test_3.txt
@@ -209,3 +222,4 @@ rm test_11.txt
 rm test_12.txt
 rm test_13.txt
 rm test_14.txt
+rm test_15.txt

slither/tools/upgradeability/checks/all_checks.py

@@ -7,6 +7,7 @@
     MissingCalls,
     MultipleCalls,
     InitializeTarget,
+    MultipleReinitializers,
 )
 
 from slither.tools.upgradeability.checks.functions_ids import IDCollision, FunctionShadowing

slither/tools/upgradeability/checks/initialization.py

@@ -400,3 +400,96 @@ def _check(self):
         ]
         json = self.generate_result(info)
         return [json]
+
+class MultipleReinitializers(AbstractCheck):
+    ARGUMENT = "multiple-new-reinitializers"
+    IMPACT = CheckClassification.LOW
+
+    HELP = "Multiple new reinitializers in the updated contract"
+    WIKI = "https://github.com/crytic/slither/wiki/Upgradeability-Checks#multiple-new-reinitializers"
+    WIKI_TITLE = "Multiple new reinitializers in the updated contract"
+
+    # region wiki_description
+    WIKI_DESCRIPTION = """
+Detect multiple new reinitializers in the updated contract`.
+"""
+    # endregion wiki_description
+
+    # region wiki_exploit_scenario
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract Counter is Initializable {
+    uint256 public x;
+
+    function initialize(uint256 _x) public initializer {
+        x = _x;
+    }
+
+    function changeX() public {
+        x++;
+    }
+}
+
+contract CounterV2 is Initializable {
+    uint256 public x;
+    uint256 public y;
+    uint256 public z;
+
+    function initializeV2(uint256 _y) public reinitializer(2) {
+        y = _y;
+    }
+
+    function initializeV3(uint256 _z) public reinitializer(3) {
+        z = _z;
+    }
+
+    function changeX() public {
+        x = x + y + z;
+    }
+}
+```
+`CounterV2` has two reinitializers with new versions `2` and `3`. If `initializeV3()` is called first, the `initializeV2()` can't be called to initialize `y`, which may brings security risks.
+"""
+    # endregion wiki_exploit_scenario
+
+    # region wiki_recommendation
+    WIKI_RECOMMENDATION = """
+Do not use multiple reinitializers with higher versions in the updated contract.
+"""
+    # endregion wiki_recommendation
+
+    REQUIRE_CONTRACT = True
+    REQUIRE_CONTRACT_V2 = True
+
+    def _check(self):
+        contract_v1 = self.contract
+        contract_v2 = self.contract_v2
+
+        if contract_v2 is None:
+            raise Exception("multiple-new-reinitializers requires a V2 contract")
+
+        initializerV1 = contract_v1.get_modifier_from_canonical_name("Initializable.initializer()")
+        reinitializerV1 = contract_v1.get_modifier_from_canonical_name("Initializable.reinitializer(uint64)")
+        reinitializerV2 = contract_v2.get_modifier_from_canonical_name("Initializable.reinitializer(uint64)")
+
+        # contractV1 has initializer or reinitializer
+        if initializerV1 is None and reinitializerV1 is None:
+            return []
+        # contractV2 has reinitializer
+        if reinitializerV2 is None:
+            return []
+
+        initializer_funcs_v1 = _get_initialize_functions(contract_v1)
+        initializer_funcs_v2 = _get_initialize_functions(contract_v2)
+        new_reinitializer_funcs = []
+        for fv2 in initializer_funcs_v2:
+            if not any((fv2.full_name == fv1.full_name) for fv1 in initializer_funcs_v1):
+                new_reinitializer_funcs.append(fv2)
+
+        results = []
+        if len(new_reinitializer_funcs) > 1:
+            for f in new_reinitializer_funcs:
+                info = [f, " multiple new reinitializers.\n"]
+                json = self.generate_result(info)
+                results.append(json)
+        return results

tests/tools/check_upgradeability/contract_initialization.sol

@@ -59,3 +59,45 @@ contract Contract_double_call is Contract_no_bug, Contract_no_bug_inherits{
     }
 
 }
+
+contract Contract_reinitializer_V2 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 Counter_reinitializer_V3_V4 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;
+    }
+}

tests/tools/check_upgradeability/test_10.txt

@@ -10,4 +10,4 @@ Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#missing-
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:4 findings, 21 detectors run
+INFO:Slither:4 findings, 22 detectors run

tests/tools/check_upgradeability/test_12.txt

@@ -4,4 +4,4 @@ Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initiali
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:2 findings, 21 detectors run
+INFO:Slither:2 findings, 22 detectors run

tests/tools/check_upgradeability/test_13.txt

@@ -9,4 +9,4 @@ Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrec
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:3 findings, 21 detectors run
+INFO:Slither:3 findings, 22 detectors run

tests/tools/check_upgradeability/test_15.txt

@@ -0,0 +1,15 @@
+INFO:Slither:
+Contract_reinitializer_V2 (tests/tools/check_upgradeability/contract_initialization.sol#63-77) needs to be initialized by Contract_reinitializer_V2.initialize(uint256) (tests/tools/check_upgradeability/contract_initialization.sol#66-68).
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+INFO:Slither:
+Extra variables in Counter_reinitializer_V3_V4 (tests/tools/check_upgradeability/contract_initialization.sol#79-104): Counter_reinitializer_V3_V4.y (tests/tools/check_upgradeability/contract_initialization.sol#81)
+Extra variables in Counter_reinitializer_V3_V4 (tests/tools/check_upgradeability/contract_initialization.sol#79-104): Counter_reinitializer_V3_V4.z (tests/tools/check_upgradeability/contract_initialization.sol#82)
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2
+INFO:Slither:
+Counter_reinitializer_V3_V4.initializeV3(uint256) (tests/tools/check_upgradeability/contract_initialization.sol#92-94) multiple new reinitializers.
+Counter_reinitializer_V3_V4.initializeV4(uint256) (tests/tools/check_upgradeability/contract_initialization.sol#96-98) multiple new reinitializers.
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#multiple-new-reinitializers
+INFO:Slither:
+Counter_reinitializer_V3_V4 (tests/tools/check_upgradeability/contract_initialization.sol#79-104) needs to be initialized by Counter_reinitializer_V3_V4.initialize(uint256) (tests/tools/check_upgradeability/contract_initialization.sol#84-86).
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+INFO:Slither:6 findings, 22 detectors run

tests/tools/check_upgradeability/test_2.txt

@@ -4,4 +4,4 @@ Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initiali
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:2 findings, 25 detectors run
+INFO:Slither:2 findings, 26 detectors run

tests/tools/check_upgradeability/test_3.txt

@@ -20,4 +20,4 @@ Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-va
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:6 findings, 25 detectors run
+INFO:Slither:6 findings, 26 detectors run

tests/tools/check_upgradeability/test_4.txt

@@ -17,4 +17,4 @@ Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-va
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing
-INFO:Slither:5 findings, 25 detectors run
+INFO:Slither:5 findings, 26 detectors run