From cc2ff82711caa21bbf6be1f304b740aa90bcbac2 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 5 May 2020 11:22:37 +0200 Subject: [PATCH 1/2] - APIs add: - Add utils.tests_pattern functions - Add contract.is_test - Add contract.is_token / contract.is_possible_token - Add pragma.is_abi_encoder_v2 - Standard library: add support for @openzepeelin/contracts package - Improve human summary printer: - Use the added API - Use MyPrettyTable - Add features collumn --- slither/core/declarations/contract.py | 44 ++++++ slither/core/declarations/pragma_directive.py | 6 + slither/printers/summary/human_summary.py | 148 +++++++++++++----- slither/utils/standard_libraries.py | 10 +- slither/utils/tests_pattern.py | 46 ++++++ 5 files changed, 213 insertions(+), 41 deletions(-) create mode 100644 slither/utils/tests_pattern.py diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index e9a430289..a6c780c88 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -2,6 +2,9 @@ Contract module """ import logging +from pathlib import Path + +from crytic_compile.platform import Type as PlatformType from slither.core.children.child_slither import ChildSlither from slither.core.source_mapping.source_mapping import SourceMapping @@ -9,6 +12,7 @@ from slither.core.declarations.function import Function from slither.utils.erc import ERC20_signatures, \ ERC165_signatures, ERC223_signatures, ERC721_signatures, \ ERC1820_signatures, ERC777_signatures +from slither.utils.tests_pattern import is_test_contract logger = logging.getLogger("Contract") @@ -781,6 +785,14 @@ class Contract(ChildSlither, SourceMapping): full_names = self.functions_signatures return all((s in full_names for s in ERC777_signatures)) + @property + def is_token(self) -> bool: + """ + Check if the contract follows one of the standard ERC token + :return: + """ + return self.is_erc20() or self.is_erc721() or self.is_erc165() or self.is_erc223() or self.is_erc777() + def is_possible_erc20(self): """ Checks if the provided contract could be attempting to implement ERC20 standards. @@ -808,6 +820,13 @@ class Contract(ChildSlither, SourceMapping): 'getApproved(uint256)' in full_names or 'isApprovedForAll(address,address)' in full_names) + @property + def is_possible_token(self) -> bool: + """ + Check if the contract is a potential token (it might not implement all the functions) + :return: + """ + return self.is_possible_erc20() or self.is_possible_erc721() # endregion ################################################################################### @@ -821,6 +840,31 @@ class Contract(ChildSlither, SourceMapping): return False return self.slither.crytic_compile.is_dependency(self.source_mapping['filename_absolute']) + # endregion + ################################################################################### + ################################################################################### + # region Test + ################################################################################### + ################################################################################### + + @property + def is_truffle_migration(self) -> bool: + """ + Return true if the contract is the Migrations contract needed for Truffle + :return: + """ + if self.slither.crytic_compile: + if self.slither.crytic_compile.platform == PlatformType.TRUFFLE: + if self.name == 'Migrations': + paths = Path(self.source_mapping['filename_absolute']).parts + if len(paths) >= 2: + return paths[-2] == 'contracts' and paths[-1] == 'migrations.sol' + return False + + @property + def is_test(self) -> bool: + return is_test_contract(self) or self.is_truffle_migration + # endregion ################################################################################### ################################################################################### diff --git a/slither/core/declarations/pragma_directive.py b/slither/core/declarations/pragma_directive.py index 4ebe216f8..d8ef80b7d 100644 --- a/slither/core/declarations/pragma_directive.py +++ b/slither/core/declarations/pragma_directive.py @@ -27,5 +27,11 @@ class Pragma(SourceMapping): return self._directive[0].lower() == 'solidity' return False + @property + def is_abi_encoder_v2(self): + if len(self._directive) == 2: + return self._directive[0] == 'experimental' and self._directive[1] == 'ABIEncoderV2' + return False + def __str__(self): return 'pragma '+''.join(self.directive) diff --git a/slither/printers/summary/human_summary.py b/slither/printers/summary/human_summary.py index aa5961f40..37c3475e6 100644 --- a/slither/printers/summary/human_summary.py +++ b/slither/printers/summary/human_summary.py @@ -2,14 +2,20 @@ Module printing summary of the contract """ import logging +from pathlib import Path from typing import Tuple, List, Dict +from slither.core.declarations import SolidityFunction, Function +from slither.core.variables.state_variable import StateVariable from slither.printers.abstract_printer import AbstractPrinter +from slither.slithir.operations import LowLevelCall, HighLevelCall, Transfer, Send, SolidityCall from slither.utils import output from slither.utils.code_complexity import compute_cyclomatic_complexity from slither.utils.colors import green, red, yellow +from slither.utils.myprettytable import MyPrettyTable from slither.utils.standard_libraries import is_standard_library from slither.core.cfg.node import NodeType +from slither.utils.tests_pattern import is_test_file class PrinterHumanSummary(AbstractPrinter): @@ -27,40 +33,36 @@ class PrinterHumanSummary(AbstractPrinter): pause = 'pause' in functions_name if 'mint' in functions_name: - if not 'mintingFinished' in state_variables: - mint_limited = False + if 'mintingFinished' in state_variables: + mint_unlimited = False else: - mint_limited = True + mint_unlimited = True else: - mint_limited = None # no minting + mint_unlimited = None # no minting race_condition_mitigated = 'increaseApproval' in functions_name or \ 'safeIncreaseAllowance' in functions_name - return pause, mint_limited, race_condition_mitigated + return pause, mint_unlimited, race_condition_mitigated def get_summary_erc20(self, contract): txt = '' - pause, mint_limited, race_condition_mitigated = self._get_summary_erc20(contract) + pause, mint_unlimited, race_condition_mitigated = self._get_summary_erc20(contract) if pause: - txt += "\t\t Can be paused? : {}\n".format(yellow('Yes')) - else: - txt += "\t\t Can be paused? : {}\n".format(green('No')) + txt += yellow("Pausable") + "\n" - if mint_limited is None: - txt += "\t\t Minting restriction? : {}\n".format(green('No Minting')) + if mint_unlimited is None: + txt += green("No Minting") + "\n" else: - if mint_limited: - txt += "\t\t Minting restriction? : {}\n".format(red('Yes')) + if mint_unlimited: + txt += red("∞ Minting") + "\n" else: - txt += "\t\t Minting restriction? : {}\n".format(yellow('No')) + txt += yellow("Minting") + "\n" - if race_condition_mitigated: - txt += "\t\t ERC20 race condition mitigation: {}\n".format(green('Yes')) - else: - txt += "\t\t ERC20 race condition mitigation: {}\n".format(red('No')) + if not race_condition_mitigated: + txt += red("Approve Race Cond.") + "\n" return txt @@ -139,8 +141,7 @@ class PrinterHumanSummary(AbstractPrinter): is_complex = self._is_complex_code(contract) result = red('Yes') if is_complex else green('No') - - return "\tComplex code? {}\n".format(result) + return result @staticmethod def _number_functions(contract): @@ -151,6 +152,8 @@ class PrinterHumanSummary(AbstractPrinter): return None total_dep_lines = 0 total_lines = 0 + total_tests_lines = 0 + for filename, source_code in self.slither.source_code.items(): lines = len(source_code.splitlines()) is_dep = False @@ -159,8 +162,11 @@ class PrinterHumanSummary(AbstractPrinter): if is_dep: total_dep_lines += lines else: - total_lines += lines - return total_lines, total_dep_lines + if is_test_file(Path(filename)): + total_tests_lines += lines + else: + total_lines += lines + return total_lines, total_dep_lines, total_tests_lines def _get_number_of_assembly_lines(self): total_asm_lines = 0 @@ -176,14 +182,14 @@ class PrinterHumanSummary(AbstractPrinter): def _compilation_type(self): if self.slither.crytic_compile is None: return 'Compilation non standard\n' - return f'Compiled with {self.slither.crytic_compile.type}\n' + return f'Compiled with {str(self.slither.crytic_compile.type)}\n' def _number_contracts(self): if self.slither.crytic_compile is None: len(self.slither.contracts), 0 deps = [c for c in self.slither.contracts if c.is_from_dependency()] - contracts = [c for c in self.slither.contracts if not c.is_from_dependency()] - return len(contracts), len(deps) + tests = [c for c in self.slither.contracts if c.is_test] + return len(self.slither.contracts) - len(deps) - len(tests), len(deps), len(tests) def _standard_libraries(self): libraries = [] @@ -200,6 +206,59 @@ class PrinterHumanSummary(AbstractPrinter): ercs += contract.ercs() return list(set(ercs)) + def _get_features(self, contract): + + has_payable = False + can_send_eth = False + can_selfdestruct = False + has_ecrecover = False + can_delegatecall = False + has_token_interaction = False + + has_assembly = False + + use_abi_encoder = False + + for pragma in self.slither.pragma_directives: + if pragma.source_mapping["filename_absolute"] == contract.source_mapping["filename_absolute"]: + if pragma.is_abi_encoder_v2: + use_abi_encoder = True + + for function in contract.functions: + if function.payable: + has_payable = True + + if function.contains_assembly: + has_assembly = True + + for ir in function.slithir_operations: + if isinstance(ir, (LowLevelCall, HighLevelCall, Send, Transfer)) and ir.call_value: + can_send_eth = True + if isinstance(ir, SolidityCall) and ir.function in [SolidityFunction("suicide(address)"), + SolidityFunction("selfdestruct(address)")]: + can_selfdestruct = True + if (isinstance(ir, SolidityCall) and + ir.function == SolidityFunction("ecrecover(bytes32,uint8,bytes32,bytes32)")): + has_ecrecover = True + if isinstance(ir, LowLevelCall) and ir.function_name in ["delegatecall", "callcode"]: + can_delegatecall = True + if isinstance(ir, HighLevelCall): + if isinstance(ir.function, (Function, StateVariable)) and ir.function.contract.is_possible_token: + has_token_interaction = True + + return { + "Receive ETH": has_payable, + "Send ETH": can_send_eth, + "Selfdestruct": can_selfdestruct, + "Ecrecover": has_ecrecover, + "Delegatecall": can_delegatecall, + "Tokens interaction": has_token_interaction, + "AbiEncoderV2": use_abi_encoder, + "Assembly": has_assembly, + "Upgradeable": contract.is_upgradeable, + "Proxy": contract.is_upgradeable_proxy, + } + def output(self, _filename): """ _filename is not used @@ -225,16 +284,16 @@ class PrinterHumanSummary(AbstractPrinter): lines_number = self._lines_number() if lines_number: - total_lines, total_dep_lines = lines_number - txt += f'Number of lines: {total_lines} (+ {total_dep_lines} in dependencies)\n' + total_lines, total_dep_lines, total_tests_lines = lines_number + txt += f'Number of lines: {total_lines} (+ {total_dep_lines} in dependencies, + {total_tests_lines} in tests)\n' results['number_lines'] = total_lines results['number_lines__dependencies'] = total_dep_lines total_asm_lines = self._get_number_of_assembly_lines() txt += f"Number of assembly lines: {total_asm_lines}\n" results['number_lines_assembly'] = total_asm_lines - number_contracts, number_contracts_deps = self._number_contracts() - txt += f'Number of contracts: {number_contracts} (+ {number_contracts_deps} in dependencies) \n\n' + number_contracts, number_contracts_deps, number_contracts_tests = self._number_contracts() + txt += f'Number of contracts: {number_contracts} (+ {number_contracts_deps} in dependencies, + {number_contracts_tests} tests) \n\n' txt_detectors, detectors_results, optimization, info, low, medium, high = self.get_detectors_result() txt += txt_detectors @@ -258,26 +317,36 @@ class PrinterHumanSummary(AbstractPrinter): txt += f'ERCs: {", ".join(ercs)}\n' results['ercs'] = [str(e) for e in ercs] + table = MyPrettyTable(["Name", "# functions", "ERCS", "ERC20 info", "Complex code", "Features"]) for contract in self.slither.contracts_derived: - txt += "\nContract {}\n".format(contract.name) - txt += self.is_complex_code(contract) - txt += '\tNumber of functions: {}\n'.format(self._number_functions(contract)) - ercs = contract.ercs() - if ercs: - txt += '\tERCs: ' + ','.join(ercs) + '\n' + + if contract.is_from_dependency() or contract.is_test: + continue + + is_complex = self.is_complex_code(contract) + number_functions = self._number_functions(contract) + ercs = ','.join(contract.ercs()) is_erc20 = contract.is_erc20() + erc20_info = '' if is_erc20: - txt += '\tERC20 info:\n' - txt += self.get_summary_erc20(contract) + erc20_info += self.get_summary_erc20(contract) + + features = "\n".join([name for name, to_print in self._get_features(contract).items() if to_print]) - self.info(txt) + table.add_row([contract.name, number_functions, ercs, erc20_info, is_complex, features]) + + self.info(txt + '\n' + str(table)) results_contract = output.Output('') for contract in self.slither.contracts_derived: + if contract.is_test or contract.is_from_dependency(): + continue + contract_d = {'contract_name': contract.name, 'is_complex_code': self._is_complex_code(contract), 'is_erc20': contract.is_erc20(), - 'number_functions': self._number_functions(contract)} + 'number_functions': self._number_functions(contract), + 'features': [name for name, to_print in self._get_features(contract).items() if to_print]} if contract_d['is_erc20']: pause, mint_limited, race_condition_mitigated = self._get_summary_erc20(contract) contract_d['erc20_pause'] = pause @@ -287,7 +356,6 @@ class PrinterHumanSummary(AbstractPrinter): else: contract_d['erc20_can_mint'] = False contract_d['erc20_race_condition_mitigated'] = race_condition_mitigated - results_contract.add_contract(contract, additional_fields=contract_d) results['contracts']['elements'] = results_contract.elements diff --git a/slither/utils/standard_libraries.py b/slither/utils/standard_libraries.py index e138e6c35..961e10047 100644 --- a/slither/utils/standard_libraries.py +++ b/slither/utils/standard_libraries.py @@ -50,7 +50,15 @@ def is_standard_library(contract): def is_openzepellin(contract): if not contract.is_from_dependency(): return False - return 'openzeppelin-solidity' in Path(contract.source_mapping['filename_absolute']).parts + path = Path(contract.source_mapping['filename_absolute']).parts + is_zep = 'openzeppelin-solidity' in Path(contract.source_mapping['filename_absolute']).parts + try: + is_zep |= path[path.index('@openzeppelin') + 1] == 'contracts' + except IndexError: + pass + except ValueError: + pass + return is_zep def is_zos(contract): diff --git a/slither/utils/tests_pattern.py b/slither/utils/tests_pattern.py new file mode 100644 index 000000000..fb36934c6 --- /dev/null +++ b/slither/utils/tests_pattern.py @@ -0,0 +1,46 @@ +from pathlib import Path +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from slither.core.declarations.contract import Contract + +_TESTS_PATTERNS = ["Test", "test", "Mock", "mock"] +TESTS_PATTERNS = _TESTS_PATTERNS + [x + "s" for x in _TESTS_PATTERNS] + + +def _is_test_pattern(txt: str, pattern: str) -> bool: + """ + Check if the txt starts with the pattern, or ends with it + :param pattern: + :return: + """ + if txt.endswith(pattern): + return True + if not txt.startswith(pattern): + return False + length = len(pattern) + if len(txt) <= length: + return True + return txt[length] == "_" or txt[length].isupper() + + +def is_test_file(path: Path) -> bool: + """ + Check if the given path points to a test/mock file + :param path: + :return: + """ + return any((test_pattern in path.parts for test_pattern in TESTS_PATTERNS)) + + +def is_test_contract(contract: "Contract") -> bool: + """ + Check if the contract is a test/mock + :param contract: + :return: + """ + return ( + _is_test_pattern(contract.name, "Test") + or _is_test_pattern(contract.name, "Mock") + or is_test_file(Path(contract.source_mapping["filename_absolute"])) + ) From 87c307d792819a0e87d931f278897aa268e937e8 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 5 May 2020 13:15:22 +0200 Subject: [PATCH 2/2] is_test_contract: fix bug if crytic-compile is not used --- slither/utils/tests_pattern.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/slither/utils/tests_pattern.py b/slither/utils/tests_pattern.py index fb36934c6..52b2cc105 100644 --- a/slither/utils/tests_pattern.py +++ b/slither/utils/tests_pattern.py @@ -42,5 +42,8 @@ def is_test_contract(contract: "Contract") -> bool: return ( _is_test_pattern(contract.name, "Test") or _is_test_pattern(contract.name, "Mock") - or is_test_file(Path(contract.source_mapping["filename_absolute"])) + or ( + contract.source_mapping["filename_absolute"] + and is_test_file(Path(contract.source_mapping["filename_absolute"])) + ) )