From fe54eb7c9cf8cbd05897c96deecde49af7c4ef9e Mon Sep 17 00:00:00 2001
From: QiuhaoLi <Qiuhao.Li@outlook.com>
Date: Thu, 15 Aug 2024 12:49:07 +0800
Subject: [PATCH 1/4] tool: support reinitializer in
 slither-check-upgradeability

---
 scripts/ci_test_upgradability.sh                | 17 ++++++++++++++++-
 .../upgradeability/checks/initialization.py     | 13 +++++++------
 .../contract_initialization.sol                 | 12 ++++++++++++
 tests/tools/check_upgradeability/test_10.txt    |  4 ++--
 tests/tools/check_upgradeability/test_11.txt    |  2 +-
 tests/tools/check_upgradeability/test_13.txt    |  6 +++---
 tests/tools/check_upgradeability/test_14.txt    |  4 ++++
 tests/tools/check_upgradeability/test_3.txt     | 16 ++++++++--------
 tests/tools/check_upgradeability/test_4.txt     | 14 +++++++-------
 tests/tools/check_upgradeability/test_5.txt     |  2 +-
 tests/tools/check_upgradeability/test_6.txt     |  4 ++--
 tests/tools/check_upgradeability/test_7.txt     |  4 ++--
 tests/tools/check_upgradeability/test_8.txt     |  2 +-
 tests/tools/check_upgradeability/test_9.txt     |  4 ++--
 14 files changed, 68 insertions(+), 36 deletions(-)
 create mode 100644 tests/tools/check_upgradeability/test_14.txt

diff --git a/scripts/ci_test_upgradability.sh b/scripts/ci_test_upgradability.sh
index 0a0d77f519..9d320e2957 100755
--- a/scripts/ci_test_upgradability.sh
+++ b/scripts/ci_test_upgradability.sh
@@ -2,7 +2,8 @@
 
 ### Test slither-check-upgradeability
 
-DIR_TESTS="tests/check-upgradeability"
+DIR_TESTS="tests/tools/check_upgradeability"
+solc-select install "0.5.0"
 solc-select use "0.5.0"
 
 slither-check-upgradeability  "$DIR_TESTS/contractV1.sol" ContractV1 --proxy-filename "$DIR_TESTS/proxy.sol" --proxy-name Proxy  > test_1.txt 2>&1
@@ -181,6 +182,19 @@ then
     exit 255
 fi
 
+slither-check-upgradeability "$DIR_TESTS/contract_initialization.sol" Contract_no_bug_reinitializer --proxy-filename "$DIR_TESTS/proxy.sol" --proxy-name Proxy   > test_14.txt 2>&1
+DIFF=$(diff test_14.txt "$DIR_TESTS/test_14.txt")
+if [  "$DIFF" != "" ]
+then
+    echo "slither-check-upgradeability 14 failed"
+    cat test_14.txt
+    echo ""
+    cat "$DIR_TESTS/test_14.txt"
+    echo ""
+    echo "$DIFF"
+    exit 255
+fi
+
 rm test_1.txt
 rm test_2.txt
 rm test_3.txt
@@ -194,3 +208,4 @@ rm test_10.txt
 rm test_11.txt
 rm test_12.txt
 rm test_13.txt
+rm test_14.txt
diff --git a/slither/tools/upgradeability/checks/initialization.py b/slither/tools/upgradeability/checks/initialization.py
index 2055a322a7..0eb89aa742 100644
--- a/slither/tools/upgradeability/checks/initialization.py
+++ b/slither/tools/upgradeability/checks/initialization.py
@@ -18,7 +18,7 @@ class MultipleInitTarget(Exception):
 def _has_initialize_modifier(function: Function):
     if not function.modifiers:
         return False
-    return any((m.name == "initializer") for m in function.modifiers)
+    return any((m.name == "initializer" or m.name == "reinitializer") for m in function.modifiers)
 
 
 def _get_initialize_functions(contract):
@@ -164,7 +164,7 @@ class MissingInitializerModifier(AbstractCheck):
 
     # region wiki_description
     WIKI_DESCRIPTION = """
-Detect if `Initializable.initializer()` is called.
+Detect if `Initializable.initializer()` or `Initializable.reinitializer(uint64)` is called.
 """
     # endregion wiki_description
 
@@ -184,7 +184,7 @@ class MissingInitializerModifier(AbstractCheck):
 
     # region wiki_recommendation
     WIKI_RECOMMENDATION = """
-Use `Initializable.initializer()`.
+Use `Initializable.initializer()` or `Initializable.reinitializer(uint64)`.
 """
     # endregion wiki_recommendation
 
@@ -199,15 +199,16 @@ def _check(self):
         if initializable not in self.contract.inheritance:
             return []
         initializer = self.contract.get_modifier_from_canonical_name("Initializable.initializer()")
+        reinitializer = self.contract.get_modifier_from_canonical_name("Initializable.reinitializer(uint64)")
         # InitializableInitializer
-        if initializer is None:
+        if initializer is None and reinitializer is None:
             return []
 
         results = []
         all_init_functions = _get_initialize_functions(self.contract)
         for f in all_init_functions:
-            if initializer not in f.modifiers:
-                info = [f, " does not call the initializer modifier.\n"]
+            if initializer not in f.modifiers and reinitializer not in f.modifiers:
+                info = [f, " does not call the initializer or reinitializer modifier.\n"]
                 json = self.generate_result(info)
                 results.append(json)
         return results
diff --git a/tests/tools/check_upgradeability/contract_initialization.sol b/tests/tools/check_upgradeability/contract_initialization.sol
index d17125ee96..5494dff42c 100644
--- a/tests/tools/check_upgradeability/contract_initialization.sol
+++ b/tests/tools/check_upgradeability/contract_initialization.sol
@@ -6,6 +6,10 @@ contract Initializable{
         _;
     }
 
+    modifier reinitializer(uint64 version){
+        _;
+    }
+
 }
 
 contract Contract_no_bug is Initializable{
@@ -16,6 +20,14 @@ contract Contract_no_bug is Initializable{
 
 }
 
+contract Contract_no_bug_reinitializer is Initializable{
+
+    function initialize() public reinitializer(2){
+
+    }
+
+}
+
 contract Contract_lack_to_call_modifier is Initializable{
 
     function initialize() public {
diff --git a/tests/tools/check_upgradeability/test_10.txt b/tests/tools/check_upgradeability/test_10.txt
index 3d317aca5b..bbd8f17979 100644
--- a/tests/tools/check_upgradeability/test_10.txt
+++ b/tests/tools/check_upgradeability/test_10.txt
@@ -2,10 +2,10 @@ 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:
-ContractV1.destination (tests/check-upgradeability/contractV1.sol#2) was not constant but ContractV2.destination (tests/check-upgradeability/contract_v2_constant.sol#2) is.
+ContractV1.destination (tests/tools/check_upgradeability/contractV1.sol#2) was not constant but ContractV2.destination (tests/tools/check_upgradeability/contract_v2_constant.sol#2) is.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-that-should-not-be-constant
 INFO:Slither:
-Variable missing in ContractV2 (tests/check-upgradeability/contract_v2_constant.sol#1-3): ContractV1.destination (tests/check-upgradeability/contractV1.sol#2)
+Variable missing in ContractV2 (tests/tools/check_upgradeability/contract_v2_constant.sol#1-3): ContractV1.destination (tests/tools/check_upgradeability/contractV1.sol#2)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#missing-variables
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
diff --git a/tests/tools/check_upgradeability/test_11.txt b/tests/tools/check_upgradeability/test_11.txt
index e2ad677b12..d942ce6b7c 100644
--- a/tests/tools/check_upgradeability/test_11.txt
+++ b/tests/tools/check_upgradeability/test_11.txt
@@ -2,6 +2,6 @@ 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:
-ContractV1.destination (tests/check-upgradeability/contract_v1_var_init.sol#2) is a state variable with an initial value.
+ContractV1.destination (tests/tools/check_upgradeability/contract_v1_var_init.sol#2) is a state variable with an initial value.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#state-variable-initialized
 INFO:Slither:2 findings, 8 detectors run
diff --git a/tests/tools/check_upgradeability/test_13.txt b/tests/tools/check_upgradeability/test_13.txt
index 9635f9a43b..c9d02a522d 100644
--- a/tests/tools/check_upgradeability/test_13.txt
+++ b/tests/tools/check_upgradeability/test_13.txt
@@ -2,9 +2,9 @@ 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:
-Different variables between ContractV1 (tests/check-upgradeability/contractV1_struct.sol#1-8) and ContractV2 (tests/check-upgradeability/contractV2_struct_bug.sol#1-8)
-	 ContractV1.foo (tests/check-upgradeability/contractV1_struct.sol#7)
-	 ContractV2.foo (tests/check-upgradeability/contractV2_struct_bug.sol#7)
+Different variables between ContractV1 (tests/tools/check_upgradeability/contractV1_struct.sol#1-8) and ContractV2 (tests/tools/check_upgradeability/contractV2_struct_bug.sol#1-8)
+	 ContractV1.foo (tests/tools/check_upgradeability/contractV1_struct.sol#7)
+	 ContractV2.foo (tests/tools/check_upgradeability/contractV2_struct_bug.sol#7)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
diff --git a/tests/tools/check_upgradeability/test_14.txt b/tests/tools/check_upgradeability/test_14.txt
new file mode 100644
index 0000000000..da412418ec
--- /dev/null
+++ b/tests/tools/check_upgradeability/test_14.txt
@@ -0,0 +1,4 @@
+INFO:Slither:
+Contract_no_bug_reinitializer (tests/tools/check_upgradeability/contract_initialization.sol#23-29) needs to be initialized by Contract_no_bug_reinitializer.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#25-27).
+Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
+INFO:Slither:1 findings, 12 detectors run
diff --git a/tests/tools/check_upgradeability/test_3.txt b/tests/tools/check_upgradeability/test_3.txt
index fb694d5fb0..265f0d15e7 100644
--- a/tests/tools/check_upgradeability/test_3.txt
+++ b/tests/tools/check_upgradeability/test_3.txt
@@ -2,20 +2,20 @@ 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:
-Different variables between ContractV2 (tests/check-upgradeability/contractV2_bug.sol#1-5) and Proxy (tests/check-upgradeability/proxy.sol#7-27)
-	 ContractV2.destination (tests/check-upgradeability/contractV2_bug.sol#2)
-	 Proxy.destination (tests/check-upgradeability/proxy.sol#9)
+Different variables between ContractV2 (tests/tools/check_upgradeability/contractV2_bug.sol#1-5) and Proxy (tests/tools/check_upgradeability/proxy.sol#7-27)
+	 ContractV2.destination (tests/tools/check_upgradeability/contractV2_bug.sol#2)
+	 Proxy.destination (tests/tools/check_upgradeability/proxy.sol#9)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-proxy
 INFO:Slither:
-Function shadowing found: ContractV2.myFunc (tests/check-upgradeability/contractV2_bug.sol#4) Proxy.myFunc() (tests/check-upgradeability/proxy.sol#11)
+Function shadowing found: ContractV2.myFunc (tests/tools/check_upgradeability/contractV2_bug.sol#4) Proxy.myFunc() (tests/tools/check_upgradeability/proxy.sol#11)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-shadowing
 INFO:Slither:
-Different variables between ContractV1 (tests/check-upgradeability/contractV1.sol#1-3) and ContractV2 (tests/check-upgradeability/contractV2_bug.sol#1-5)
-	 ContractV1.destination (tests/check-upgradeability/contractV1.sol#2)
-	 ContractV2.destination (tests/check-upgradeability/contractV2_bug.sol#2)
+Different variables between ContractV1 (tests/tools/check_upgradeability/contractV1.sol#1-3) and ContractV2 (tests/tools/check_upgradeability/contractV2_bug.sol#1-5)
+	 ContractV1.destination (tests/tools/check_upgradeability/contractV1.sol#2)
+	 ContractV2.destination (tests/tools/check_upgradeability/contractV2_bug.sol#2)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2
 INFO:Slither:
-Extra variables in ContractV2 (tests/check-upgradeability/contractV2_bug.sol#1-5): ContractV2.myFunc (tests/check-upgradeability/contractV2_bug.sol#4)
+Extra variables in ContractV2 (tests/tools/check_upgradeability/contractV2_bug.sol#1-5): ContractV2.myFunc (tests/tools/check_upgradeability/contractV2_bug.sol#4)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
diff --git a/tests/tools/check_upgradeability/test_4.txt b/tests/tools/check_upgradeability/test_4.txt
index 4752eb706c..1a4ba08fbb 100644
--- a/tests/tools/check_upgradeability/test_4.txt
+++ b/tests/tools/check_upgradeability/test_4.txt
@@ -2,17 +2,17 @@ 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:
-Different variables between ContractV2 (tests/check-upgradeability/contractV2_bug2.sol#4-6) and Proxy (tests/check-upgradeability/proxy.sol#7-27)
-	 Base.val (tests/check-upgradeability/contractV2_bug2.sol#2)
-	 Proxy.destination (tests/check-upgradeability/proxy.sol#9)
+Different variables between ContractV2 (tests/tools/check_upgradeability/contractV2_bug2.sol#4-6) and Proxy (tests/tools/check_upgradeability/proxy.sol#7-27)
+	 Base.val (tests/tools/check_upgradeability/contractV2_bug2.sol#2)
+	 Proxy.destination (tests/tools/check_upgradeability/proxy.sol#9)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-proxy
 INFO:Slither:
-Different variables between ContractV1 (tests/check-upgradeability/contractV1.sol#1-3) and ContractV2 (tests/check-upgradeability/contractV2_bug2.sol#4-6)
-	 ContractV1.destination (tests/check-upgradeability/contractV1.sol#2)
-	 Base.val (tests/check-upgradeability/contractV2_bug2.sol#2)
+Different variables between ContractV1 (tests/tools/check_upgradeability/contractV1.sol#1-3) and ContractV2 (tests/tools/check_upgradeability/contractV2_bug2.sol#4-6)
+	 ContractV1.destination (tests/tools/check_upgradeability/contractV1.sol#2)
+	 Base.val (tests/tools/check_upgradeability/contractV2_bug2.sol#2)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2
 INFO:Slither:
-Extra variables in ContractV2 (tests/check-upgradeability/contractV2_bug2.sol#4-6): ContractV2.destination (tests/check-upgradeability/contractV2_bug2.sol#5)
+Extra variables in ContractV2 (tests/tools/check_upgradeability/contractV2_bug2.sol#4-6): ContractV2.destination (tests/tools/check_upgradeability/contractV2_bug2.sol#5)
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2
 INFO:Slither:
 Initializable contract not found, the contract does not follow a standard initalization schema.
diff --git a/tests/tools/check_upgradeability/test_5.txt b/tests/tools/check_upgradeability/test_5.txt
index 805602ee44..da889fb59c 100644
--- a/tests/tools/check_upgradeability/test_5.txt
+++ b/tests/tools/check_upgradeability/test_5.txt
@@ -1,4 +1,4 @@
 INFO:Slither:
-Contract_no_bug (tests/check-upgradeability/contract_initialization.sol#11-17) needs to be initialized by Contract_no_bug.initialize() (tests/check-upgradeability/contract_initialization.sol#13-15).
+Contract_no_bug (tests/tools/check_upgradeability/contract_initialization.sol#15-21) needs to be initialized by Contract_no_bug.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#17-19).
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
 INFO:Slither:1 findings, 12 detectors run
diff --git a/tests/tools/check_upgradeability/test_6.txt b/tests/tools/check_upgradeability/test_6.txt
index 663cb62d07..80b8d8b516 100644
--- a/tests/tools/check_upgradeability/test_6.txt
+++ b/tests/tools/check_upgradeability/test_6.txt
@@ -1,7 +1,7 @@
 INFO:Slither:
-Contract_lack_to_call_modifier (tests/check-upgradeability/contract_initialization.sol#19-24) needs to be initialized by Contract_lack_to_call_modifier.initialize() (tests/check-upgradeability/contract_initialization.sol#21-23).
+Contract_lack_to_call_modifier (tests/tools/check_upgradeability/contract_initialization.sol#31-36) needs to be initialized by Contract_lack_to_call_modifier.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#33-35).
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
 INFO:Slither:
-Contract_lack_to_call_modifier.initialize() (tests/check-upgradeability/contract_initialization.sol#21-23) does not call the initializer modifier.
+Contract_lack_to_call_modifier.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#33-35) does not call the initializer or reinitializer modifier.
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializer-is-not-called
 INFO:Slither:2 findings, 12 detectors run
diff --git a/tests/tools/check_upgradeability/test_7.txt b/tests/tools/check_upgradeability/test_7.txt
index 9f232338e7..02607207be 100644
--- a/tests/tools/check_upgradeability/test_7.txt
+++ b/tests/tools/check_upgradeability/test_7.txt
@@ -1,7 +1,7 @@
 INFO:Slither:
-Contract_not_called_super_init (tests/check-upgradeability/contract_initialization.sol#26-32) needs to be initialized by Contract_not_called_super_init.initialize() (tests/check-upgradeability/contract_initialization.sol#28-30).
+Contract_not_called_super_init (tests/tools/check_upgradeability/contract_initialization.sol#38-44) needs to be initialized by Contract_not_called_super_init.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#40-42).
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
 INFO:Slither:
-Missing call to Contract_no_bug.initialize() (tests/check-upgradeability/contract_initialization.sol#13-15) in Contract_not_called_super_init.initialize() (tests/check-upgradeability/contract_initialization.sol#28-30).
+Missing call to Contract_no_bug.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#17-19) in Contract_not_called_super_init.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#40-42).
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-not-called
 INFO:Slither:2 findings, 12 detectors run
diff --git a/tests/tools/check_upgradeability/test_8.txt b/tests/tools/check_upgradeability/test_8.txt
index 38c71e28c1..8cd703bea8 100644
--- a/tests/tools/check_upgradeability/test_8.txt
+++ b/tests/tools/check_upgradeability/test_8.txt
@@ -1,4 +1,4 @@
 INFO:Slither:
-Contract_no_bug_inherits (tests/check-upgradeability/contract_initialization.sol#34-40) needs to be initialized by Contract_no_bug_inherits.initialize() (tests/check-upgradeability/contract_initialization.sol#36-38).
+Contract_no_bug_inherits (tests/tools/check_upgradeability/contract_initialization.sol#46-52) needs to be initialized by Contract_no_bug_inherits.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#48-50).
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
 INFO:Slither:1 findings, 12 detectors run
diff --git a/tests/tools/check_upgradeability/test_9.txt b/tests/tools/check_upgradeability/test_9.txt
index a67578a08f..cece4f6ea8 100644
--- a/tests/tools/check_upgradeability/test_9.txt
+++ b/tests/tools/check_upgradeability/test_9.txt
@@ -1,7 +1,7 @@
 INFO:Slither:
-Contract_double_call (tests/check-upgradeability/contract_initialization.sol#42-49) needs to be initialized by Contract_double_call.initialize() (tests/check-upgradeability/contract_initialization.sol#44-47).
+Contract_double_call (tests/tools/check_upgradeability/contract_initialization.sol#54-61) needs to be initialized by Contract_double_call.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#56-59).
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function
 INFO:Slither:
-Contract_no_bug.initialize() (tests/check-upgradeability/contract_initialization.sol#13-15) is called multiple times in Contract_double_call.initialize() (tests/check-upgradeability/contract_initialization.sol#44-47).
+Contract_no_bug.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#17-19) is called multiple times in Contract_double_call.initialize() (tests/tools/check_upgradeability/contract_initialization.sol#56-59).
 Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-called-multiple-times
 INFO:Slither:2 findings, 12 detectors run

From 5f3138107374991e833126bb6bb104aae5a6ff88 Mon Sep 17 00:00:00 2001
From: QiuhaoLi <Qiuhao.Li@outlook.com>
Date: Thu, 15 Aug 2024 15:55:37 +0800
Subject: [PATCH 2/4] MissingCalls: don't account reinitializers

---
 slither/tools/upgradeability/checks/initialization.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/slither/tools/upgradeability/checks/initialization.py b/slither/tools/upgradeability/checks/initialization.py
index 0eb89aa742..cd420ee45e 100644
--- a/slither/tools/upgradeability/checks/initialization.py
+++ b/slither/tools/upgradeability/checks/initialization.py
@@ -272,6 +272,9 @@ def _check(self):
         all_init_functions_called = _get_all_internal_calls(most_derived_init) + [most_derived_init]
         missing_calls = [f for f in all_init_functions if not f in all_init_functions_called]
         for f in missing_calls:
+            # we don't account reinitializers
+            if any((m.name == "reinitializer") for m in f.modifiers):
+                continue
             info = ["Missing call to ", f, " in ", most_derived_init, ".\n"]
             json = self.generate_result(info)
             results.append(json)

From 440b9f49c52b42e4521a661a38d993268e74b7ce Mon Sep 17 00:00:00 2001
From: QiuhaoLi <Qiuhao.Li@outlook.com>
Date: Thu, 15 Aug 2024 17:33:42 +0800
Subject: [PATCH 3/4] tool: add detector for multiple new reinitializers

---
 scripts/ci_test_upgradability.sh              | 14 +++
 .../tools/upgradeability/checks/all_checks.py |  1 +
 .../upgradeability/checks/initialization.py   | 93 +++++++++++++++++++
 .../contract_initialization.sol               | 42 +++++++++
 tests/tools/check_upgradeability/test_10.txt  |  2 +-
 tests/tools/check_upgradeability/test_12.txt  |  2 +-
 tests/tools/check_upgradeability/test_13.txt  |  2 +-
 tests/tools/check_upgradeability/test_15.txt  | 15 +++
 tests/tools/check_upgradeability/test_2.txt   |  2 +-
 tests/tools/check_upgradeability/test_3.txt   |  2 +-
 tests/tools/check_upgradeability/test_4.txt   |  2 +-
 11 files changed, 171 insertions(+), 6 deletions(-)
 create mode 100644 tests/tools/check_upgradeability/test_15.txt

diff --git a/scripts/ci_test_upgradability.sh b/scripts/ci_test_upgradability.sh
index 9d320e2957..a4da93873c 100755
--- a/scripts/ci_test_upgradability.sh
+++ b/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
diff --git a/slither/tools/upgradeability/checks/all_checks.py b/slither/tools/upgradeability/checks/all_checks.py
index 2289c38085..1581ab5308 100644
--- a/slither/tools/upgradeability/checks/all_checks.py
+++ b/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
diff --git a/slither/tools/upgradeability/checks/initialization.py b/slither/tools/upgradeability/checks/initialization.py
index cd420ee45e..53db874cbb 100644
--- a/slither/tools/upgradeability/checks/initialization.py
+++ b/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
diff --git a/tests/tools/check_upgradeability/contract_initialization.sol b/tests/tools/check_upgradeability/contract_initialization.sol
index 5494dff42c..ab6ab8e515 100644
--- a/tests/tools/check_upgradeability/contract_initialization.sol
+++ b/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;
+    }
+}
\ No newline at end of file
diff --git a/tests/tools/check_upgradeability/test_10.txt b/tests/tools/check_upgradeability/test_10.txt
index bbd8f17979..9a7e022c1e 100644
--- a/tests/tools/check_upgradeability/test_10.txt
+++ b/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
diff --git a/tests/tools/check_upgradeability/test_12.txt b/tests/tools/check_upgradeability/test_12.txt
index 353d8ebdb7..7641e8335b 100644
--- a/tests/tools/check_upgradeability/test_12.txt
+++ b/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
diff --git a/tests/tools/check_upgradeability/test_13.txt b/tests/tools/check_upgradeability/test_13.txt
index c9d02a522d..e39b9b1ac3 100644
--- a/tests/tools/check_upgradeability/test_13.txt
+++ b/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
diff --git a/tests/tools/check_upgradeability/test_15.txt b/tests/tools/check_upgradeability/test_15.txt
new file mode 100644
index 0000000000..960f7b3565
--- /dev/null
+++ b/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
diff --git a/tests/tools/check_upgradeability/test_2.txt b/tests/tools/check_upgradeability/test_2.txt
index dcf910c00d..a3970ffc12 100644
--- a/tests/tools/check_upgradeability/test_2.txt
+++ b/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
diff --git a/tests/tools/check_upgradeability/test_3.txt b/tests/tools/check_upgradeability/test_3.txt
index 265f0d15e7..5cdc9fc7e2 100644
--- a/tests/tools/check_upgradeability/test_3.txt
+++ b/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
diff --git a/tests/tools/check_upgradeability/test_4.txt b/tests/tools/check_upgradeability/test_4.txt
index 1a4ba08fbb..eb088324d2 100644
--- a/tests/tools/check_upgradeability/test_4.txt
+++ b/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

From 3b4673af9975fa221a34757ca1a21a5d90d18969 Mon Sep 17 00:00:00 2001
From: QiuhaoLi <Qiuhao.Li@outlook.com>
Date: Tue, 20 Aug 2024 11:38:31 +0800
Subject: [PATCH 4/4] fix: comments and pylint

---
 .../upgradeability/checks/initialization.py   | 29 ++++++++++++++-----
 tests/tools/check_upgradeability/test_15.txt  |  4 +--
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/slither/tools/upgradeability/checks/initialization.py b/slither/tools/upgradeability/checks/initialization.py
index 53db874cbb..e31a71947b 100644
--- a/slither/tools/upgradeability/checks/initialization.py
+++ b/slither/tools/upgradeability/checks/initialization.py
@@ -7,6 +7,7 @@
     CheckClassification,
 )
 from slither.utils.colors import red
+from slither.exceptions import SlitherError
 
 logger = logging.getLogger("Slither-check-upgradeability")
 
@@ -18,7 +19,7 @@ class MultipleInitTarget(Exception):
 def _has_initialize_modifier(function: Function):
     if not function.modifiers:
         return False
-    return any((m.name == "initializer" or m.name == "reinitializer") for m in function.modifiers)
+    return any((m.name in ("initializer", "reinitializer")) for m in function.modifiers)
 
 
 def _get_initialize_functions(contract):
@@ -199,7 +200,9 @@ def _check(self):
         if initializable not in self.contract.inheritance:
             return []
         initializer = self.contract.get_modifier_from_canonical_name("Initializable.initializer()")
-        reinitializer = self.contract.get_modifier_from_canonical_name("Initializable.reinitializer(uint64)")
+        reinitializer = self.contract.get_modifier_from_canonical_name(
+            "Initializable.reinitializer(uint64)"
+        )
         # InitializableInitializer
         if initializer is None and reinitializer is None:
             return []
@@ -401,12 +404,15 @@ 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 = (
+        "https://github.com/crytic/slither/wiki/Upgradeability-Checks#multiple-new-reinitializers"
+    )
     WIKI_TITLE = "Multiple new reinitializers in the updated contract"
 
     # region wiki_description
@@ -454,7 +460,7 @@ class MultipleReinitializers(AbstractCheck):
 
     # region wiki_recommendation
     WIKI_RECOMMENDATION = """
-Do not use multiple reinitializers with higher versions in the updated contract.
+Do not use multiple reinitializers with higher versions in the updated contract. Please consider combining new reinitializers into a single one.
 """
     # endregion wiki_recommendation
 
@@ -466,11 +472,15 @@ def _check(self):
         contract_v2 = self.contract_v2
 
         if contract_v2 is None:
-            raise Exception("multiple-new-reinitializers requires a V2 contract")
+            raise SlitherError("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)")
+        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:
@@ -489,7 +499,10 @@ def _check(self):
         results = []
         if len(new_reinitializer_funcs) > 1:
             for f in new_reinitializer_funcs:
-                info = [f, " multiple new reinitializers.\n"]
+                info = [
+                    f,
+                    " multiple new reinitializers which should be combined into one per upgrade.\n",
+                ]
                 json = self.generate_result(info)
                 results.append(json)
         return results
diff --git a/tests/tools/check_upgradeability/test_15.txt b/tests/tools/check_upgradeability/test_15.txt
index 960f7b3565..d9c588f737 100644
--- a/tests/tools/check_upgradeability/test_15.txt
+++ b/tests/tools/check_upgradeability/test_15.txt
@@ -6,8 +6,8 @@ Extra variables in Counter_reinitializer_V3_V4 (tests/tools/check_upgradeability
 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.
+Counter_reinitializer_V3_V4.initializeV3(uint256) (tests/tools/check_upgradeability/contract_initialization.sol#92-94) multiple new reinitializers which should be combined into one per upgrade.
+Counter_reinitializer_V3_V4.initializeV4(uint256) (tests/tools/check_upgradeability/contract_initialization.sol#96-98) multiple new reinitializers which should be combined into one per upgrade.
 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).