From 1d87059712bf35eedd56022e0f71c1251ef4945c Mon Sep 17 00:00:00 2001 From: Josselin Date: Wed, 18 Mar 2020 14:27:08 +0100 Subject: [PATCH] Large refactor of slither-check-upgradeability: - Use an abstract class, similar pattern than slither's detectors - Every issue is documented (description/exploit scenario/recommendation) --- slither/tools/upgradeability/__main__.py | 191 ++++++---- .../upgradeability/check_initialization.py | 138 -------- .../check_variable_initialization.py | 31 -- .../tools/upgradeability/checks/__init__.py | 0 .../upgradeability/checks/abstract_checks.py | 127 +++++++ .../tools/upgradeability/checks/all_checks.py | 11 + .../tools/upgradeability/checks/constant.py | 162 +++++++++ .../upgradeability/checks/functions_ids.py | 147 ++++++++ .../upgradeability/checks/initialization.py | 333 ++++++++++++++++++ .../checks/variable_initialization.py | 38 ++ .../upgradeability/checks/variables_order.py | 238 +++++++++++++ .../upgradeability/compare_function_ids.py | 86 ----- .../upgradeability/compare_variables_order.py | 73 ---- .../tools/upgradeability/constant_checks.py | 85 ----- .../tools/upgradeability/utils/__init__.py | 0 .../upgradeability/utils/command_line.py | 126 +++++++ tests/check-upgradeability/test_1.txt | 16 +- tests/check-upgradeability/test_10.txt | 25 +- tests/check-upgradeability/test_11.txt | 13 +- tests/check-upgradeability/test_2.txt | 31 +- tests/check-upgradeability/test_3.txt | 55 ++- tests/check-upgradeability/test_4.txt | 52 ++- tests/check-upgradeability/test_5.txt | 22 +- tests/check-upgradeability/test_6.txt | 25 +- tests/check-upgradeability/test_7.txt | 25 +- tests/check-upgradeability/test_8.txt | 22 +- tests/check-upgradeability/test_9.txt | 25 +- 27 files changed, 1404 insertions(+), 693 deletions(-) delete mode 100644 slither/tools/upgradeability/check_initialization.py delete mode 100644 slither/tools/upgradeability/check_variable_initialization.py create mode 100644 slither/tools/upgradeability/checks/__init__.py create mode 100644 slither/tools/upgradeability/checks/abstract_checks.py create mode 100644 slither/tools/upgradeability/checks/all_checks.py create mode 100644 slither/tools/upgradeability/checks/constant.py create mode 100644 slither/tools/upgradeability/checks/functions_ids.py create mode 100644 slither/tools/upgradeability/checks/initialization.py create mode 100644 slither/tools/upgradeability/checks/variable_initialization.py create mode 100644 slither/tools/upgradeability/checks/variables_order.py delete mode 100644 slither/tools/upgradeability/compare_function_ids.py delete mode 100644 slither/tools/upgradeability/compare_variables_order.py delete mode 100644 slither/tools/upgradeability/constant_checks.py create mode 100644 slither/tools/upgradeability/utils/__init__.py create mode 100644 slither/tools/upgradeability/utils/command_line.py diff --git a/slither/tools/upgradeability/__main__.py b/slither/tools/upgradeability/__main__.py index df43a7dfb..5d9cd1dee 100644 --- a/slither/tools/upgradeability/__main__.py +++ b/slither/tools/upgradeability/__main__.py @@ -1,36 +1,29 @@ -import logging import argparse +import inspect +import json +import logging import sys -from collections import defaultdict -from slither import Slither from crytic_compile import cryticparser + +from slither import Slither from slither.exceptions import SlitherException from slither.utils.colors import red from slither.utils.output import output_to_json - -from .compare_variables_order import compare_variables_order -from .compare_function_ids import compare_function_ids -from .check_initialization import check_initialization -from .check_variable_initialization import check_variable_initialization -from .constant_checks import constant_conformance_check +from .checks import all_checks +from .checks.abstract_checks import AbstractCheck +from .utils.command_line import output_detectors_json, output_wiki, output_detectors, output_to_markdown +from ...utils.command_line import output_detectors_json logging.basicConfig() -logger = logging.getLogger("Slither-check-upgradeability") +logger = logging.getLogger("Slither") logger.setLevel(logging.INFO) -ch = logging.StreamHandler() -ch.setLevel(logging.INFO) -formatter = logging.Formatter('%(message)s') -logger.addHandler(ch) -logger.handlers[0].setFormatter(formatter) -logger.propagate = False def parse_args(): - - parser = argparse.ArgumentParser(description='Slither Upgradeability Checks. For usage information see https://github.com/crytic/slither/wiki/Upgradeability-Checks.', - usage="slither-check-upgradeability contract.sol ContractName") - + parser = argparse.ArgumentParser( + description='Slither Upgradeability Checks. For usage information see https://github.com/crytic/slither/wiki/Upgradeability-Checks.', + usage="slither-check-upgradeability contract.sol ContractName") parser.add_argument('contract.sol', help='Codebase to analyze') parser.add_argument('ContractName', help='Contract name (logic contract)') @@ -45,7 +38,34 @@ def parse_args(): help='Export the results as a JSON file ("--json -" to export to stdout)', action='store', default=False) - + + parser.add_argument('--list-detectors', + help='List available detectors', + action=ListDetectors, + nargs=0, + default=False) + + parser.add_argument('--markdown-root', + help='URL for markdown generation', + action='store', + default="") + + parser.add_argument('--wiki-detectors', + help=argparse.SUPPRESS, + action=OutputWiki, + default=False) + + parser.add_argument('--list-detectors-json', + help=argparse.SUPPRESS, + action=ListDetectorsJson, + nargs=0, + default=False) + + parser.add_argument('--markdown', + help=argparse.SUPPRESS, + action=OutputMarkdown, + default=False) + cryticparser.init(parser) if len(sys.argv) == 1: @@ -57,49 +77,67 @@ def parse_args(): ################################################################################### ################################################################################### -# region +# region checks ################################################################################### ################################################################################### -def _checks_on_contract(contract, json_results): - """ +def _get_checks(): + detectors = [getattr(all_checks, name) for name in dir(all_checks)] + detectors = [c for c in detectors if inspect.isclass(c) and issubclass(c, AbstractCheck)] + return detectors + + +class ListDetectors(argparse.Action): + def __call__(self, parser, *args, **kwargs): + checks = _get_checks() + output_detectors(checks) + parser.exit() + + +class ListDetectorsJson(argparse.Action): + def __call__(self, parser, *args, **kwargs): + checks = _get_checks() + detector_types_json = output_detectors_json(checks) + print(json.dumps(detector_types_json)) + parser.exit() - :param contract: - :param json_results: - :return: - """ - json_results['check-initialization'][contract.name] = check_initialization(contract) - json_results['variable-initialization'][contract.name] = check_variable_initialization(contract) +class OutputMarkdown(argparse.Action): + def __call__(self, parser, args, values, option_string=None): + checks = _get_checks() + output_to_markdown(checks, values) + parser.exit() -def _checks_on_contract_update(contract_v1, contract_v2, json_results): - """ - :param contract_v1: - :param contract_v2: - :param json_results: - :return: - """ +class OutputWiki(argparse.Action): + def __call__(self, parser, args, values, option_string=None): + checks = _get_checks() + output_wiki(checks, values) + parser.exit() - ret = compare_variables_order(contract_v1, contract_v2) - json_results['compare-variables-order-implementation'][contract_v1.name][contract_v2.name] = ret - json_results['constant_conformance'][contract_v1.name][contract_v2.name] = constant_conformance_check(contract_v1, - contract_v2) +def _run_checks(detectors): + results = [d.check() for d in detectors] + results = [r for r in results if r] + results = [item for sublist in results for item in sublist] # flatten + return results -def _checks_on_contract_and_proxy(contract, proxy, json_results, missing_variable_check=True): - """ +def _checks_on_contract(detectors, contract): + detectors = [d(logger, contract) for d in detectors if (not d.REQUIRE_PROXY and + not d.REQUIRE_CONTRACT_V2)] + return _run_checks(detectors), len(detectors) + + +def _checks_on_contract_update(detectors, contract_v1, contract_v2): + detectors = [d(logger, contract_v1, contract_v2=contract_v2) for d in detectors if d.REQUIRE_CONTRACT_V2] + return _run_checks(detectors), len(detectors) + + +def _checks_on_contract_and_proxy(detectors, contract, proxy): + detectors = [d(logger, contract, proxy=proxy) for d in detectors if d.REQUIRE_PROXY] + return _run_checks(detectors), len(detectors) - :param contract: - :param proxy: - :param json_results: - :return: - """ - json_results['compare-function-ids'][contract.name] = compare_function_ids(contract, proxy) - json_results['compare-variables-order-proxy'][contract.name] = compare_variables_order(contract, - proxy, - missing_variable_check) # endregion ################################################################################### @@ -111,20 +149,16 @@ def _checks_on_contract_and_proxy(contract, proxy, json_results, missing_variabl def main(): json_results = { - 'check-initialization': defaultdict(dict), - 'variable-initialization': defaultdict(dict), - 'compare-function-ids': defaultdict(dict), - 'compare-variables-order-implementation': defaultdict(dict), - 'compare-variables-order-proxy': defaultdict(dict), - 'constant_conformance': defaultdict(dict), 'proxy-present': False, - 'contract_v2-present': False + 'contract_v2-present': False, + 'detectors': [] } args = parse_args() v1_filename = vars(args)['contract.sol'] - + number_detectors_run = 0 + detectors = _get_checks() try: v1 = Slither(v1_filename, **vars(args)) @@ -135,10 +169,12 @@ def main(): info = 'Contract {} not found in {}'.format(v1_name, v1.filename) logger.error(red(info)) if args.json: - output_to_json(args.json, str(info), {"upgradeability-check": json_results}) + output_to_json(args.json, str(info), json_results) return - _checks_on_contract(v1_contract, json_results) + detectors_results, number_detectors = _checks_on_contract(detectors, v1_contract) + json_results['detectors'] += detectors_results + number_detectors_run += number_detectors # Analyze Proxy proxy_contract = None @@ -153,11 +189,13 @@ def main(): info = 'Proxy {} not found in {}'.format(args.proxy_name, proxy.filename) logger.error(red(info)) if args.json: - output_to_json(args.json, str(info), {"upgradeability-check": json_results}) + output_to_json(args.json, str(info), json_results) return json_results['proxy-present'] = True - _checks_on_contract_and_proxy(v1_contract, proxy_contract, json_results) + detectors_results, number_detectors = _checks_on_contract_and_proxy(detectors, v1_contract, proxy_contract) + json_results['detectors'] += detectors_results + number_detectors_run += number_detectors # Analyze new version if args.new_contract_name: if args.new_contract_filename: @@ -170,25 +208,34 @@ def main(): info = 'New logic contract {} not found in {}'.format(args.new_contract_name, v2.filename) logger.error(red(info)) if args.json: - output_to_json(args.json, str(info), {"upgradeability-check": json_results}) + output_to_json(args.json, str(info), json_results) return json_results['contract_v2-present'] = True if proxy_contract: - _checks_on_contract_and_proxy(v2_contract, - proxy_contract, - json_results, - missing_variable_check=False) + detectors_results, _ = _checks_on_contract_and_proxy(detectors, + v2_contract, + proxy_contract) + + json_results['detectors'] += detectors_results + + detectors_results, number_detectors = _checks_on_contract_update(detectors, v1_contract, v2_contract) + json_results['detectors'] += detectors_results + number_detectors_run += number_detectors - _checks_on_contract_update(v1_contract, v2_contract, json_results) + # If there is a V2, we run the contract-only check on the V2 + detectors_results, _ = _checks_on_contract(detectors, v2_contract) + json_results['detectors'] += detectors_results + number_detectors_run += number_detectors + logger.info(f'{len(json_results["detectors"])} findings, {number_detectors_run} detectors run') if args.json: - output_to_json(args.json, None, {"upgradeability-check": json_results}) + output_to_json(args.json, None, json_results) except SlitherException as e: logger.error(str(e)) if args.json: - output_to_json(args.json, str(e), {"upgradeability-check": json_results}) + output_to_json(args.json, str(e), json_results) return # endregion diff --git a/slither/tools/upgradeability/check_initialization.py b/slither/tools/upgradeability/check_initialization.py deleted file mode 100644 index 4fe07801a..000000000 --- a/slither/tools/upgradeability/check_initialization.py +++ /dev/null @@ -1,138 +0,0 @@ -import logging - -from slither.slithir.operations import InternalCall -from slither.utils.output import Output -from slither.utils.colors import red, yellow, green - -logger = logging.getLogger("Slither-check-upgradeability") - -class MultipleInitTarget(Exception): - pass - -def _get_initialize_functions(contract): - return [f for f in contract.functions if f.name == 'initialize' and f.is_implemented] - -def _get_all_internal_calls(function): - all_ir = function.all_slithir_operations() - return [i.function for i in all_ir if isinstance(i, InternalCall) and i.function_name == "initialize"] - - -def _get_most_derived_init(contract): - init_functions = [f for f in contract.functions if not f.is_shadowed and f.name == 'initialize'] - if len(init_functions) > 1: - if len([f for f in init_functions if f.contract_declarer == contract]) == 1: - return next((f for f in init_functions if f.contract_declarer == contract)) - raise MultipleInitTarget - if init_functions: - return init_functions[0] - return None - -def check_initialization(contract): - - results = { - 'Initializable-present': False, - 'Initializable-inherited': False, - 'Initializable.initializer()-present': False, - 'missing-initializer-modifier': [], - 'initialize_target': {}, - 'missing-calls': [], - 'multiple-calls': [] - } - - error_found = False - - logger.info(green( - '\n## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)')) - - # Check if the Initializable contract is present - initializable = contract.slither.get_contract_from_name('Initializable') - if initializable is None: - logger.info(yellow('Initializable contract not found, the contract does not follow a standard initalization schema.')) - return results - results['Initializable-present'] = True - - # Check if the Initializable contract is inherited - if initializable not in contract.inheritance: - logger.info( - yellow('The logic contract does not call the initializer.')) - return results - results['Initializable-inherited'] = True - - # Check if the Initializable contract is inherited - initializer = contract.get_modifier_from_canonical_name('Initializable.initializer()') - if initializer is None: - logger.info( - yellow('Initializable.initializer() does not exist')) - return results - results['Initializable.initializer()-present'] = True - - # Check if a init function lacks the initializer modifier - initializer_modifier_missing = False - all_init_functions = _get_initialize_functions(contract) - for f in all_init_functions: - if not initializer in f.modifiers: - initializer_modifier_missing = True - info = f'{f.canonical_name} does not call the initializer modifier' - logger.info(red(info)) - res = Output(info) - res.add(f) - results['missing-initializer-modifier'].append(res.data) - - if not initializer_modifier_missing: - logger.info(green('All the init functions have the initializer modifier')) - - # Check if we can determine the initialize function that will be called - # TODO: handle MultipleInitTarget - try: - most_derived_init = _get_most_derived_init(contract) - except MultipleInitTarget: - logger.info(red('Too many init targets')) - return results - - if most_derived_init is None: - init_info = f'{contract.name} has no initialize function\n' - logger.info(green(init_info)) - results['initialize_target'] = {} - return results - # results['initialize_target'] is set at the end, as we want to print it last - - # Check if an initialize function is not called from the most_derived_init function - missing_call = False - 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: - info = f'Missing call to {f.canonical_name} in {most_derived_init.canonical_name}' - logger.info(red(info)) - res = Output(info) - res.add(f, {"is_most_derived_init_function": False}) - res.add(most_derived_init, {"is_most_derived_init_function": True}) - results['missing-calls'].append(res.data) - missing_call = True - if not missing_call: - logger.info(green('No missing call to an init function found')) - - # Check if an init function is called multiple times - double_calls = list(set([f for f in all_init_functions_called if all_init_functions_called.count(f) > 1])) - double_calls_found = False - for f in double_calls: - info = f'{f.canonical_name} is called multiple times in {most_derived_init.full_name}' - logger.info(red(info)) - res = Output(info) - res.add(f) - results['multiple-calls'].append(res.data) - double_calls_found = True - if not double_calls_found: - logger.info(green('No double call to init functions found')) - - # Print the initialize_target info - - init_info = f'{contract.name} needs to be initialized by {most_derived_init.full_name}\n' - logger.info(green('Check the deployement script to ensure that these functions are called:\n' + init_info)) - res = Output(init_info) - res.add(most_derived_init) - results['initialize_target'] = res.data - - if not error_found: - logger.info(green('No error found')) - - return results diff --git a/slither/tools/upgradeability/check_variable_initialization.py b/slither/tools/upgradeability/check_variable_initialization.py deleted file mode 100644 index cb2473f67..000000000 --- a/slither/tools/upgradeability/check_variable_initialization.py +++ /dev/null @@ -1,31 +0,0 @@ -import logging - -from slither.utils import output -from slither.utils.colors import red, green - -logger = logging.getLogger("Slither-check-upgradeability") - - -def check_variable_initialization(contract): - results = { - 'variables-initialized': [] - } - - logger.info(green( - '\n## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks)')) - - error_found = False - - for s in contract.state_variables: - if s.initialized and not s.is_constant: - info = f'{s.canonical_name} has an initial value ({s.source_mapping_str})' - logger.info(red(info)) - res = output.Output(info) - res.add(s) - results['variables-initialized'].append(res.data) - error_found = True - - if not error_found: - logger.info(green('No error found')) - - return results \ No newline at end of file diff --git a/slither/tools/upgradeability/checks/__init__.py b/slither/tools/upgradeability/checks/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/slither/tools/upgradeability/checks/abstract_checks.py b/slither/tools/upgradeability/checks/abstract_checks.py new file mode 100644 index 000000000..94d55e107 --- /dev/null +++ b/slither/tools/upgradeability/checks/abstract_checks.py @@ -0,0 +1,127 @@ +import abc + +from slither.utils.colors import green, yellow, red +from slither.utils.output import Output + + +class IncorrectCheckInitialization(Exception): + pass + + +class CheckClassification: + HIGH = 0 + MEDIUM = 1 + LOW = 2 + INFORMATIONAL = 3 + + +classification_colors = { + CheckClassification.INFORMATIONAL: green, + CheckClassification.LOW: yellow, + CheckClassification.MEDIUM: yellow, + CheckClassification.HIGH: red +} + +classification_txt = { + CheckClassification.INFORMATIONAL: 'Informational', + CheckClassification.LOW: 'Low', + CheckClassification.MEDIUM: 'Medium', + CheckClassification.HIGH: 'High', +} + + +class AbstractCheck(metaclass=abc.ABCMeta): + ARGUMENT = '' + HELP = '' + IMPACT = None + + WIKI = '' + + WIKI_TITLE = '' + WIKI_DESCRIPTION = '' + WIKI_EXPLOIT_SCENARIO = '' + WIKI_RECOMMENDATION = '' + + REQUIRE_CONTRACT = False + REQUIRE_PROXY = False + REQUIRE_CONTRACT_V2 = False + + def __init__(self, logger, contract, proxy=None, contract_v2=None): + self.logger = logger + self.contract = contract + self.proxy = proxy + self.contract_v2 = contract_v2 + + if not self.ARGUMENT: + raise IncorrectCheckInitialization('NAME is not initialized {}'.format(self.__class__.__name__)) + + if not self.HELP: + raise IncorrectCheckInitialization('HELP is not initialized {}'.format(self.__class__.__name__)) + + if not self.WIKI: + raise IncorrectCheckInitialization('WIKI is not initialized {}'.format(self.__class__.__name__)) + + if not self.WIKI_TITLE: + raise IncorrectCheckInitialization('WIKI_TITLE is not initialized {}'.format(self.__class__.__name__)) + + if not self.WIKI_DESCRIPTION: + raise IncorrectCheckInitialization('WIKI_DESCRIPTION is not initialized {}'.format(self.__class__.__name__)) + + if not self.WIKI_EXPLOIT_SCENARIO and self.IMPACT not in [CheckClassification.INFORMATIONAL]: + raise IncorrectCheckInitialization('WIKI_EXPLOIT_SCENARIO is not initialized {}'.format(self.__class__.__name__)) + + if not self.WIKI_RECOMMENDATION: + raise IncorrectCheckInitialization('WIKI_RECOMMENDATION is not initialized {}'.format(self.__class__.__name__)) + + if self.REQUIRE_PROXY and self.REQUIRE_CONTRACT_V2: + # This is not a fundatemenal issues + # But it requires to change __main__ to avoid running two times the detectors + txt = 'REQUIRE_PROXY and REQUIRE_CONTRACT_V2 needs change in __main___ {}'.format(self.__class__.__name__) + raise IncorrectCheckInitialization(txt) + + if self.IMPACT not in [CheckClassification.LOW, + CheckClassification.MEDIUM, + CheckClassification.HIGH, + CheckClassification.INFORMATIONAL]: + raise IncorrectCheckInitialization('IMPACT is not initialized {}'.format(self.__class__.__name__)) + + if self.REQUIRE_CONTRACT_V2 and contract_v2 is None: + raise IncorrectCheckInitialization('ContractV2 is not initialized {}'.format(self.__class__.__name__)) + + if self.REQUIRE_PROXY and proxy is None: + raise IncorrectCheckInitialization('Proxy is not initialized {}'.format(self.__class__.__name__)) + + @abc.abstractmethod + def _check(self): + """TODO Documentation""" + return [] + + def check(self): + all_results = self._check() + # Keep only dictionaries + all_results = [r.data for r in all_results] + if all_results: + if self.logger: + info = '\n' + for idx, result in enumerate(all_results): + info += result['description'] + info += 'Reference: {}'.format(self.WIKI) + self._log(info) + return all_results + + def generate_result(self, info, additional_fields=None): + output = Output(info, + additional_fields, + markdown_root=self.contract.slither.markdown_root) + + output.data['check'] = self.ARGUMENT + + return output + + def _log(self, info): + if self.logger: + self.logger.info(self.color(info)) + + @property + def color(self): + return classification_colors[self.IMPACT] diff --git a/slither/tools/upgradeability/checks/all_checks.py b/slither/tools/upgradeability/checks/all_checks.py new file mode 100644 index 000000000..1c41316c6 --- /dev/null +++ b/slither/tools/upgradeability/checks/all_checks.py @@ -0,0 +1,11 @@ +from .initialization import (InitializablePresent, InitializableInherited, + InitializableInitializer, MissingInitializerModifier, MissingCalls, MultipleCalls, InitializeTarget) + +from .functions_ids import IDCollision, FunctionShadowing + +from .variable_initialization import VariableWithInit + +from .variables_order import (MissingVariable, DifferentVariableContractProxy, + DifferentVariableContractNewContract, ExtraVariablesProxy, ExtraVariablesNewContract) + +from .constant import WereConstant, BecameConstant \ No newline at end of file diff --git a/slither/tools/upgradeability/checks/constant.py b/slither/tools/upgradeability/checks/constant.py new file mode 100644 index 000000000..60f37f8d3 --- /dev/null +++ b/slither/tools/upgradeability/checks/constant.py @@ -0,0 +1,162 @@ +from slither.tools.upgradeability.checks.abstract_checks import AbstractCheck, CheckClassification + + +class WereConstant(AbstractCheck): + ARGUMENT = 'were-constant' + IMPACT = CheckClassification.HIGH + + HELP = 'Variables that should be constant' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-that-should-be-constant' + WIKI_TITLE = 'Variables that should be constant' + WIKI_DESCRIPTION = ''' +Detect state variables that should be `constant̀€`. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + uint variable1; + uint constant variable2; + uint variable3; +} + +contract ContractV2{ + uint variable1; + uint variable2; + uint variable3; +} +``` +Because `variable2` is not anymore a `constant`, the storage location of `variable3` will be different. +As a result, `ContractV2` will have a corrupted storage layout. +''' + + WIKI_RECOMMENDATION = ''' +Do not remove `constant` from a state variables during an update. +''' + + REQUIRE_CONTRACT = True + REQUIRE_CONTRACT_V2 = True + + def _check(self): + contract_v1 = self.contract + contract_v2 = self.contract_v2 + + state_variables_v1 = contract_v1.state_variables + state_variables_v2 = contract_v2.state_variables + + v2_additional_variables = len(state_variables_v2) - len(state_variables_v1) + if v2_additional_variables < 0: + v2_additional_variables = 0 + + # We keep two index, because we need to have them out of sync if v2 + # has additional non constant variables + idx_v1 = 0 + idx_v2 = 0 + + results = [] + while idx_v1 < len(state_variables_v1): + + state_v1 = contract_v1.state_variables[idx_v1] + if len(state_variables_v2) <= idx_v2: + break + + state_v2 = contract_v2.state_variables[idx_v2] + + if state_v2: + if state_v1.is_constant: + if not state_v2.is_constant: + # If v2 has additional non constant variables, we need to skip them + if ((state_v1.name != state_v2.name or state_v1.type != state_v2.type) + and v2_additional_variables > 0): + v2_additional_variables -= 1 + idx_v2 += 1 + continue + info = [state_v1, ' was constant, but ', state_v2, 'is not.\n'] + json = self.generate_result(info) + results.append(json) + + idx_v1 += 1 + idx_v2 += 1 + + return results + +class BecameConstant(AbstractCheck): + ARGUMENT = 'became-constant' + IMPACT = CheckClassification.HIGH + + HELP = 'Variables that should not be constant' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-that-should-not-be-constant' + WIKI_TITLE = 'Variables that should not be constant' + + WIKI_DESCRIPTION = ''' +Detect state variables that should not be `constant̀€`. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + uint variable1; + uint variable2; + uint variable3; +} + +contract ContractV2{ + uint variable1; + uint constant variable2; + uint variable3; +} +``` +Because `variable2` is now a `constant`, the storage location of `variable3` will be different. +As a result, `ContractV2` will have a corrupted storage layout. +''' + + WIKI_RECOMMENDATION = ''' +Do not make an existing state variable `constant`. +''' + + REQUIRE_CONTRACT = True + REQUIRE_CONTRACT_V2 = True + + def _check(self): + contract_v1 = self.contract + contract_v2 = self.contract_v2 + + state_variables_v1 = contract_v1.state_variables + state_variables_v2 = contract_v2.state_variables + + v2_additional_variables = len(state_variables_v2) - len(state_variables_v1) + if v2_additional_variables < 0: + v2_additional_variables = 0 + + # We keep two index, because we need to have them out of sync if v2 + # has additional non constant variables + idx_v1 = 0 + idx_v2 = 0 + + results = [] + while idx_v1 < len(state_variables_v1): + + state_v1 = contract_v1.state_variables[idx_v1] + if len(state_variables_v2) <= idx_v2: + break + + state_v2 = contract_v2.state_variables[idx_v2] + + if state_v2: + if state_v1.is_constant: + if not state_v2.is_constant: + # If v2 has additional non constant variables, we need to skip them + if ((state_v1.name != state_v2.name or state_v1.type != state_v2.type) + and v2_additional_variables > 0): + v2_additional_variables -= 1 + idx_v2 += 1 + continue + elif state_v2.is_constant: + info = [state_v1, ' was not constant but ', state_v2, ' is.\n'] + json = self.generate_result(info) + results.append(json) + + idx_v1 += 1 + idx_v2 += 1 + + return results diff --git a/slither/tools/upgradeability/checks/functions_ids.py b/slither/tools/upgradeability/checks/functions_ids.py new file mode 100644 index 000000000..ecacbb798 --- /dev/null +++ b/slither/tools/upgradeability/checks/functions_ids.py @@ -0,0 +1,147 @@ +from slither.exceptions import SlitherError +from slither.tools.upgradeability.checks.abstract_checks import AbstractCheck, CheckClassification +from slither.utils.function import get_function_id + + +def get_signatures(c): + functions = c.functions + functions = [f.full_name for f in functions if f.visibility in ['public', 'external'] and + not f.is_constructor and not f.is_fallback] + + variables = c.state_variables + variables = [variable.name + '()' for variable in variables if variable.visibility in ['public']] + return list(set(functions + variables)) + + +def _get_function_or_variable(contract, signature): + f = contract.get_function_from_signature(signature) + + if f: + return f + + for variable in contract.state_variables: + # Todo: can lead to incorrect variable in case of shadowing + if variable.visibility in ['public']: + if variable.name + '()' == signature: + return variable + + raise SlitherError(f'Function id checks: {signature} not found in {contract.name}') + + +class IDCollision(AbstractCheck): + ARGUMENT = 'function-id-collision' + IMPACT = CheckClassification.HIGH + + HELP = 'Functions ids collision' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-collisions' + WIKI_TITLE = 'Functions ids collisions' + + WIKI_DESCRIPTION = ''' +Detect function id collision between the contract and the proxy. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + function gsf() public { + // ... + } +} + +contract Proxy{ + function tgeo() public { + // ... + } +} +``` +`Proxy.tgeo()` and `Contract.gsf()` have the same function id (0x67e43e43). +As a result, `Proxy.tgeo()` will shadow Contract.gsf()`. +''' + + WIKI_RECOMMENDATION = ''' +Rename the function. Avoid public functions in the proxy. +''' + + REQUIRE_CONTRACT = True + REQUIRE_PROXY = True + + def _check(self): + signatures_implem = get_signatures(self.contract) + signatures_proxy = get_signatures(self.proxy) + + signatures_ids_implem = {get_function_id(s): s for s in signatures_implem} + signatures_ids_proxy = {get_function_id(s): s for s in signatures_proxy} + + results = [] + + for (k, _) in signatures_ids_implem.items(): + if k in signatures_ids_proxy: + if signatures_ids_implem[k] != signatures_ids_proxy[k]: + implem_function = _get_function_or_variable(self.contract, signatures_ids_implem[k]) + proxy_function = _get_function_or_variable(self.proxy, signatures_ids_proxy[k]) + + info = ['Function id collision found: ', implem_function, + ' ', proxy_function, '\n'] + json = self.generate_result(info) + results.append(json) + + return results + + +class FunctionShadowing(AbstractCheck): + ARGUMENT = 'function-shadowing' + IMPACT = CheckClassification.HIGH + + HELP = 'Functions shadowing' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-shadowing' + WIKI_TITLE = 'Functions shadowing' + + WIKI_DESCRIPTION = ''' +Detect function shadowing between the contract and the proxy. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + function get() public { + // ... + } +} + +contract Proxy{ + function get() public { + // ... + } +} +``` +`Proxy.get` will shadow any call to `get()`. As a result `get()` is never executed in the logic contract and cannot be updated. +''' + + WIKI_RECOMMENDATION = ''' +Rename the function. Avoid public functions in the proxy. +''' + + REQUIRE_CONTRACT = True + REQUIRE_PROXY = True + + def _check(self): + signatures_implem = get_signatures(self.contract) + signatures_proxy = get_signatures(self.proxy) + + signatures_ids_implem = {get_function_id(s): s for s in signatures_implem} + signatures_ids_proxy = {get_function_id(s): s for s in signatures_proxy} + + results = [] + + for (k, _) in signatures_ids_implem.items(): + if k in signatures_ids_proxy: + if signatures_ids_implem[k] == signatures_ids_proxy[k]: + implem_function = _get_function_or_variable(self.contract, signatures_ids_implem[k]) + proxy_function = _get_function_or_variable(self.proxy, signatures_ids_proxy[k]) + + info = ['Function shadowing found: ', implem_function, + ' ', proxy_function, '\n'] + json = self.generate_result(info) + results.append(json) + + return results diff --git a/slither/tools/upgradeability/checks/initialization.py b/slither/tools/upgradeability/checks/initialization.py new file mode 100644 index 000000000..3809f294b --- /dev/null +++ b/slither/tools/upgradeability/checks/initialization.py @@ -0,0 +1,333 @@ +import logging + +from slither.slithir.operations import InternalCall +from slither.tools.upgradeability.checks.abstract_checks import AbstractCheck, CheckClassification +from slither.utils.output import Output +from slither.utils.colors import red, yellow, green + +logger = logging.getLogger("Slither-check-upgradeability") + + +class MultipleInitTarget(Exception): + pass + + +def _get_initialize_functions(contract): + return [f for f in contract.functions if f.name == 'initialize' and f.is_implemented] + + +def _get_all_internal_calls(function): + all_ir = function.all_slithir_operations() + return [i.function for i in all_ir if isinstance(i, InternalCall) and i.function_name == "initialize"] + + +def _get_most_derived_init(contract): + init_functions = [f for f in contract.functions if not f.is_shadowed and f.name == 'initialize'] + if len(init_functions) > 1: + if len([f for f in init_functions if f.contract_declarer == contract]) == 1: + return next((f for f in init_functions if f.contract_declarer == contract)) + raise MultipleInitTarget + if init_functions: + return init_functions[0] + return None + + +class InitializablePresent(AbstractCheck): + ARGUMENT = 'init-missing' + IMPACT = CheckClassification.INFORMATIONAL + + HELP = 'Initializable is missing' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing' + WIKI_TITLE = 'Initializable is missing' + WIKI_DESCRIPTION = ''' +Detect if a contract `Initializable` is present. +''' + + WIKI_RECOMMENDATION = ''' +Review manually the contract's initialization.. +Consider using a `Initializable` contract to follow [standard practice](https://docs.openzeppelin.com/upgrades/2.7/writing-upgradeable). +''' + + def _check(self): + initializable = self.contract.slither.get_contract_from_name('Initializable') + if initializable is None: + info = ["Initializable contract not found, the contract does not follow a standard initalization schema.\n"] + json = self.generate_result(info) + return [json] + return [] + + +class InitializableInherited(AbstractCheck): + ARGUMENT = 'init-inherited' + IMPACT = CheckClassification.INFORMATIONAL + + HELP = 'Initializable is not inherited' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-not-inherited' + WIKI_TITLE = 'Initializable is not inherited' + + WIKI_DESCRIPTION = ''' +Detect if `Initializable` is inherited. +''' + + WIKI_RECOMMENDATION = ''' +Review manually the contract's initialization. Consider inheriting `Initializable`. +''' + + REQUIRE_CONTRACT = True + + def _check(self): + initializable = self.contract.slither.get_contract_from_name('Initializable') + # See InitializablePresent + if initializable is None: + return [] + if initializable not in self.contract.inheritance: + info = [self.contract, ' does not inherit from ', initializable, '.\n'] + json = self.generate_result(info) + return [json] + return [] + + +class InitializableInitializer(AbstractCheck): + ARGUMENT = 'initializer-missing' + IMPACT = CheckClassification.INFORMATIONAL + + HELP = 'initializer() is missing' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializer-is-missing' + WIKI_TITLE = 'initializer() is missing' + + WIKI_DESCRIPTION = ''' +Detect the lack of `Initializable.initializer()` modifier. +''' + + WIKI_RECOMMENDATION = ''' +Review manually the contract's initialization. Consider inheriting a `Initializable.initializer()` modifier. +''' + + REQUIRE_CONTRACT = True + + def _check(self): + initializable = self.contract.slither.get_contract_from_name('Initializable') + # See InitializablePresent + if initializable is None: + return [] + # See InitializableInherited + if initializable not in self.contract.inheritance: + return [] + + initializer = self.contract.get_modifier_from_canonical_name('Initializable.initializer()') + if initializer is None: + info = ['Initializable.initializer() does not exist.\n'] + json = self.generate_result(info) + return [json] + return [] + + +class MissingInitializerModifier(AbstractCheck): + ARGUMENT = 'missing-init-modifier' + IMPACT = CheckClassification.HIGH + + HELP = 'initializer() is not called' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializer-is-not-called' + WIKI_TITLE = 'initializer() is not called' + WIKI_DESCRIPTION = ''' +Detect if `Initializable.initializer()` is called. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + function initialize() public{ + /// + } +} + +``` +`initialize` should have the `initializer` modifier to prevent someone from initializing the contract multiple times. +''' + + WIKI_RECOMMENDATION = ''' +Use `Initializable.initializer()`. +''' + + REQUIRE_CONTRACT = True + + def _check(self): + initializable = self.contract.slither.get_contract_from_name('Initializable') + # See InitializablePresent + if initializable is None: + return [] + # See InitializableInherited + if initializable not in self.contract.inheritance: + return [] + initializer = self.contract.get_modifier_from_canonical_name('Initializable.initializer()') + # InitializableInitializer + if initializer 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'] + json = self.generate_result(info) + results.append(json) + return results + + +class MissingCalls(AbstractCheck): + ARGUMENT = 'missing-calls' + IMPACT = CheckClassification.HIGH + + HELP = 'Missing calls to init functions' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-not-called' + WIKI_TITLE = 'Initialize functions are not called' + WIKI_DESCRIPTION = ''' +Detect missing calls to initialize functions. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Base{ + function initialize() public{ + /// + } +} +contract Derived is Base{ + function initialize() public{ + /// + } +} + +``` +`Derived.initialize` does not call `Base.initialize` leading the contract to not be correctly initialized. +''' + + WIKI_RECOMMENDATION = ''' +Ensure all the initialize functions are reached by the most derived initialize function. +''' + + REQUIRE_CONTRACT = True + + def _check(self): + results = [] + + # TODO: handle MultipleInitTarget + try: + most_derived_init = _get_most_derived_init(self.contract) + except MultipleInitTarget: + logger.error(red(f'Too many init targets in {self.contract}')) + return [] + + if most_derived_init is None: + return [] + + all_init_functions = _get_initialize_functions(self.contract) + 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: + info = ['Missing call to ', f, ' in ', most_derived_init, '.\n'] + json = self.generate_result(info) + results.append(json) + return results + + +class MultipleCalls(AbstractCheck): + ARGUMENT = 'multiple-calls' + IMPACT = CheckClassification.HIGH + + HELP = 'Init functions called multiple times' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-called-multiple-times' + WIKI_TITLE = 'Initialize functions are called multiple times' + WIKI_DESCRIPTION = ''' +Detect multiple calls to a initialize function. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Base{ + function initialize(uint) public{ + /// + } +} +contract Derived is Base{ + function initialize(uint a, uint b) public{ + initialize(a); + } +} + +contract DerivedDerived is Derived{ + function initialize() public{ + initialize(0); + initialize(0, 1 ); + } +} + +``` +`Base.initialize(uint)` is called two times in `DerivedDerived.initiliaze` execution, leading to a potential corruption. +''' + + WIKI_RECOMMENDATION = ''' +Call only one time every initialize function. +''' + + REQUIRE_CONTRACT = True + + def _check(self): + results = [] + + # TODO: handle MultipleInitTarget + try: + most_derived_init = _get_most_derived_init(self.contract) + except MultipleInitTarget: + # Should be already reported by MissingCalls + #logger.error(red(f'Too many init targets in {self.contract}')) + return [] + + if most_derived_init is None: + return [] + + all_init_functions_called = _get_all_internal_calls(most_derived_init) + [most_derived_init] + double_calls = list(set([f for f in all_init_functions_called if all_init_functions_called.count(f) > 1])) + for f in double_calls: + info = [f, ' is called multiple times in ', most_derived_init, '.\n'] + json = self.generate_result(info) + results.append(json) + + return results + +class InitializeTarget(AbstractCheck): + ARGUMENT = 'initialize-target' + IMPACT = CheckClassification.INFORMATIONAL + + HELP = 'Initialize function that must be called' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function' + WIKI_TITLE = 'Initialize function' + + WIKI_DESCRIPTION = ''' +Show the function that must be called at deployment. + +This finding does not have an immediate security impact and is informative. +''' + + WIKI_RECOMMENDATION = ''' +Ensure that the function is called at deployment. +''' + + REQUIRE_CONTRACT = True + + def _check(self): + + # TODO: handle MultipleInitTarget + try: + most_derived_init = _get_most_derived_init(self.contract) + except MultipleInitTarget: + # Should be already reported by MissingCalls + #logger.error(red(f'Too many init targets in {self.contract}')) + return [] + + if most_derived_init is None: + return [] + + info = [self.contract, f' needs to be initialized by ', most_derived_init, '.\n'] + json = self.generate_result(info) + return [json] diff --git a/slither/tools/upgradeability/checks/variable_initialization.py b/slither/tools/upgradeability/checks/variable_initialization.py new file mode 100644 index 000000000..9c03e0789 --- /dev/null +++ b/slither/tools/upgradeability/checks/variable_initialization.py @@ -0,0 +1,38 @@ +from slither.tools.upgradeability.checks.abstract_checks import CheckClassification, AbstractCheck + + +class VariableWithInit(AbstractCheck): + ARGUMENT = 'variables-initialized' + IMPACT = CheckClassification.HIGH + + HELP = 'State variables with an initial value' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#state-variable-initialized' + WIKI_TITLE = 'State variable initialized' + + WIKI_DESCRIPTION = ''' +Detect state variables that are initialized. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + uint variable = 10; +} +``` +Using `Contract` will the delegatecall proxy pattern will lead `variable` to be 0 when called through the proxy. +''' + + WIKI_RECOMMENDATION = ''' +Using initialize functions to write initial values in state variables. +''' + + REQUIRE_CONTRACT = True + + def _check(self): + results = [] + for s in self.contract.state_variables: + if s.initialized and not s.is_constant: + info = [s, ' is a state variable with an initial value.\n'] + json = self.generate_result(info) + results.append(json) + return results diff --git a/slither/tools/upgradeability/checks/variables_order.py b/slither/tools/upgradeability/checks/variables_order.py new file mode 100644 index 000000000..f300b9dca --- /dev/null +++ b/slither/tools/upgradeability/checks/variables_order.py @@ -0,0 +1,238 @@ +from slither.tools.upgradeability.checks.abstract_checks import CheckClassification, AbstractCheck + + +class MissingVariable(AbstractCheck): + ARGUMENT = 'missing-variables' + IMPACT = CheckClassification.MEDIUM + + HELP = 'Variable missing in the v2' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#missing-variables' + WIKI_TITLE = 'Missing variables' + WIKI_DESCRIPTION = ''' +Detect variables that were present in the original contracts but are not in the updated one. +''' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract V1{ + uint variable1; + uint variable2; +} + +contract V2{ + uint variable1; +} +``` +The new version, `V2` does not contain `variable1`. +If a new variable is added in an update of `V2`, this variable will hold the latest value of `variable2` and +will be corrupted. +''' + + WIKI_RECOMMENDATION = ''' +Do not change the order of the state variables in the updated contract. +''' + + REQUIRE_CONTRACT = True + REQUIRE_CONTRACT_V2 = True + + def _check(self): + contract1 = self.contract + contract2 = self.contract_v2 + order1 = [variable for variable in contract1.state_variables if not variable.is_constant] + order2 = [variable for variable in contract2.state_variables if not variable.is_constant] + + results = [] + for idx in range(0, len(order1)): + variable1 = order1[idx] + if len(order2) <= idx: + info = ['Variable missing in ', contract2, ': ', variable1, '\n'] + json = self.generate_result(info) + results.append(json) + + return results + + +class DifferentVariableContractProxy(AbstractCheck): + ARGUMENT = 'order-vars-proxy' + IMPACT = CheckClassification.HIGH + + HELP = 'Incorrect vars order with the proxy' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-proxy' + WIKI_TITLE = 'Incorrect variables with the proxy' + + WIKI_DESCRIPTION = ''' +Detect variables that are different between the contract and the proxy. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + uint variable1; +} + +contract Proxy{ + address variable1; +} +``` +`Contract` and `Proxy` do not have the same storage layout. As a result the storage of both contracts can be corrupted. +''' + + WIKI_RECOMMENDATION = ''' +Avoid variables in the proxy. If a variable is in the proxy, ensure it has the same layout than in the contract. +''' + + REQUIRE_CONTRACT = True + REQUIRE_PROXY = True + + def _contract1(self): + return self.contract + + def _contract2(self): + return self.proxy + + def _check(self): + contract1 = self._contract1() + contract2 = self._contract2() + order1 = [variable for variable in contract1.state_variables if not variable.is_constant] + order2 = [variable for variable in contract2.state_variables if not variable.is_constant] + + results = [] + for idx in range(0, len(order1)): + if len(order2) <= idx: + # Handle by MissingVariable + return results + + variable1 = order1[idx] + variable2 = order2[idx] + if (variable1.name != variable2.name) or (variable1.type != variable2.type): + info = ['Different variables between ', contract1, ' and ', contract2, '\n'] + info += [f'\t ', variable1, '\n'] + info += [f'\t ', variable2, '\n'] + json = self.generate_result(info) + results.append(json) + + return results + + +class DifferentVariableContractNewContract(DifferentVariableContractProxy): + ARGUMENT = 'order-vars-contracts' + + HELP = 'Incorrect vars order with the v2' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2' + WIKI_TITLE = 'Incorrect variables with the v2' + + WIKI_DESCRIPTION = ''' +Detect variables that are different between the original contract and the updated one. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + uint variable1; +} + +contract ContractV2{ + address variable1; +} +``` +`Contract` and `ContractV2` do not have the same storage layout. As a result the storage of both contracts can be corrupted. +''' + + WIKI_RECOMMENDATION = ''' +Respect the variable order of the original contract in the updated contract. +''' + + REQUIRE_CONTRACT = True + REQUIRE_PROXY = False + REQUIRE_CONTRACT_V2 = True + + def _contract2(self): + return self.contract_v2 + + +class ExtraVariablesProxy(AbstractCheck): + ARGUMENT = 'extra-vars-proxy' + IMPACT = CheckClassification.MEDIUM + + HELP = 'Extra vars in the proxy' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-proxy' + WIKI_TITLE = 'Extra variables in the proxy' + + WIKI_DESCRIPTION = ''' +Detect variables that are in the proxy and not in the contract. +''' + + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Contract{ + uint variable1; +} + +contract Proxy{ + uint variable1; + uint variable2; +} +``` +`Proxy` contains additional variables. A future update of `Contract` is likely to corrupt the proxy. +''' + + WIKI_RECOMMENDATION = ''' +Avoid variables in the proxy. If a variable is in the proxy, ensure it has the same layout than in the contract. +''' + + REQUIRE_CONTRACT = True + REQUIRE_PROXY = True + + def _contract1(self): + return self.contract + + def _contract2(self): + return self.proxy + + def _check(self): + contract1 = self._contract1() + contract2 = self._contract2() + order1 = [variable for variable in contract1.state_variables if not variable.is_constant] + order2 = [variable for variable in contract2.state_variables if not variable.is_constant] + + results = [] + + if len(order2) <= len(order1): + return [] + + idx = len(order2) - len(order1) + + while idx < len(order2): + variable2 = order2[idx] + info = ['Extra variables in ', contract2, ': ', variable2, '\n'] + json = self.generate_result(info) + results.append(json) + idx = idx + 1 + + return results + + +class ExtraVariablesNewContract(ExtraVariablesProxy): + ARGUMENT = 'extra-vars-v2' + + HELP = 'Extra vars in the v2' + WIKI = 'https://github.com/crytic/slither/wiki/Upgradeability-Checks#extra-variables-in-the-v2' + WIKI_TITLE = 'Extra variables in the v2' + + WIKI_DESCRIPTION = ''' +Show new variables in the updated contract. + +This finding does not have an immediate security impact and is informative. +''' + + WIKI_RECOMMENDATION = ''' +Ensure that all the new variables are expected. +''' + + IMPACT = CheckClassification.INFORMATIONAL + + REQUIRE_CONTRACT = True + REQUIRE_PROXY = False + REQUIRE_CONTRACT_V2 = True + + def _contract2(self): + return self.contract_v2 diff --git a/slither/tools/upgradeability/compare_function_ids.py b/slither/tools/upgradeability/compare_function_ids.py deleted file mode 100644 index 34b63d8bf..000000000 --- a/slither/tools/upgradeability/compare_function_ids.py +++ /dev/null @@ -1,86 +0,0 @@ -''' - Check for functions collisions between a proxy and the implementation - More for information: https://medium.com/nomic-labs-blog/malicious-backdoors-in-ethereum-proxies-62629adf3357 -''' - -import logging -from slither.core.declarations import Function -from slither.exceptions import SlitherError -from slither.utils.output import Output -from slither.utils.function import get_function_id -from slither.utils.colors import red, green - -logger = logging.getLogger("Slither-check-upgradeability") - -def get_signatures(c): - functions = c.functions - functions = [f.full_name for f in functions if f.visibility in ['public', 'external'] and - not f.is_constructor and not f.is_fallback] - - variables = c.state_variables - variables = [variable.name+ '()' for variable in variables if variable.visibility in ['public']] - return list(set(functions+variables)) - - -def _get_function_or_variable(contract, signature): - f = contract.get_function_from_signature(signature) - - if f: - return f - - for variable in contract.state_variables: - # Todo: can lead to incorrect variable in case of shadowing - if variable.visibility in ['public']: - if variable.name + '()' == signature: - return variable - - raise SlitherError(f'Function id checks: {signature} not found in {contract.name}') - -def compare_function_ids(implem, proxy): - - results = { - 'function-id-collision':[], - 'shadowing':[], - } - - logger.info(green('\n## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks)')) - - signatures_implem = get_signatures(implem) - signatures_proxy = get_signatures(proxy) - - signatures_ids_implem = {get_function_id(s): s for s in signatures_implem} - signatures_ids_proxy = {get_function_id(s): s for s in signatures_proxy} - - error_found = False - for (k, _) in signatures_ids_implem.items(): - if k in signatures_ids_proxy: - error_found = True - if signatures_ids_implem[k] != signatures_ids_proxy[k]: - - implem_function = _get_function_or_variable(implem, signatures_ids_implem[k]) - proxy_function = _get_function_or_variable(proxy, signatures_ids_proxy[k]) - - info = f'Function id collision found: {implem_function.canonical_name} ({implem_function.source_mapping_str}) {proxy_function.canonical_name} ({proxy_function.source_mapping_str})' - logger.info(red(info)) - res = Output(info) - res.add(implem_function) - res.add(proxy_function) - results['function-id-collision'].append(res.data) - - else: - - implem_function = _get_function_or_variable(implem, signatures_ids_implem[k]) - proxy_function = _get_function_or_variable(proxy, signatures_ids_proxy[k]) - - info = f'Shadowing between {implem_function.canonical_name} ({implem_function.source_mapping_str}) and {proxy_function.canonical_name} ({proxy_function.source_mapping_str})' - logger.info(red(info)) - - res = Output(info) - res.add(implem_function) - res.add(proxy_function) - results['shadowing'].append(res.data) - - if not error_found: - logger.info(green('No error found')) - - return results diff --git a/slither/tools/upgradeability/compare_variables_order.py b/slither/tools/upgradeability/compare_variables_order.py deleted file mode 100644 index ea5852d3f..000000000 --- a/slither/tools/upgradeability/compare_variables_order.py +++ /dev/null @@ -1,73 +0,0 @@ -''' - Check if the variables respect the same ordering -''' -import logging - -from slither.utils.output import Output -from slither.utils.colors import red, green, yellow - -logger = logging.getLogger("Slither-check-upgradeability") - - -def compare_variables_order(contract1, contract2, missing_variable_check=True): - - results = { - 'missing_variables': [], - 'different-variables': [], - 'extra-variables': [] - } - - logger.info(green( - f'\n## Run variables ordering checks between {contract1.name} and {contract2.name}... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)')) - - order1 = [variable for variable in contract1.state_variables if not variable.is_constant] - order2 = [variable for variable in contract2.state_variables if not variable.is_constant] - - error_found = False - idx = 0 - for idx in range(0, len(order1)): - variable1 = order1[idx] - if len(order2) <= idx: - if missing_variable_check: - info = f'Variable only in {contract1.name}: {variable1.name} ({variable1.source_mapping_str})' - logger.info(yellow(info)) - - res = Output(info) - res.add(variable1) - results['missing_variables'].append(res.data) - - error_found = True - continue - - variable2 = order2[idx] - - if (variable1.name != variable2.name) or (variable1.type != variable2.type): - info = f'Different variables between {contract1.name} and {contract2.name}:\n' - info += f'\t Variable {idx} in {contract1.name}: {variable1.name} {variable1.type} ({variable1.source_mapping_str})\n' - info += f'\t Variable {idx} in {contract2.name}: {variable2.name} {variable2.type} ({variable2.source_mapping_str})\n' - logger.info(red(info)) - - res = Output(info, additional_fields={'index': idx}) - res.add(variable1) - res.add(variable2) - results['different-variables'].append(res.data) - - error_found = True - - idx = idx + 1 - - while idx < len(order2): - variable2 = order2[idx] - - info = f'Extra variables in {contract2.name}: {variable2.name} ({variable2.source_mapping_str})\n' - logger.info(yellow(info)) - res = Output(info, additional_fields={'index': idx}) - res.add(variable2) - results['extra-variables'].append(res.data) - idx = idx + 1 - - if not error_found: - logger.info(green('No error found')) - - return results - diff --git a/slither/tools/upgradeability/constant_checks.py b/slither/tools/upgradeability/constant_checks.py deleted file mode 100644 index e25924350..000000000 --- a/slither/tools/upgradeability/constant_checks.py +++ /dev/null @@ -1,85 +0,0 @@ -import logging - -from slither.utils.output import Output -from slither.utils.colors import red, yellow, green - -logger = logging.getLogger("Slither-check-upgradeability") - -def constant_conformance_check(contract_v1, contract_v2): - - results = { - "became_constants": [], - "were_constants": [], - "not_found_in_v2": [], - } - - logger.info(green( - '\n## Run variable constants conformance check... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks)')) - error_found = False - - state_variables_v1 = contract_v1.state_variables - state_variables_v2 = contract_v2.state_variables - - v2_additional_variables = len(state_variables_v2) - len(state_variables_v1) - if v2_additional_variables < 0: - v2_additional_variables = 0 - - # We keep two index, because we need to have them out of sync if v2 - # has additional non constant variables - idx_v1 = 0 - idx_v2 = 0 - while idx_v1 < len(state_variables_v1): - - state_v1 = contract_v1.state_variables[idx_v1] - if len(state_variables_v2) <= idx_v2: - break - - state_v2 = contract_v2.state_variables[idx_v2] - - if state_v2: - if state_v1.is_constant: - if not state_v2.is_constant: - - # If v2 has additional non constant variables, we need to skip them - if (state_v1.name != state_v2.name or state_v1.type != state_v2.type) and v2_additional_variables>0: - v2_additional_variables -= 1 - idx_v2 += 1 - continue - - info = f'{state_v1.canonical_name} ({state_v1.source_mapping_str}) was constant and {state_v2.canonical_name} is not ({state_v2.source_mapping_str})' - logger.info(red(info)) - - res = Output(info) - res.add(state_v1) - res.add(state_v2) - results['were_constants'].append(res.data) - error_found = True - - elif state_v2.is_constant: - info = f'{state_v1.canonical_name} ({state_v1.source_mapping_str}) was not constant but {state_v2.canonical_name} is ({state_v2.source_mapping_str})' - logger.info(red(info)) - - res = Output(info) - res.add(state_v1) - res.add(state_v2) - results['became_constants'].append(res.data) - error_found = True - - else: - info = f'{state_v1.canonical_name} not found in {contract_v2.name}, not check was done' - logger.info(yellow(info)) - - res = Output(info) - res.add(state_v1) - res.add(contract_v2) - results['not_found_in_v2'].append(res.data) - - error_found = True - - idx_v1 += 1 - idx_v2 += 1 - - if not error_found: - logger.info(green('No error found')) - - return results \ No newline at end of file diff --git a/slither/tools/upgradeability/utils/__init__.py b/slither/tools/upgradeability/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/slither/tools/upgradeability/utils/command_line.py b/slither/tools/upgradeability/utils/command_line.py new file mode 100644 index 000000000..2b5210b80 --- /dev/null +++ b/slither/tools/upgradeability/utils/command_line.py @@ -0,0 +1,126 @@ +from prettytable import PrettyTable + +from slither.tools.upgradeability.checks.abstract_checks import classification_txt + + +def output_wiki(detector_classes, filter_wiki): + # Sort by impact, confidence, and name + detectors_list = sorted(detector_classes, key=lambda element: (element.IMPACT, element.ARGUMENT)) + + for detector in detectors_list: + if filter_wiki not in detector.WIKI: + continue + argument = detector.ARGUMENT + impact = classification_txt[detector.IMPACT] + title = detector.WIKI_TITLE + description = detector.WIKI_DESCRIPTION + exploit_scenario = detector.WIKI_EXPLOIT_SCENARIO + recommendation = detector.WIKI_RECOMMENDATION + + print('\n## {}'.format(title)) + print('### Configuration') + print('* Check: `{}`'.format(argument)) + print('* Severity: `{}`'.format(impact)) + print('\n### Description') + print(description) + if exploit_scenario: + print('\n### Exploit Scenario:') + print(exploit_scenario) + print('\n### Recommendation') + print(recommendation) + + +def output_detectors(detector_classes): + detectors_list = [] + for detector in detector_classes: + argument = detector.ARGUMENT + help_info = detector.HELP + impact = detector.IMPACT + require_proxy = detector.REQUIRE_PROXY + require_v2 = detector.REQUIRE_CONTRACT_V2 + detectors_list.append((argument, help_info, impact, require_proxy, require_v2)) + table = PrettyTable(["Num", + "Check", + "What it Detects", + "Impact", + "Proxy", + "Contract V2"]) + + # Sort by impact, confidence, and name + detectors_list = sorted(detectors_list, key=lambda element: (element[2], element[0])) + idx = 1 + for (argument, help_info, impact, proxy, v2) in detectors_list: + table.add_row([idx, argument, help_info, classification_txt[impact], 'X' if proxy else '', 'X' if v2 else '']) + idx = idx + 1 + print(table) + + +def output_to_markdown(detector_classes, filter_wiki): + def extract_help(cls): + if cls.WIKI == '': + return cls.HELP + return '[{}]({})'.format(cls.HELP, cls.WIKI) + + detectors_list = [] + for detector in detector_classes: + argument = detector.ARGUMENT + help_info = extract_help(detector) + impact = detector.IMPACT + require_proxy = detector.REQUIRE_PROXY + require_v2 = detector.REQUIRE_CONTRACT_V2 + detectors_list.append((argument, help_info, impact, require_proxy, require_v2)) + table = PrettyTable(["Num", + "Check", + "What it Detects", + "Impact", + "Proxy", + "Contract V2"]) + + # Sort by impact, confidence, and name + detectors_list = sorted(detectors_list, key=lambda element: (element[2], element[0])) + idx = 1 + for (argument, help_info, impact, proxy, v2) in detectors_list: + print('{} | `{}` | {} | {} | {} | {}'.format(idx, + argument, + help_info, + classification_txt[impact], + 'X' if proxy else '', + 'X' if v2 else '')) + idx = idx + 1 + + + + +def output_detectors_json(detector_classes): + detectors_list = [] + for detector in detector_classes: + argument = detector.ARGUMENT + help_info = detector.HELP + impact = detector.IMPACT + wiki_url = detector.WIKI + wiki_description = detector.WIKI_DESCRIPTION + wiki_exploit_scenario = detector.WIKI_EXPLOIT_SCENARIO + wiki_recommendation = detector.WIKI_RECOMMENDATION + detectors_list.append((argument, + help_info, + impact, + wiki_url, + wiki_description, + wiki_exploit_scenario, + wiki_recommendation)) + + # Sort by impact, confidence, and name + detectors_list = sorted(detectors_list, key=lambda element: (element[2], element[0])) + idx = 1 + table = [] + for (argument, help_info, impact, wiki_url, description, exploit, recommendation) in detectors_list: + table.append({'index': idx, + 'detector': argument, + 'title': help_info, + 'impact': classification_txt[impact], + 'wiki_url': wiki_url, + 'description': description, + 'exploit_scenario':exploit, + 'recommendation':recommendation}) + idx = idx + 1 + return table diff --git a/tests/check-upgradeability/test_1.txt b/tests/check-upgradeability/test_1.txt index df22e2702..5681c4619 100644 --- a/tests/check-upgradeability/test_1.txt +++ b/tests/check-upgradeability/test_1.txt @@ -1,12 +1,4 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -Initializable contract not found, the contract does not follow a standard initalization schema. - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between ContractV1 and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found +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:1 findings, 12 detectors run diff --git a/tests/check-upgradeability/test_10.txt b/tests/check-upgradeability/test_10.txt index 89589b364..1e0f99584 100644 --- a/tests/check-upgradeability/test_10.txt +++ b/tests/check-upgradeability/test_10.txt @@ -1,12 +1,13 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -Initializable contract not found, the contract does not follow a standard initalization schema. - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run variables ordering checks between ContractV1 and ContractV2... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -Variable only in ContractV1: destination (tests/check-upgradeability/contractV1.sol#2) - -## Run variable constants conformance check... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -ContractV1.destination (tests/check-upgradeability/contractV1.sol#2) was not constant but ContractV2.destination is (tests/check-upgradeability/contract_v2_constant.sol#2) +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. +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) +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. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing +INFO:Slither:4 findings, 18 detectors run diff --git a/tests/check-upgradeability/test_11.txt b/tests/check-upgradeability/test_11.txt index 24764c8c5..f6e8009f5 100644 --- a/tests/check-upgradeability/test_11.txt +++ b/tests/check-upgradeability/test_11.txt @@ -1,6 +1,7 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -Initializable contract not found, the contract does not follow a standard initalization schema. - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -ContractV1.destination has an initial value (tests/check-upgradeability/contract_v1_var_init.sol#2) +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. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#state-variable-initialized +INFO:Slither:2 findings, 8 detectors run diff --git a/tests/check-upgradeability/test_2.txt b/tests/check-upgradeability/test_2.txt index 9137454fe..ecff55f48 100644 --- a/tests/check-upgradeability/test_2.txt +++ b/tests/check-upgradeability/test_2.txt @@ -1,24 +1,7 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -Initializable contract not found, the contract does not follow a standard initalization schema. - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between ContractV1 and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between ContractV2 and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found - -## Run variables ordering checks between ContractV1 and ContractV2... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found - -## Run variable constants conformance check... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found +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: +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, 22 detectors run diff --git a/tests/check-upgradeability/test_3.txt b/tests/check-upgradeability/test_3.txt index ebb21b107..8b40490a8 100644 --- a/tests/check-upgradeability/test_3.txt +++ b/tests/check-upgradeability/test_3.txt @@ -1,32 +1,23 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -Initializable contract not found, the contract does not follow a standard initalization schema. - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between ContractV1 and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -Shadowing between ContractV2.myFunc (tests/check-upgradeability/contractV2_bug.sol#4) and Proxy.myFunc() (tests/check-upgradeability/proxy.sol#11) - -## Run variables ordering checks between ContractV2 and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -Different variables between ContractV2 and Proxy: - Variable 0 in ContractV2: destination uint256 (tests/check-upgradeability/contractV2_bug.sol#2) - Variable 0 in Proxy: destination address (tests/check-upgradeability/proxy.sol#9) - - -## Run variables ordering checks between ContractV1 and ContractV2... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -Different variables between ContractV1 and ContractV2: - Variable 0 in ContractV1: destination address (tests/check-upgradeability/contractV1.sol#2) - Variable 0 in ContractV2: destination uint256 (tests/check-upgradeability/contractV2_bug.sol#2) - -Extra variables in ContractV2: myFunc (tests/check-upgradeability/contractV2_bug.sol#4) - - -## Run variable constants conformance check... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found +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) +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) +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) +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) +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. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing +INFO:Slither:6 findings, 22 detectors run diff --git a/tests/check-upgradeability/test_4.txt b/tests/check-upgradeability/test_4.txt index cf9c9acd6..7411e06cb 100644 --- a/tests/check-upgradeability/test_4.txt +++ b/tests/check-upgradeability/test_4.txt @@ -1,32 +1,20 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -Initializable contract not found, the contract does not follow a standard initalization schema. - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between ContractV1 and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between ContractV2 and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -Different variables between ContractV2 and Proxy: - Variable 0 in ContractV2: val uint256 (tests/check-upgradeability/contractV2_bug2.sol#2) - Variable 0 in Proxy: destination address (tests/check-upgradeability/proxy.sol#9) - - -## Run variables ordering checks between ContractV1 and ContractV2... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -Different variables between ContractV1 and ContractV2: - Variable 0 in ContractV1: destination address (tests/check-upgradeability/contractV1.sol#2) - Variable 0 in ContractV2: val uint256 (tests/check-upgradeability/contractV2_bug2.sol#2) - -Extra variables in ContractV2: destination (tests/check-upgradeability/contractV2_bug2.sol#5) - - -## Run variable constants conformance check... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found +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) +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) +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) +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. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing +INFO:Slither:5 findings, 22 detectors run diff --git a/tests/check-upgradeability/test_5.txt b/tests/check-upgradeability/test_5.txt index 58547b641..99bdf0bab 100644 --- a/tests/check-upgradeability/test_5.txt +++ b/tests/check-upgradeability/test_5.txt @@ -1,18 +1,4 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -All the init functions have the initializer modifier -No missing call to an init function found -No double call to init functions found -Check the deployement script to ensure that these functions are called: -Contract_no_bug needs to be initialized by initialize() - -No error found - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between Contract_no_bug and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found +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). +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function +INFO:Slither:1 findings, 12 detectors run diff --git a/tests/check-upgradeability/test_6.txt b/tests/check-upgradeability/test_6.txt index 85da69ae4..842708b8f 100644 --- a/tests/check-upgradeability/test_6.txt +++ b/tests/check-upgradeability/test_6.txt @@ -1,18 +1,7 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -Contract_lack_to_call_modifier.initialize() does not call the initializer modifier -No missing call to an init function found -No double call to init functions found -Check the deployement script to ensure that these functions are called: -Contract_lack_to_call_modifier needs to be initialized by initialize() - -No error found - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between Contract_lack_to_call_modifier and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found +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). +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. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializer-is-not-called +INFO:Slither:2 findings, 12 detectors run diff --git a/tests/check-upgradeability/test_7.txt b/tests/check-upgradeability/test_7.txt index d2110df9c..160cd95e4 100644 --- a/tests/check-upgradeability/test_7.txt +++ b/tests/check-upgradeability/test_7.txt @@ -1,18 +1,7 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -All the init functions have the initializer modifier -Missing call to Contract_no_bug.initialize() in Contract_not_called_super_init.initialize() -No double call to init functions found -Check the deployement script to ensure that these functions are called: -Contract_not_called_super_init needs to be initialized by initialize() - -No error found - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between Contract_not_called_super_init and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found +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). +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). +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/check-upgradeability/test_8.txt b/tests/check-upgradeability/test_8.txt index 69ff9ebec..6a7debd30 100644 --- a/tests/check-upgradeability/test_8.txt +++ b/tests/check-upgradeability/test_8.txt @@ -1,18 +1,4 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -All the init functions have the initializer modifier -No missing call to an init function found -No double call to init functions found -Check the deployement script to ensure that these functions are called: -Contract_no_bug_inherits needs to be initialized by initialize() - -No error found - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between Contract_no_bug_inherits and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found +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). +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-function +INFO:Slither:1 findings, 12 detectors run diff --git a/tests/check-upgradeability/test_9.txt b/tests/check-upgradeability/test_9.txt index 3e12a4525..9c2fa353e 100644 --- a/tests/check-upgradeability/test_9.txt +++ b/tests/check-upgradeability/test_9.txt @@ -1,18 +1,7 @@ - -## Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -All the init functions have the initializer modifier -No missing call to an init function found -Contract_no_bug.initialize() is called multiple times in initialize() -Check the deployement script to ensure that these functions are called: -Contract_double_call needs to be initialized by initialize() - -No error found - -## Run variable initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks) -No error found - -## Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks) -No error found - -## Run variables ordering checks between Contract_double_call and Proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks) -No error found +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). +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). +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialize-functions-are-called-multiple-times +INFO:Slither:2 findings, 12 detectors run