Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tool: support reinitializer in slither-check-upgradeability #2534

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion scripts/ci_test_upgradability.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -194,3 +208,4 @@ rm test_10.txt
rm test_11.txt
rm test_12.txt
rm test_13.txt
rm test_14.txt
16 changes: 10 additions & 6 deletions slither/tools/upgradeability/checks/initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
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)

Check warning on line 21 in slither/tools/upgradeability/checks/initialization.py

View workflow job for this annotation

GitHub Actions / Lint Code Base

R1714: Consider merging these comparisons with 'in' by using 'm.name in ('initializer', 'reinitializer')'. Use a set instead if elements are hashable. (consider-using-in)


def _get_initialize_functions(contract):
Expand Down Expand Up @@ -164,7 +164,7 @@

# region wiki_description
WIKI_DESCRIPTION = """
Detect if `Initializable.initializer()` is called.
Detect if `Initializable.initializer()` or `Initializable.reinitializer(uint64)` is called.
"""
# endregion wiki_description

Expand All @@ -184,7 +184,7 @@

# region wiki_recommendation
WIKI_RECOMMENDATION = """
Use `Initializable.initializer()`.
Use `Initializable.initializer()` or `Initializable.reinitializer(uint64)`.
"""
# endregion wiki_recommendation

Expand All @@ -199,15 +199,16 @@
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
Expand Down Expand Up @@ -271,6 +272,9 @@
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)
Expand Down
12 changes: 12 additions & 0 deletions tests/tools/check_upgradeability/contract_initialization.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ contract Initializable{
_;
}

modifier reinitializer(uint64 version){
_;
}

}

contract Contract_no_bug is Initializable{
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions tests/tools/check_upgradeability/test_10.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion tests/tools/check_upgradeability/test_11.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions tests/tools/check_upgradeability/test_13.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions tests/tools/check_upgradeability/test_14.txt
Original file line number Diff line number Diff line change
@@ -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
16 changes: 8 additions & 8 deletions tests/tools/check_upgradeability/test_3.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 7 additions & 7 deletions tests/tools/check_upgradeability/test_4.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion tests/tools/check_upgradeability/test_5.txt
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions tests/tools/check_upgradeability/test_6.txt
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions tests/tools/check_upgradeability/test_7.txt
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion tests/tools/check_upgradeability/test_8.txt
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions tests/tools/check_upgradeability/test_9.txt
Original file line number Diff line number Diff line change
@@ -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
Loading