From 917967399ac7ceab23041c95caa0f48b4936d838 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 21 Jan 2019 11:20:12 +0100 Subject: [PATCH 1/6] Add function id printer Add function collission detection utility New dependency: pysha3 --- setup.py | 2 +- slither/__main__.py | 4 +- slither/printers/summary/function_ids.py | 34 ++++++++++++++++ slither/utils/function.py | 14 +++++++ utils/upgradability/compare_function_ids.py | 45 +++++++++++++++++++++ 5 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 slither/printers/summary/function_ids.py create mode 100644 slither/utils/function.py create mode 100644 utils/upgradability/compare_function_ids.py diff --git a/setup.py b/setup.py index d4c980c04..fabba5d4e 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ setup( version='0.5.0', packages=find_packages(), python_requires='>=3.6', - install_requires=['prettytable>=0.7.2'], + install_requires=['prettytable>=0.7.2', 'pysha3>=1.0.2'], license='AGPL-3.0', long_description=open('README.md').read(), entry_points={ diff --git a/slither/__main__.py b/slither/__main__.py index 8d8109c22..ef523bd90 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -179,6 +179,7 @@ def get_detectors_and_printers(): from slither.printers.summary.slithir_ssa import PrinterSlithIRSSA from slither.printers.summary.human_summary import PrinterHumanSummary from slither.printers.functions.cfg import CFG + from slither.printers.summary.function_ids import FunctionIds printers = [FunctionSummary, ContractSummary, @@ -189,7 +190,8 @@ def get_detectors_and_printers(): PrinterSlithIR, PrinterSlithIRSSA, PrinterHumanSummary, - CFG] + CFG, + FunctionIds] # Handle plugins! for entry_point in iter_entry_points(group='slither_analyzer.plugin', name=None): diff --git a/slither/printers/summary/function_ids.py b/slither/printers/summary/function_ids.py new file mode 100644 index 000000000..034afd4a5 --- /dev/null +++ b/slither/printers/summary/function_ids.py @@ -0,0 +1,34 @@ +""" + Module printing summary of the contract +""" +import collections +from prettytable import PrettyTable +from slither.printers.abstract_printer import AbstractPrinter +from slither.utils.colors import blue, green, magenta +from slither.utils.function import get_function_id + +class FunctionIds(AbstractPrinter): + + ARGUMENT = 'function-id' + HELP = 'Print the heccack256 signature of the functions' + + def output(self, _filename): + """ + _filename is not used + Args: + _filename(string) + """ + + txt = '' + for contract in self.slither.contracts_derived: + txt = '\n{}:\n'.format(contract.name) + table = PrettyTable(['Name', 'ID']) + for function in contract.functions: + if function.visibility in ['public', 'external']: + table.add_row([function.full_name, hex(get_function_id(function.full_name))]) + for variable in contract.state_variables: + if variable.visibility in ['public']: + table.add_row([variable.name+'()', hex(get_function_id(variable.name+'()'))]) + txt += str(table) + '\n' + + self.info(txt) diff --git a/slither/utils/function.py b/slither/utils/function.py new file mode 100644 index 000000000..7e142e342 --- /dev/null +++ b/slither/utils/function.py @@ -0,0 +1,14 @@ +import hashlib +import sha3 + +def get_function_id(sig): + '''' + Return the function id of the given signature + Args: + sig (str) + Return: + (int) + ''' + s = sha3.keccak_256() + s.update(sig.encode('utf-8')) + return int("0x" + s.hexdigest()[:8], 16) diff --git a/utils/upgradability/compare_function_ids.py b/utils/upgradability/compare_function_ids.py new file mode 100644 index 000000000..9d5f1364e --- /dev/null +++ b/utils/upgradability/compare_function_ids.py @@ -0,0 +1,45 @@ +''' + This utility looks 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 sys +from slither import Slither +from slither.utils.function import get_function_id +from slither.utils.colors import red, green + + +def get_signatures(s): + functions = [contract.functions for contract in s.contracts_derived] + functions = [item for sublist in functions for item in sublist] + functions = [f.full_name for f in functions if f.visibility in ['public', 'external']] + + variables = [contract.state_variables for contract in s.contracts_derived] + variables = [item for sublist in variables for item in sublist] + variables = [variable.name+ '()' for variable in variables if variable.visibility in ['public']] + return list(set(functions+variables)) + +if __name__ == "__main__": + + if len(sys.argv) != 3: + print('Usage: python3 compare_function_ids.py implem.sol proxy.sol') + + implem = Slither(sys.argv[1]) + proxy = Slither(sys.argv[2]) + + 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} + + found = False + for (k,v) in signatures_ids_implem.items(): + if k in signatures_ids_proxy: + found = True + print(red('Collision found {} {}'.format(signatures_ids_implem[k], + signatures_ids_proxy[k]))) + + if not found: + print(green('No collision found')) + From 318f2fe0367fbc46391b4245f9a6bb4b22318730 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 21 Jan 2019 13:22:20 +0100 Subject: [PATCH 2/6] Add print to show the state variables ordering Add utility to compare the state variables order given two version of a contract --- slither/__main__.py | 5 +- slither/core/solidity_types/mapping_type.py | 2 +- slither/printers/summary/function_ids.py | 2 +- slither/printers/summary/variables_order.py | 29 ++++++++++ .../upgradability/compare_variables_order.py | 56 +++++++++++++++++++ 5 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 slither/printers/summary/variables_order.py create mode 100644 utils/upgradability/compare_variables_order.py diff --git a/slither/__main__.py b/slither/__main__.py index 9ef12a008..0b3c7c3bc 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -180,7 +180,7 @@ def get_detectors_and_printers(): from slither.printers.summary.human_summary import PrinterHumanSummary from slither.printers.functions.cfg import CFG from slither.printers.summary.function_ids import FunctionIds - + from slither.printers.summary.variables_order import VariablesOrder printers = [FunctionSummary, ContractSummary, PrinterInheritance, @@ -191,7 +191,8 @@ def get_detectors_and_printers(): PrinterSlithIRSSA, PrinterHumanSummary, CFG, - FunctionIds] + FunctionIds, + VariablesOrder] # Handle plugins! for entry_point in iter_entry_points(group='slither_analyzer.plugin', name=None): diff --git a/slither/core/solidity_types/mapping_type.py b/slither/core/solidity_types/mapping_type.py index 1c016c8c6..cf7e0794a 100644 --- a/slither/core/solidity_types/mapping_type.py +++ b/slither/core/solidity_types/mapping_type.py @@ -18,7 +18,7 @@ class MappingType(Type): return self._to def __str__(self): - return 'mapping({} => {}'.format(str(self._from), str(self._to)) + return 'mapping({} => {})'.format(str(self._from), str(self._to)) def __eq__(self, other): if not isinstance(other, MappingType): diff --git a/slither/printers/summary/function_ids.py b/slither/printers/summary/function_ids.py index 034afd4a5..cba119c11 100644 --- a/slither/printers/summary/function_ids.py +++ b/slither/printers/summary/function_ids.py @@ -21,7 +21,7 @@ class FunctionIds(AbstractPrinter): txt = '' for contract in self.slither.contracts_derived: - txt = '\n{}:\n'.format(contract.name) + txt += '\n{}:\n'.format(contract.name) table = PrettyTable(['Name', 'ID']) for function in contract.functions: if function.visibility in ['public', 'external']: diff --git a/slither/printers/summary/variables_order.py b/slither/printers/summary/variables_order.py new file mode 100644 index 000000000..a32acd53d --- /dev/null +++ b/slither/printers/summary/variables_order.py @@ -0,0 +1,29 @@ +""" + Module printing summary of the contract +""" + +from prettytable import PrettyTable +from slither.printers.abstract_printer import AbstractPrinter + +class VariablesOrder(AbstractPrinter): + + ARGUMENT = 'variables-order' + HELP = 'Print the storage order of the state variables' + + def output(self, _filename): + """ + _filename is not used + Args: + _filename(string) + """ + + txt = '' + for contract in self.slither.contracts_derived: + txt += '\n{}:\n'.format(contract.name) + table = PrettyTable(['Name', 'Type']) + for variable in contract.state_variables: + if not variable.is_constant: + table.add_row([variable.name, str(variable.type)]) + txt += str(table) + '\n' + + self.info(txt) diff --git a/utils/upgradability/compare_variables_order.py b/utils/upgradability/compare_variables_order.py new file mode 100644 index 000000000..64f4238c2 --- /dev/null +++ b/utils/upgradability/compare_variables_order.py @@ -0,0 +1,56 @@ +''' + This utility looks 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 sys +from slither import Slither +from slither.utils.function import get_function_id +from slither.utils.colors import red, green + +if __name__ == "__main__": + + if len(sys.argv) != 5: + print('Usage: python3 compare_variables_order.py v1.sol Contract1 v2.sol Contract2') + + v1 = Slither(sys.argv[1]) + v2 = Slither(sys.argv[3]) + + contract_v1 = v1.get_contract_from_name(sys.argv[2]) + if contract_v1 is None: + print(red('Contract {} not found'.format(sys.argv[2]))) + exit(-1) + + contract_v2 = v2.get_contract_from_name(sys.argv[4]) + if contract_v2 is None: + print(red('Contract {} not found'.format(sys.argv[4]))) + exit(-1) + + + order_v1 = [(variable.name, variable.type) for variable in contract_v1.state_variables if not variable.is_constant] + order_v2 = [(variable.name, variable.type) for variable in contract_v2.state_variables if not variable.is_constant] + + + found = False + for idx in range(0, len(order_v1)): + (v1_name, v1_type) = order_v1[idx] + if len(order_v2) < idx: + print(red('Missing variable in the new version: {} {}'.format(v1_name, v1_type))) + continue + (v2_name, v2_type) = order_v2[idx] + + if (v1_name != v2_name) or (v1_type != v2_type): + found = True + print(red('Different variable: {} {} -> {} {}'.format(v1_name, + v1_type, + v2_name, + v2_type))) + + if len(order_v2) > len(order_v1): + new_variables = order_v2[len(order_v1):] + for (name, t) in new_variables: + print(green('New variable: {} {}'.format(name, t))) + + if not found: + print(green('No error found')) + From 3c792a51f0f22761659dd45693a764851def6d39 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 28 Jan 2019 20:06:30 +0000 Subject: [PATCH 3/6] Create a cli command for upgradability: slither-check-upgradability Improve code --- setup.py | 3 +- slither/printers/summary/function_ids.py | 2 +- utils/upgradability/__main__.py | 46 +++++++++++++++++++ utils/upgradability/compare_function_ids.py | 21 ++++----- .../upgradability/compare_variables_order.py | 35 ++++++-------- 5 files changed, 73 insertions(+), 34 deletions(-) create mode 100644 utils/upgradability/__main__.py diff --git a/setup.py b/setup.py index 40e9b3ac3..c55f9c38b 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,8 @@ setup( long_description=open('README.md').read(), entry_points={ 'console_scripts': [ - 'slither = slither.__main__:main' + 'slither = slither.__main__:main', + 'slither-check-upgradability = utils.upgradability.__main__:main' ] } ) diff --git a/slither/printers/summary/function_ids.py b/slither/printers/summary/function_ids.py index cba119c11..187bd53f4 100644 --- a/slither/printers/summary/function_ids.py +++ b/slither/printers/summary/function_ids.py @@ -10,7 +10,7 @@ from slither.utils.function import get_function_id class FunctionIds(AbstractPrinter): ARGUMENT = 'function-id' - HELP = 'Print the heccack256 signature of the functions' + HELP = 'Print the keccack256 signature of the functions' def output(self, _filename): """ diff --git a/utils/upgradability/__main__.py b/utils/upgradability/__main__.py new file mode 100644 index 000000000..8b3edfcbd --- /dev/null +++ b/utils/upgradability/__main__.py @@ -0,0 +1,46 @@ +import logging +import argparse +import sys + +from slither import Slither + +from .compare_variables_order import compare_variables_order +from .compare_function_ids import compare_function_ids + +logging.basicConfig() +logger = logging.getLogger("Slither-check-upgradability") + +def parse_args(): + + parser = argparse.ArgumentParser(description='Slither Upgradability Checks', + usage="slither-check-upgradability proxy.sol ProxyName v1.sol V1Name v2.sol V2Name") + + + parser.add_argument('proxy.sol', help='Proxy filename') + parser.add_argument('ProxyName', help='Contract name') + + parser.add_argument('v1.sol', help='Version 1 filename') + parser.add_argument('V1Name', help='Contract name') + + parser.add_argument('v2.sol', help='Version 2 filename') + parser.add_argument('V2Name', help='Contract name') + + if len(sys.argv) == 1: + parser.print_help(sys.stderr) + sys.exit(1) + + return parser.parse_args() + +def main(): + args = parse_args() + + + proxy = Slither(vars(args)['proxy.sol']) + proxy_name = args.ProxyName + v1 = Slither(vars(args)['v1.sol']) + v1_name = args.V1Name + v2 = Slither(vars(args)['v2.sol']) + v2_name = args.V2Name + + compare_variables_order(v1, v1_name, v2, v2_name) + compare_function_ids(v2, proxy) diff --git a/utils/upgradability/compare_function_ids.py b/utils/upgradability/compare_function_ids.py index 9d5f1364e..b95e7f110 100644 --- a/utils/upgradability/compare_function_ids.py +++ b/utils/upgradability/compare_function_ids.py @@ -1,13 +1,15 @@ ''' - This utility looks for functions collisions between a proxy and the implementation + 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 sys +import logging from slither import Slither from slither.utils.function import get_function_id from slither.utils.colors import red, green +logger = logging.getLogger("CompareFunctions") +logger.setLevel(logging.INFO) def get_signatures(s): functions = [contract.functions for contract in s.contracts_derived] @@ -19,13 +21,8 @@ def get_signatures(s): variables = [variable.name+ '()' for variable in variables if variable.visibility in ['public']] return list(set(functions+variables)) -if __name__ == "__main__": - if len(sys.argv) != 3: - print('Usage: python3 compare_function_ids.py implem.sol proxy.sol') - - implem = Slither(sys.argv[1]) - proxy = Slither(sys.argv[2]) +def compare_function_ids(implem, proxy): signatures_implem = get_signatures(implem) signatures_proxy = get_signatures(proxy) @@ -34,12 +31,12 @@ if __name__ == "__main__": signatures_ids_proxy = {get_function_id(s): s for s in signatures_proxy} found = False - for (k,v) in signatures_ids_implem.items(): + for (k, _) in signatures_ids_implem.items(): if k in signatures_ids_proxy: found = True - print(red('Collision found {} {}'.format(signatures_ids_implem[k], - signatures_ids_proxy[k]))) + logger.info(red('Collision found {} {}'.format(signatures_ids_implem[k], + signatures_ids_proxy[k]))) if not found: - print(green('No collision found')) + logger.info(green('No collision found')) diff --git a/utils/upgradability/compare_variables_order.py b/utils/upgradability/compare_variables_order.py index 64f4238c2..c26ac1213 100644 --- a/utils/upgradability/compare_variables_order.py +++ b/utils/upgradability/compare_variables_order.py @@ -1,29 +1,24 @@ ''' - This utility looks for functions collisions between a proxy and the implementation - More for information: https://medium.com/nomic-labs-blog/malicious-backdoors-in-ethereum-proxies-62629adf3357 + Check if the variables respect the same ordering ''' - -import sys +import logging from slither import Slither from slither.utils.function import get_function_id from slither.utils.colors import red, green -if __name__ == "__main__": - - if len(sys.argv) != 5: - print('Usage: python3 compare_variables_order.py v1.sol Contract1 v2.sol Contract2') +logger = logging.getLogger("VariablesOrder") +logger.setLevel(logging.INFO) - v1 = Slither(sys.argv[1]) - v2 = Slither(sys.argv[3]) +def compare_variables_order(v1, contract_name1, v2, contract_name2): - contract_v1 = v1.get_contract_from_name(sys.argv[2]) + contract_v1 = v1.get_contract_from_name(contract_name1) if contract_v1 is None: - print(red('Contract {} not found'.format(sys.argv[2]))) + logger.info(red('Contract {} not found'.format(contract_name1))) exit(-1) - contract_v2 = v2.get_contract_from_name(sys.argv[4]) + contract_v2 = v2.get_contract_from_name(contract_name2) if contract_v2 is None: - print(red('Contract {} not found'.format(sys.argv[4]))) + logger.info(red('Contract {} not found'.format(contract_name2))) exit(-1) @@ -35,22 +30,22 @@ if __name__ == "__main__": for idx in range(0, len(order_v1)): (v1_name, v1_type) = order_v1[idx] if len(order_v2) < idx: - print(red('Missing variable in the new version: {} {}'.format(v1_name, v1_type))) + logger.info(red('Missing variable in the new version: {} {}'.format(v1_name, v1_type))) continue (v2_name, v2_type) = order_v2[idx] if (v1_name != v2_name) or (v1_type != v2_type): found = True - print(red('Different variable: {} {} -> {} {}'.format(v1_name, - v1_type, - v2_name, + logger.info(red('Different variable: {} {} -> {} {}'.format(v1_name, + v1_type, + v2_name, v2_type))) if len(order_v2) > len(order_v1): new_variables = order_v2[len(order_v1):] for (name, t) in new_variables: - print(green('New variable: {} {}'.format(name, t))) + logger.info(green('New variable: {} {}'.format(name, t))) if not found: - print(green('No error found')) + logger.info(green('No error found')) From d7f5d4218fe06afc99550ea640acfe8c544f7f5e Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 29 Jan 2019 09:24:11 +0000 Subject: [PATCH 4/6] Improve upgradable cli --- utils/upgradability/__main__.py | 36 +++++++----- utils/upgradability/compare_function_ids.py | 6 +- .../upgradability/compare_variables_order.py | 57 +++++++++++++++++-- 3 files changed, 77 insertions(+), 22 deletions(-) diff --git a/utils/upgradability/__main__.py b/utils/upgradability/__main__.py index 8b3edfcbd..471e92a79 100644 --- a/utils/upgradability/__main__.py +++ b/utils/upgradability/__main__.py @@ -4,7 +4,7 @@ import sys from slither import Slither -from .compare_variables_order import compare_variables_order +from .compare_variables_order import compare_variables_order_implementation, compare_variables_order_proxy from .compare_function_ids import compare_function_ids logging.basicConfig() @@ -13,17 +13,17 @@ logger = logging.getLogger("Slither-check-upgradability") def parse_args(): parser = argparse.ArgumentParser(description='Slither Upgradability Checks', - usage="slither-check-upgradability proxy.sol ProxyName v1.sol V1Name v2.sol V2Name") + usage="slither-check-upgradability proxy.sol ProxyName implem.sol ContractName") parser.add_argument('proxy.sol', help='Proxy filename') parser.add_argument('ProxyName', help='Contract name') - parser.add_argument('v1.sol', help='Version 1 filename') - parser.add_argument('V1Name', help='Contract name') + parser.add_argument('implem.sol', help='Implementation filename') + parser.add_argument('ContractName', help='Contract name') - parser.add_argument('v2.sol', help='Version 2 filename') - parser.add_argument('V2Name', help='Contract name') + parser.add_argument('--new-version', help='New implementation filename') + parser.add_argument('--new-contract-name', help='New contract name (if changed)') if len(sys.argv) == 1: parser.print_help(sys.stderr) @@ -34,13 +34,23 @@ def parse_args(): def main(): args = parse_args() - proxy = Slither(vars(args)['proxy.sol']) proxy_name = args.ProxyName - v1 = Slither(vars(args)['v1.sol']) - v1_name = args.V1Name - v2 = Slither(vars(args)['v2.sol']) - v2_name = args.V2Name + v1 = Slither(vars(args)['implem.sol']) + v1_name = args.ContractName + + last_contract = v1 + last_name = v1_name + + if args.new_version: + v2 = Slither(args.new_version) + last_contract = v2 + + if args.new_contract_name: + last_name = args.new_contract_name + + compare_function_ids(last_contract, proxy) + compare_variables_order_proxy(last_contract, last_name, proxy, proxy_name) - compare_variables_order(v1, v1_name, v2, v2_name) - compare_function_ids(v2, proxy) + if args.new_version: + compare_variables_order_implementation(v1, v1_name, v2, last_name) diff --git a/utils/upgradability/compare_function_ids.py b/utils/upgradability/compare_function_ids.py index b95e7f110..aaee290d7 100644 --- a/utils/upgradability/compare_function_ids.py +++ b/utils/upgradability/compare_function_ids.py @@ -34,9 +34,9 @@ def compare_function_ids(implem, proxy): for (k, _) in signatures_ids_implem.items(): if k in signatures_ids_proxy: found = True - logger.info(red('Collision found {} {}'.format(signatures_ids_implem[k], - signatures_ids_proxy[k]))) + logger.info(red('Function id collision found {} {}'.format(signatures_ids_implem[k], + signatures_ids_proxy[k]))) if not found: - logger.info(green('No collision found')) + logger.info(green('No function id collision found')) diff --git a/utils/upgradability/compare_variables_order.py b/utils/upgradability/compare_variables_order.py index c26ac1213..4a658c07f 100644 --- a/utils/upgradability/compare_variables_order.py +++ b/utils/upgradability/compare_variables_order.py @@ -4,21 +4,21 @@ import logging from slither import Slither from slither.utils.function import get_function_id -from slither.utils.colors import red, green +from slither.utils.colors import red, green, yellow logger = logging.getLogger("VariablesOrder") logger.setLevel(logging.INFO) -def compare_variables_order(v1, contract_name1, v2, contract_name2): +def compare_variables_order_implementation(v1, contract_name1, v2, contract_name2): contract_v1 = v1.get_contract_from_name(contract_name1) if contract_v1 is None: - logger.info(red('Contract {} not found'.format(contract_name1))) + logger.info(red('Contract {} not found in {}'.format(contract_name1, v1.filename))) exit(-1) contract_v2 = v2.get_contract_from_name(contract_name2) if contract_v2 is None: - logger.info(red('Contract {} not found'.format(contract_name2))) + logger.info(red('Contract {} not found in {}'.format(contract_name2, v2.filename))) exit(-1) @@ -39,7 +39,7 @@ def compare_variables_order(v1, contract_name1, v2, contract_name2): logger.info(red('Different variable: {} {} -> {} {}'.format(v1_name, v1_type, v2_name, - v2_type))) + v2_type))) if len(order_v2) > len(order_v1): new_variables = order_v2[len(order_v1):] @@ -47,5 +47,50 @@ def compare_variables_order(v1, contract_name1, v2, contract_name2): logger.info(green('New variable: {} {}'.format(name, t))) if not found: - logger.info(green('No error found')) + logger.info(green('No error found (variables ordering between implementations)')) + +def compare_variables_order_proxy(implem, implem_name, proxy, proxy_name): + + contract_implem = implem.get_contract_from_name(implem_name) + if contract_implem is None: + logger.info(red('Contract {} not found in {}'.format(implem_name, implem.filename))) + exit(-1) + + contract_proxy = proxy.get_contract_from_name(proxy_name) + if contract_proxy is None: + logger.info(red('Contract {} not found in {}'.format(proxy_name, proxy.filename))) + exit(-1) + + + order_implem = [(variable.name, variable.type) for variable in contract_implem.state_variables if not variable.is_constant] + order_proxy = [(variable.name, variable.type) for variable in contract_proxy.state_variables if not variable.is_constant] + + + found = False + for idx in range(0, len(order_proxy)): + (proxy_name, proxy_type) = order_proxy[idx] + if len(order_proxy) < idx: + logger.info(red('Extra variable in the proxy: {} {}'.format(proxy_name, proxy_type))) + continue + (implem_name, implem_type) = order_implem[idx] + + if (proxy_name != implem_name) or (proxy_type != implem_type): + found = True + logger.info(red('Different variable: {} {} -> {} {}'.format(proxy_name, + proxy_type, + implem_name, + implem_type))) + else: + logger.info(yellow('Variable in the proxy: {} {}'.format(proxy_name, + proxy_type))) + + + #if len(order_implem) > len(order_proxy): + # new_variables = order_implem[len(order_proxy):] + # for (name, t) in new_variables: + # logger.info(green('Variable only in implem: {} {}'.format(name, t))) + + if not found: + logger.info(green('No error found (variables ordering proxy <-> implementation)')) + From 9baf839da594e24e480e32fb295bc18ee5e8eef4 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 5 Feb 2019 07:24:05 -0500 Subject: [PATCH 5/6] slither-check-upgradability: - Add unit test - Improve output - Add --solc cli option --- .travis.yml | 1 + scripts/travis_install.sh | 2 + scripts/travis_test_upgradability.sh | 50 +++++++++++++++++++ slither/solc_parsing/slitherSolc.py | 4 ++ tests/check-upgradability/contractV1.sol | 3 ++ tests/check-upgradability/contractV2.sol | 3 ++ tests/check-upgradability/contractV2_bug.sol | 5 ++ tests/check-upgradability/contractV2_bug2.sol | 6 +++ tests/check-upgradability/proxy.sol | 27 ++++++++++ tests/check-upgradability/test_1.txt | 3 ++ tests/check-upgradability/test_2.txt | 4 ++ tests/check-upgradability/test_3.txt | 4 ++ tests/check-upgradability/test_4.txt | 4 ++ utils/upgradability/__main__.py | 7 +-- .../upgradability/compare_variables_order.py | 4 +- 15 files changed, 122 insertions(+), 5 deletions(-) create mode 100755 scripts/travis_test_upgradability.sh create mode 100644 tests/check-upgradability/contractV1.sol create mode 100644 tests/check-upgradability/contractV2.sol create mode 100644 tests/check-upgradability/contractV2_bug.sol create mode 100644 tests/check-upgradability/contractV2_bug2.sol create mode 100644 tests/check-upgradability/proxy.sol create mode 100644 tests/check-upgradability/test_1.txt create mode 100644 tests/check-upgradability/test_2.txt create mode 100644 tests/check-upgradability/test_3.txt create mode 100644 tests/check-upgradability/test_4.txt diff --git a/.travis.yml b/.travis.yml index 8373f6b78..4b87def6b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,5 +16,6 @@ install: script: - scripts/travis_test_4.sh - scripts/travis_test_5.sh + - scripts/travis_test_upgradability.sh diff --git a/scripts/travis_install.sh b/scripts/travis_install.sh index 124527beb..ae3287f2e 100755 --- a/scripts/travis_install.sh +++ b/scripts/travis_install.sh @@ -8,6 +8,8 @@ function install_solc { sudo chmod +x /usr/bin/solc-0.4.25 sudo wget -O /usr/bin/solc-0.5.1 https://github.com/ethereum/solidity/releases/download/v0.5.1/solc-static-linux sudo chmod +x /usr/bin/solc-0.5.1 + sudo wget -O /usr/bin/solc-0.5.0 https://github.com/ethereum/solidity/releases/download/v0.5.0/solc-static-linux + sudo chmod +x /usr/bin/solc-0.5.0 sudo cp /usr/bin/solc-0.5.1 /usr/bin/solc } diff --git a/scripts/travis_test_upgradability.sh b/scripts/travis_test_upgradability.sh new file mode 100755 index 000000000..92bad4490 --- /dev/null +++ b/scripts/travis_test_upgradability.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash + +### Test slither-check-upgradability + +DIR_TESTS="tests/check-upgradability" + +slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 > test_1.txt 2>&1 +DIFF=$(diff test_1.txt "$DIR_TESTS/test_1.txt") +if [ "$DIFF" != "" ] +then + echo "slither-check-upgradability failed" + cat test_1.txt + cat "$DIR_TESTS/test_1.txt" + exit -1 +fi + +slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2.sol" --new-contract-name ContractV2 > test_2.txt 2>&1 +DIFF=$(diff test_2.txt "$DIR_TESTS/test_2.txt") +if [ "$DIFF" != "" ] +then + echo "slither-check-upgradability failed" + cat test_2.txt + cat "$DIR_TESTS/test_2.txt" + exit -1 +fi + +slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug.sol" --new-contract-name ContractV2 > test_3.txt 2>&1 +DIFF=$(diff test_3.txt "$DIR_TESTS/test_3.txt") +if [ "$DIFF" != "" ] +then + echo "slither-check-upgradability failed" + cat test_3.txt + cat "$DIR_TESTS/test_3.txt" + exit -1 +fi + +slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug2.sol" --new-contract-name ContractV2 > test_4.txt 2>&1 +DIFF=$(diff test_4.txt "$DIR_TESTS/test_4.txt") +if [ "$DIFF" != "" ] +then + echo "slither-check-upgradability failed" + cat test_4.txt + cat "$DIR_TESTS/test_4.txt" + exit -1 +fi + +rm test_1.txt +rm test_2.txt +rm test_3.txt +rm test_4.txt diff --git a/slither/solc_parsing/slitherSolc.py b/slither/solc_parsing/slitherSolc.py index ee51e9f41..165e1af6a 100644 --- a/slither/solc_parsing/slitherSolc.py +++ b/slither/solc_parsing/slitherSolc.py @@ -3,7 +3,9 @@ import json import re import logging +logging.basicConfig() logger = logging.getLogger("SlitherSolcParsing") +logger.setLevel(logging.INFO) from slither.solc_parsing.declarations.contract import ContractSolc04 from slither.core.slither_core import Slither @@ -138,6 +140,8 @@ class SlitherSolc(Slither): def _analyze_contracts(self): + if not self._contractsNotParsed: + logger.info(f'No contract were found in {self.filename}, check the correct compilation') if self._analyzed: raise Exception('Contract analysis can be run only once!') diff --git a/tests/check-upgradability/contractV1.sol b/tests/check-upgradability/contractV1.sol new file mode 100644 index 000000000..29c1592c8 --- /dev/null +++ b/tests/check-upgradability/contractV1.sol @@ -0,0 +1,3 @@ +contract ContractV1{ + address destination; +} diff --git a/tests/check-upgradability/contractV2.sol b/tests/check-upgradability/contractV2.sol new file mode 100644 index 000000000..607ddafe7 --- /dev/null +++ b/tests/check-upgradability/contractV2.sol @@ -0,0 +1,3 @@ +contract ContractV2{ + address destination; +} diff --git a/tests/check-upgradability/contractV2_bug.sol b/tests/check-upgradability/contractV2_bug.sol new file mode 100644 index 000000000..75efb323a --- /dev/null +++ b/tests/check-upgradability/contractV2_bug.sol @@ -0,0 +1,5 @@ +contract ContractV2{ + uint destination; + + uint public myFunc; +} diff --git a/tests/check-upgradability/contractV2_bug2.sol b/tests/check-upgradability/contractV2_bug2.sol new file mode 100644 index 000000000..0b2b9f128 --- /dev/null +++ b/tests/check-upgradability/contractV2_bug2.sol @@ -0,0 +1,6 @@ +contract Base { + uint val; +} +contract ContractV2 is Base{ + address destination; +} diff --git a/tests/check-upgradability/proxy.sol b/tests/check-upgradability/proxy.sol new file mode 100644 index 000000000..9bc1d1d73 --- /dev/null +++ b/tests/check-upgradability/proxy.sol @@ -0,0 +1,27 @@ +/* + Fake proxy, do not use +*/ + +pragma solidity ^0.5.0; + +contract Proxy{ + + address destination; + + function myFunc() public{} + + function () external{ + uint size_destination_code; + assembly{ + size_destination_code := extcodesize(destination_slot) + } + require(size_destination_code>0); + (bool ret_status,bytes memory ret_values) = destination.delegatecall(msg.data); + require(ret_status); + uint length = ret_values.length; + assembly{ + return (ret_values, length) + } + } + +} diff --git a/tests/check-upgradability/test_1.txt b/tests/check-upgradability/test_1.txt new file mode 100644 index 000000000..717d2cb33 --- /dev/null +++ b/tests/check-upgradability/test_1.txt @@ -0,0 +1,3 @@ +INFO:CompareFunctions:No function id collision found +INFO:VariablesOrder:Variable in the proxy: destination address +INFO:VariablesOrder:No error found (variables ordering proxy <-> implementation) diff --git a/tests/check-upgradability/test_2.txt b/tests/check-upgradability/test_2.txt new file mode 100644 index 000000000..7c95cd855 --- /dev/null +++ b/tests/check-upgradability/test_2.txt @@ -0,0 +1,4 @@ +INFO:CompareFunctions:No function id collision found +INFO:VariablesOrder:Variable in the proxy: destination address +INFO:VariablesOrder:No error found (variables ordering proxy <-> implementation) +INFO:VariablesOrder:No error found (variables ordering between implementations) diff --git a/tests/check-upgradability/test_3.txt b/tests/check-upgradability/test_3.txt new file mode 100644 index 000000000..4b0a0d3fb --- /dev/null +++ b/tests/check-upgradability/test_3.txt @@ -0,0 +1,4 @@ +INFO:CompareFunctions:Function id collision found myFunc() myFunc() +INFO:VariablesOrder:Different variables between proxy and implem: destination address -> destination uint256 +INFO:VariablesOrder:Different variables between v1 and v2: destination address -> destination uint256 +INFO:VariablesOrder:New variable: myFunc uint256 diff --git a/tests/check-upgradability/test_4.txt b/tests/check-upgradability/test_4.txt new file mode 100644 index 000000000..0367a6cc4 --- /dev/null +++ b/tests/check-upgradability/test_4.txt @@ -0,0 +1,4 @@ +INFO:CompareFunctions:No function id collision found +INFO:VariablesOrder:Different variables between proxy and implem: destination address -> val uint256 +INFO:VariablesOrder:Different variables between v1 and v2: destination address -> val uint256 +INFO:VariablesOrder:New variable: destination address diff --git a/utils/upgradability/__main__.py b/utils/upgradability/__main__.py index 471e92a79..d98fe1a26 100644 --- a/utils/upgradability/__main__.py +++ b/utils/upgradability/__main__.py @@ -24,6 +24,7 @@ def parse_args(): parser.add_argument('--new-version', help='New implementation filename') parser.add_argument('--new-contract-name', help='New contract name (if changed)') + parser.add_argument('--solc', help='solc path', default='solc') if len(sys.argv) == 1: parser.print_help(sys.stderr) @@ -34,16 +35,16 @@ def parse_args(): def main(): args = parse_args() - proxy = Slither(vars(args)['proxy.sol']) + proxy = Slither(vars(args)['proxy.sol'], solc=args.solc) proxy_name = args.ProxyName - v1 = Slither(vars(args)['implem.sol']) + v1 = Slither(vars(args)['implem.sol'], solc=args.solc) v1_name = args.ContractName last_contract = v1 last_name = v1_name if args.new_version: - v2 = Slither(args.new_version) + v2 = Slither(args.new_version, solc=args.solc) last_contract = v2 if args.new_contract_name: diff --git a/utils/upgradability/compare_variables_order.py b/utils/upgradability/compare_variables_order.py index 4a658c07f..725e31ae1 100644 --- a/utils/upgradability/compare_variables_order.py +++ b/utils/upgradability/compare_variables_order.py @@ -36,7 +36,7 @@ def compare_variables_order_implementation(v1, contract_name1, v2, contract_name if (v1_name != v2_name) or (v1_type != v2_type): found = True - logger.info(red('Different variable: {} {} -> {} {}'.format(v1_name, + logger.info(red('Different variables between v1 and v2: {} {} -> {} {}'.format(v1_name, v1_type, v2_name, v2_type))) @@ -76,7 +76,7 @@ def compare_variables_order_proxy(implem, implem_name, proxy, proxy_name): if (proxy_name != implem_name) or (proxy_type != implem_type): found = True - logger.info(red('Different variable: {} {} -> {} {}'.format(proxy_name, + logger.info(red('Different variables between proxy and implem: {} {} -> {} {}'.format(proxy_name, proxy_type, implem_name, implem_type))) From e6e23173c784626376407daf4ffc6ee57dff6db7 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 5 Feb 2019 07:32:10 -0500 Subject: [PATCH 6/6] Add missing __init__.py file --- utils/__init__.py | 0 utils/upgradability/__init__.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 utils/__init__.py create mode 100644 utils/upgradability/__init__.py diff --git a/utils/__init__.py b/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/utils/upgradability/__init__.py b/utils/upgradability/__init__.py new file mode 100644 index 000000000..e69de29bb