From 8d2a2ce09c33f75b3262184e11ce3c82e6d6fb60 Mon Sep 17 00:00:00 2001 From: David Pokora Date: Fri, 26 Apr 2019 16:43:17 -0400 Subject: [PATCH 1/8] Fix false positives as a result of ERC721 interfaces appearing similar to ERC20. --- slither/core/declarations/contract.py | 2 +- slither/detectors/erc20/incorrect_interface.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 787172843..8a3fcbca9 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -535,7 +535,7 @@ class Contract(ChildSlither, SourceMapping): Returns: bool """ - full_names = [f.full_name for f in self.functions] + full_names = set([f.full_name for f in self.functions]) return 'transfer(address,uint256)' in full_names and\ 'transferFrom(address,address,uint256)' in full_names and\ 'approve(address,uint256)' in full_names diff --git a/slither/detectors/erc20/incorrect_interface.py b/slither/detectors/erc20/incorrect_interface.py index 94dc6f15b..ec7d308e2 100644 --- a/slither/detectors/erc20/incorrect_interface.py +++ b/slither/detectors/erc20/incorrect_interface.py @@ -52,6 +52,19 @@ contract Token{ Returns: list(str) : list of incorrect function signatures """ + # Obtain all function names for this contract + full_names = set([f.full_name for f in contract.functions]) + + # If this contract implements a function from ERC721, we can assume it is an ERC721 token. These tokens + # offer functions which are similar to ERC20, but are not compatible. + if ('ownerOf(uint256)' in full_names or + 'safeTransferFrom(address,address,uint256,bytes)' in full_names or + 'safeTransferFrom(address,address,uint256)' in full_names or + 'setApprovalForAll(address,bool)' in full_names or + 'getApproved(uint256)' in full_names or + 'isApprovedForAll(address,address)' in full_names): + return [] + functions = [f for f in contract.functions if f.contract == contract and \ IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] return functions From 7238e9b0337aa002b348d2837a6a2eb93d0be74b Mon Sep 17 00:00:00 2001 From: David Pokora Date: Fri, 26 Apr 2019 20:27:03 -0400 Subject: [PATCH 2/8] -Relaxed erc20-interface detector to report incorrect function signatures even if the function was not declared in that contract immediately -Created erc721-interface detector, similar to erc20-interface. --- slither/detectors/all_detectors.py | 5 +- slither/detectors/{erc20 => erc}/__init__.py | 0 .../incorrect_erc20_interface.py} | 44 +++++-- .../erc/incorrect_erc721_interface.py | 113 ++++++++++++++++++ .../unindexed_event_parameters.py | 0 tests/incorrect_erc20_interface.sol | 7 +- tests/incorrect_erc721_interface.sol | 16 +++ 7 files changed, 168 insertions(+), 17 deletions(-) rename slither/detectors/{erc20 => erc}/__init__.py (100%) rename slither/detectors/{erc20/incorrect_interface.py => erc/incorrect_erc20_interface.py} (62%) create mode 100644 slither/detectors/erc/incorrect_erc721_interface.py rename slither/detectors/{erc20 => erc}/unindexed_event_parameters.py (100%) create mode 100644 tests/incorrect_erc721_interface.sol diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 36c35496c..f0382516d 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -28,8 +28,9 @@ from .shadowing.builtin_symbols import BuiltinSymbolShadowing from .operations.block_timestamp import Timestamp from .statements.calls_in_loop import MultipleCallsInLoop from .statements.incorrect_strict_equality import IncorrectStrictEquality -from .erc20.incorrect_interface import IncorrectERC20InterfaceDetection -from .erc20.unindexed_event_parameters import UnindexedERC20EventParameters +from .erc.incorrect_erc20_interface import IncorrectERC20InterfaceDetection +from .erc.incorrect_erc721_interface import IncorrectERC721InterfaceDetection +from .erc.unindexed_event_parameters import UnindexedERC20EventParameters from .statements.deprecated_calls import DeprecatedStandards from .source.rtlo import RightToLeftOverride # diff --git a/slither/detectors/erc20/__init__.py b/slither/detectors/erc/__init__.py similarity index 100% rename from slither/detectors/erc20/__init__.py rename to slither/detectors/erc/__init__.py diff --git a/slither/detectors/erc20/incorrect_interface.py b/slither/detectors/erc/incorrect_erc20_interface.py similarity index 62% rename from slither/detectors/erc20/incorrect_interface.py rename to slither/detectors/erc/incorrect_erc20_interface.py index ec7d308e2..2dd4ad3e0 100644 --- a/slither/detectors/erc20/incorrect_interface.py +++ b/slither/detectors/erc/incorrect_erc20_interface.py @@ -5,6 +5,18 @@ Some contracts do not return a bool on transfer/transferFrom/approve, which may from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +def is_possible_erc20(contract): + """ + Checks if the provided contract could be attempting to implement ERC20 standards. + :param contract: The contract to check for token compatibility. + :return: Returns a boolean indicating if the provided contract met the token standard. + """ + full_names = set([f.full_name for f in contract.functions]) + return 'transfer(address,uint256)' in full_names or \ + 'transferFrom(address,address,uint256)' in full_names or \ + 'approve(address,uint256)' in full_names + + class IncorrectERC20InterfaceDetection(AbstractDetector): """ Incorrect ERC20 Interface @@ -18,7 +30,7 @@ class IncorrectERC20InterfaceDetection(AbstractDetector): WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc20-interface' WIKI_TITLE = 'Incorrect erc20 interface' - WIKI_DESCRIPTION = 'Lack of return value for the ERC20 `approve`/`transfer`/`transferFrom` functions. A contract compiled with solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing.' + WIKI_DESCRIPTION = 'Incorrect return values for ERC20 functions. A contract compiled with solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing.' WIKI_EXPLOIT_SCENARIO = ''' ```solidity contract Token{ @@ -28,7 +40,7 @@ contract Token{ ``` `Token.transfer` does not return a boolean. Bob deploys the token. Alice creates a contract that interacts with it but assumes a correct ERC20 interface implementation. Alice's contract is unable to interact with Bob's contract.''' - WIKI_RECOMMENDATION = 'Return a boolean for the `approve`/`transfer`/`transferFrom` functions.' + WIKI_RECOMMENDATION = 'Set the appropriate return values and value-types for the defined ERC20 functions.' @staticmethod def incorrect_erc20_interface(signature): @@ -43,6 +55,15 @@ contract Token{ if name == 'approve' and parameters == ['address', 'uint256'] and returnVars != ['bool']: return True + if name == 'allowance' and parameters == ['address', 'address'] and returnVars != ['uint256']: + return True + + if name == 'balanceOf' and parameters == ['address'] and returnVars != ['uint256']: + return True + + if name == 'totalSupply' and parameters == [] and returnVars != ['uint256']: + return True + return False @staticmethod @@ -52,28 +73,25 @@ contract Token{ Returns: list(str) : list of incorrect function signatures """ - # Obtain all function names for this contract - full_names = set([f.full_name for f in contract.functions]) + # Verify this is an ERC20 contract. + if not is_possible_erc20(contract): + return [] # If this contract implements a function from ERC721, we can assume it is an ERC721 token. These tokens # offer functions which are similar to ERC20, but are not compatible. - if ('ownerOf(uint256)' in full_names or - 'safeTransferFrom(address,address,uint256,bytes)' in full_names or - 'safeTransferFrom(address,address,uint256)' in full_names or - 'setApprovalForAll(address,bool)' in full_names or - 'getApproved(uint256)' in full_names or - 'isApprovedForAll(address,address)' in full_names): + # NOTE: This detector is dependent on this one, so we import here to avoid errors. + from slither.detectors.erc.incorrect_erc721_interface import is_possible_erc721 + if is_possible_erc721(contract): return [] - functions = [f for f in contract.functions if f.contract == contract and \ - IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] + functions = [f for f in contract.functions if IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] return functions def _detect(self): """ Detect incorrect erc20 interface Returns: - dict: [contrat name] = set(str) events + dict: [contract name] = set(str) events """ results = [] for c in self.contracts: diff --git a/slither/detectors/erc/incorrect_erc721_interface.py b/slither/detectors/erc/incorrect_erc721_interface.py new file mode 100644 index 000000000..900666674 --- /dev/null +++ b/slither/detectors/erc/incorrect_erc721_interface.py @@ -0,0 +1,113 @@ +""" +Detect incorrect erc721 interface. +""" +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.detectors.erc.incorrect_erc20_interface import is_possible_erc20 + + +def is_possible_erc721(contract): + """ + Checks if the provided contract could be attempting to implement ERC721 standards. + :param contract: The contract to check for token compatibility. + :return: Returns a boolean indicating if the provided contract met the token standard. + """ + full_names = set([f.full_name for f in contract.functions]) + return is_possible_erc20(contract) and \ + ('ownerOf(uint256)' in full_names or + 'safeTransferFrom(address,address,uint256,bytes)' in full_names or + 'safeTransferFrom(address,address,uint256)' in full_names or + 'setApprovalForAll(address,bool)' in full_names or + 'getApproved(uint256)' in full_names or + 'isApprovedForAll(address,address)' in full_names) + + +class IncorrectERC721InterfaceDetection(AbstractDetector): + """ + Incorrect ERC721 Interface + """ + + ARGUMENT = 'erc721-interface' + HELP = 'Incorrect ERC721 interfaces' + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc721-interface' + + WIKI_TITLE = 'Incorrect erc721 interface' + WIKI_DESCRIPTION = 'Incorrect return values for ERC721 functions. A contract compiled with solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing.' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract Token{ + function ownerOf(uint256 _tokenId) external view returns (bool); + //... +} +``` +`Token.ownerOf` does not return an address as ERC721 expects. Bob deploys the token. Alice creates a contract that interacts with it but assumes a correct ERC721 interface implementation. Alice's contract is unable to interact with Bob's contract.''' + + WIKI_RECOMMENDATION = 'Set the appropriate return values and value-types for the defined ERC721 functions.' + + @staticmethod + def incorrect_erc721_interface(signature): + (name, parameters, returnVars) = signature + + # ERC721 + if name == 'balanceOf' and parameters == ['address'] and returnVars != ['uint256']: + return True + if name == 'ownerOf' and parameters == ['uint256'] and returnVars != ['address']: + return True + if name == 'safeTransferFrom' and parameters == ['address', 'address', 'uint256', 'bytes'] and returnVars != []: + return True + if name == 'safeTransferFrom' and parameters == ['address', 'address', 'uint256'] and returnVars != []: + return True + if name == 'transferFrom' and parameters == ['address', 'address', 'uint256'] and returnVars != []: + return True + if name == 'approve' and parameters == ['address', 'uint256'] and returnVars != []: + return True + if name == 'setApprovalForAll' and parameters == ['address', 'bool'] and returnVars != []: + return True + if name == 'getApproved' and parameters == ['uint256'] and returnVars != ['address']: + return True + if name == 'isApprovedForAll' and parameters == ['address', 'address'] and returnVars != ['bool']: + return True + + # ERC165 (dependency) + if name == 'supportsInterface' and parameters == ['bytes4'] and returnVars != ['bool']: + return True + + return False + + @staticmethod + def detect_incorrect_erc721_interface(contract): + """ Detect incorrect ERC721 interface + + Returns: + list(str) : list of incorrect function signatures + """ + + # Verify this is an ERC721 contract. + if not is_possible_erc721(contract) or not is_possible_erc20(contract): + return [] + + functions = [f for f in contract.functions if IncorrectERC721InterfaceDetection.incorrect_erc721_interface(f.signature)] + return functions + + def _detect(self): + """ Detect incorrect erc721 interface + + Returns: + dict: [contract name] = set(str) events + """ + results = [] + for c in self.contracts: + functions = IncorrectERC721InterfaceDetection.detect_incorrect_erc721_interface(c) + if functions: + info = "{} ({}) has incorrect ERC721 function interface(s):\n" + info = info.format(c.name, + c.source_mapping_str) + for function in functions: + info += "\t-{} ({})\n".format(function.name, function.source_mapping_str) + json = self.generate_json_result(info) + self.add_functions_to_json(functions, json) + results.append(json) + + return results diff --git a/slither/detectors/erc20/unindexed_event_parameters.py b/slither/detectors/erc/unindexed_event_parameters.py similarity index 100% rename from slither/detectors/erc20/unindexed_event_parameters.py rename to slither/detectors/erc/unindexed_event_parameters.py diff --git a/tests/incorrect_erc20_interface.sol b/tests/incorrect_erc20_interface.sol index e082b77ef..6fdcb71a2 100644 --- a/tests/incorrect_erc20_interface.sol +++ b/tests/incorrect_erc20_interface.sol @@ -1,7 +1,10 @@ pragma solidity ^0.4.24; contract Token{ - function transfer(address to, uint value) external; - + function approve(address spender, uint value) external; + function transferFrom(address from, address to, uint value) external; + function totalSupply() external; + function balanceOf(address who) external; + function allowance(address owner, address spender) external; } diff --git a/tests/incorrect_erc721_interface.sol b/tests/incorrect_erc721_interface.sol new file mode 100644 index 000000000..a8e3e607b --- /dev/null +++ b/tests/incorrect_erc721_interface.sol @@ -0,0 +1,16 @@ +pragma solidity ^0.4.24; + +interface IERC165 { + function supportsInterface(bytes4 interfaceID) external; +} +contract Token is IERC165{ + function balanceOf(address _owner) external; + function ownerOf(uint256 _tokenId) external; + function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external returns (bool); + function safeTransferFrom(address _from, address _to, uint256 _tokenId) external returns (bool); + function transferFrom(address _from, address _to, uint256 _tokenId) external returns (bool); + function approve(address _approved, uint256 _tokenId) external returns (bool); + function setApprovalForAll(address _operator, bool _approved) external returns (bool); + function getApproved(uint256 _tokenId) external; + function isApprovedForAll(address _owner, address _operator) external; +} From 5e1eb423113d5524ad044f0b59e82dd700fa2593 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 6 May 2019 17:53:51 +0100 Subject: [PATCH 3/8] ERC20/721 detector: move is_possible_* function to contract.has_an_erc*_function Add contract.is_erc721 --- scripts/tests_generate_expected_json_4.sh | 1 + scripts/travis_test_4.sh | 1 + slither/core/declarations/contract.py | 46 ++++ .../erc/incorrect_erc20_interface.py | 18 +- .../erc/incorrect_erc721_interface.py | 19 +- ...rrect_erc20_interface.erc20-interface.json | 218 +++++++++++++++++- ...orrect_erc20_interface.erc20-interface.txt | 9 +- 7 files changed, 271 insertions(+), 41 deletions(-) diff --git a/scripts/tests_generate_expected_json_4.sh b/scripts/tests_generate_expected_json_4.sh index e69148347..184c199f1 100755 --- a/scripts/tests_generate_expected_json_4.sh +++ b/scripts/tests_generate_expected_json_4.sh @@ -23,6 +23,7 @@ generate_expected_json(){ #generate_expected_json tests/deprecated_calls.sol "deprecated-standards" #generate_expected_json tests/erc20_indexed.sol "erc20-indexed" #generate_expected_json tests/incorrect_erc20_interface.sol "erc20-interface" +#generate_expected_json tests/incorrect_erc721_interface.sol "erc721-interface" #generate_expected_json tests/uninitialized.sol "uninitialized-state" #generate_expected_json tests/backdoor.sol "backdoor" #generate_expected_json tests/backdoor.sol "suicidal" diff --git a/scripts/travis_test_4.sh b/scripts/travis_test_4.sh index 5150a92f6..171079d9e 100755 --- a/scripts/travis_test_4.sh +++ b/scripts/travis_test_4.sh @@ -72,6 +72,7 @@ test_slither(){ test_slither tests/deprecated_calls.sol "deprecated-standards" test_slither tests/erc20_indexed.sol "erc20-indexed" test_slither tests/incorrect_erc20_interface.sol "erc20-interface" +test_slither tests/incorrect_erc721_interface.sol "erc721-interface" test_slither tests/uninitialized.sol "uninitialized-state" test_slither tests/backdoor.sol "backdoor" test_slither tests/backdoor.sol "suicidal" diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 8a3fcbca9..f53e2a717 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -527,6 +527,14 @@ class Contract(ChildSlither, SourceMapping): """ return all((not f.is_implemented) for f in self.functions) + # endregion + ################################################################################### + ################################################################################### + # region ERC conformance + ################################################################################### + ################################################################################### + + def is_erc20(self): """ Check if the contract is an erc20 token @@ -540,6 +548,44 @@ class Contract(ChildSlither, SourceMapping): 'transferFrom(address,address,uint256)' in full_names and\ 'approve(address,uint256)' in full_names + def is_erc721(self): + full_names = set([f.full_name for f in self.functions]) + return self.is_erc20() and\ + 'ownerOf(uint256)' in full_names and\ + 'safeTransferFrom(address,address,uint256,bytes)' in full_names and\ + 'safeTransferFrom(address,address,uint256)' in full_names and\ + 'setApprovalForAll(address,bool)' in full_names and\ + 'getApproved(uint256)' in full_names and\ + 'isApprovedForAll(address,address)' in full_names + + def has_an_erc20_function(self): + """ + Checks if the provided contract could be attempting to implement ERC20 standards. + :param contract: The contract to check for token compatibility. + :return: Returns a boolean indicating if the provided contract met the token standard. + """ + full_names = set([f.full_name for f in self.functions]) + return 'transfer(address,uint256)' in full_names or \ + 'transferFrom(address,address,uint256)' in full_names or \ + 'approve(address,uint256)' in full_names + + def has_an_erc721_function(self): + """ + Checks if the provided contract could be attempting to implement ERC721 standards. + :param contract: The contract to check for token compatibility. + :return: Returns a boolean indicating if the provided contract met the token standard. + """ + full_names = set([f.full_name for f in self.functions]) + return self.has_an_erc20_function() and \ + ('ownerOf(uint256)' in full_names or + 'safeTransferFrom(address,address,uint256,bytes)' in full_names or + 'safeTransferFrom(address,address,uint256)' in full_names or + 'setApprovalForAll(address,bool)' in full_names or + 'getApproved(uint256)' in full_names or + 'isApprovedForAll(address,address)' in full_names) + + + # endregion ################################################################################### ################################################################################### diff --git a/slither/detectors/erc/incorrect_erc20_interface.py b/slither/detectors/erc/incorrect_erc20_interface.py index 2dd4ad3e0..ab577fbab 100644 --- a/slither/detectors/erc/incorrect_erc20_interface.py +++ b/slither/detectors/erc/incorrect_erc20_interface.py @@ -5,18 +5,6 @@ Some contracts do not return a bool on transfer/transferFrom/approve, which may from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -def is_possible_erc20(contract): - """ - Checks if the provided contract could be attempting to implement ERC20 standards. - :param contract: The contract to check for token compatibility. - :return: Returns a boolean indicating if the provided contract met the token standard. - """ - full_names = set([f.full_name for f in contract.functions]) - return 'transfer(address,uint256)' in full_names or \ - 'transferFrom(address,address,uint256)' in full_names or \ - 'approve(address,uint256)' in full_names - - class IncorrectERC20InterfaceDetection(AbstractDetector): """ Incorrect ERC20 Interface @@ -74,14 +62,12 @@ contract Token{ list(str) : list of incorrect function signatures """ # Verify this is an ERC20 contract. - if not is_possible_erc20(contract): + if not contract.has_an_erc20_function(): return [] # If this contract implements a function from ERC721, we can assume it is an ERC721 token. These tokens # offer functions which are similar to ERC20, but are not compatible. - # NOTE: This detector is dependent on this one, so we import here to avoid errors. - from slither.detectors.erc.incorrect_erc721_interface import is_possible_erc721 - if is_possible_erc721(contract): + if contract.has_an_erc721_function(): return [] functions = [f for f in contract.functions if IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] diff --git a/slither/detectors/erc/incorrect_erc721_interface.py b/slither/detectors/erc/incorrect_erc721_interface.py index 900666674..5ddea37ff 100644 --- a/slither/detectors/erc/incorrect_erc721_interface.py +++ b/slither/detectors/erc/incorrect_erc721_interface.py @@ -2,23 +2,6 @@ Detect incorrect erc721 interface. """ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -from slither.detectors.erc.incorrect_erc20_interface import is_possible_erc20 - - -def is_possible_erc721(contract): - """ - Checks if the provided contract could be attempting to implement ERC721 standards. - :param contract: The contract to check for token compatibility. - :return: Returns a boolean indicating if the provided contract met the token standard. - """ - full_names = set([f.full_name for f in contract.functions]) - return is_possible_erc20(contract) and \ - ('ownerOf(uint256)' in full_names or - 'safeTransferFrom(address,address,uint256,bytes)' in full_names or - 'safeTransferFrom(address,address,uint256)' in full_names or - 'setApprovalForAll(address,bool)' in full_names or - 'getApproved(uint256)' in full_names or - 'isApprovedForAll(address,address)' in full_names) class IncorrectERC721InterfaceDetection(AbstractDetector): @@ -85,7 +68,7 @@ contract Token{ """ # Verify this is an ERC721 contract. - if not is_possible_erc721(contract) or not is_possible_erc20(contract): + if not contract.has_an_erc721_function() or not contract.has_an_erc20_function(): return [] functions = [f for f in contract.functions if IncorrectERC721InterfaceDetection.incorrect_erc721_interface(f.signature)] diff --git a/tests/expected_json/incorrect_erc20_interface.erc20-interface.json b/tests/expected_json/incorrect_erc20_interface.erc20-interface.json index 180113da4..dceec9be7 100644 --- a/tests/expected_json/incorrect_erc20_interface.erc20-interface.json +++ b/tests/expected_json/incorrect_erc20_interface.erc20-interface.json @@ -3,20 +3,184 @@ "check": "erc20-interface", "impact": "Medium", "confidence": "High", - "description": "Token (tests/incorrect_erc20_interface.sol#3-7) has incorrect ERC20 function interface(s):\n\t-transfer (tests/incorrect_erc20_interface.sol#5)\n", + "description": "Token (tests/incorrect_erc20_interface.sol#3-10) has incorrect ERC20 function interface(s):\n\t-transfer (tests/incorrect_erc20_interface.sol#4)\n\t-approve (tests/incorrect_erc20_interface.sol#5)\n\t-transferFrom (tests/incorrect_erc20_interface.sol#6)\n\t-totalSupply (tests/incorrect_erc20_interface.sol#7)\n\t-balanceOf (tests/incorrect_erc20_interface.sol#8)\n\t-allowance (tests/incorrect_erc20_interface.sol#9)\n", "elements": [ + { + "type": "function", + "name": "allowance", + "source_mapping": { + "start": 319, + "length": 60, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 9 + ], + "starting_column": 5, + "ending_column": 65 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 26, + "length": 355, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "approve", + "source_mapping": { + "start": 102, + "length": 55, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 5 + ], + "starting_column": 5, + "ending_column": 60 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 26, + "length": 355, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "balanceOf", + "source_mapping": { + "start": 273, + "length": 41, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 8 + ], + "starting_column": 5, + "ending_column": 46 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 26, + "length": 355, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "totalSupply", + "source_mapping": { + "start": 236, + "length": 32, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 7 + ], + "starting_column": 5, + "ending_column": 37 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 26, + "length": 355, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, { "type": "function", "name": "transfer", "source_mapping": { - "start": 47, + "start": 46, "length": 51, "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", "filename_relative": "tests/incorrect_erc20_interface.sol", "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", "filename_short": "tests/incorrect_erc20_interface.sol", "lines": [ - 5 + 4 ], "starting_column": 5, "ending_column": 56 @@ -26,7 +190,48 @@ "name": "Token", "source_mapping": { "start": 26, - "length": 75, + "length": 355, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "transferFrom", + "source_mapping": { + "start": 162, + "length": 69, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_relative": "tests/incorrect_erc20_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", + "filename_short": "tests/incorrect_erc20_interface.sol", + "lines": [ + 6 + ], + "starting_column": 5, + "ending_column": 74 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 26, + "length": 355, "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", "filename_relative": "tests/incorrect_erc20_interface.sol", "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc20_interface.sol", @@ -36,7 +241,10 @@ 4, 5, 6, - 7 + 7, + 8, + 9, + 10 ], "starting_column": 1, "ending_column": 2 diff --git a/tests/expected_json/incorrect_erc20_interface.erc20-interface.txt b/tests/expected_json/incorrect_erc20_interface.erc20-interface.txt index 4e9c44437..acff5f223 100644 --- a/tests/expected_json/incorrect_erc20_interface.erc20-interface.txt +++ b/tests/expected_json/incorrect_erc20_interface.erc20-interface.txt @@ -1,5 +1,10 @@ INFO:Detectors: -Token (tests/incorrect_erc20_interface.sol#3-7) has incorrect ERC20 function interface(s): - -transfer (tests/incorrect_erc20_interface.sol#5) +Token (tests/incorrect_erc20_interface.sol#3-10) has incorrect ERC20 function interface(s): + -transfer (tests/incorrect_erc20_interface.sol#4) + -approve (tests/incorrect_erc20_interface.sol#5) + -transferFrom (tests/incorrect_erc20_interface.sol#6) + -totalSupply (tests/incorrect_erc20_interface.sol#7) + -balanceOf (tests/incorrect_erc20_interface.sol#8) + -allowance (tests/incorrect_erc20_interface.sol#9) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc20-interface INFO:Slither:tests/incorrect_erc20_interface.sol analyzed (1 contracts), 1 result(s) found From 1d4d681e2c90b6e4020e341f5024b10818196d24 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 7 May 2019 09:33:42 +0100 Subject: [PATCH 4/8] Add missing files --- ...ect_erc721_interface.erc721-interface.json | 442 ++++++++++++++++++ ...rect_erc721_interface.erc721-interface.txt | 14 + 2 files changed, 456 insertions(+) create mode 100644 tests/expected_json/incorrect_erc721_interface.erc721-interface.json create mode 100644 tests/expected_json/incorrect_erc721_interface.erc721-interface.txt diff --git a/tests/expected_json/incorrect_erc721_interface.erc721-interface.json b/tests/expected_json/incorrect_erc721_interface.erc721-interface.json new file mode 100644 index 000000000..9f48193b7 --- /dev/null +++ b/tests/expected_json/incorrect_erc721_interface.erc721-interface.json @@ -0,0 +1,442 @@ +[ + { + "check": "erc721-interface", + "impact": "Medium", + "confidence": "High", + "description": "Token (tests/incorrect_erc721_interface.sol#6-16) has incorrect ERC721 function interface(s):\n\t-supportsInterface (tests/incorrect_erc721_interface.sol#4)\n\t-balanceOf (tests/incorrect_erc721_interface.sol#7)\n\t-ownerOf (tests/incorrect_erc721_interface.sol#8)\n\t-safeTransferFrom (tests/incorrect_erc721_interface.sol#9)\n\t-safeTransferFrom (tests/incorrect_erc721_interface.sol#10)\n\t-transferFrom (tests/incorrect_erc721_interface.sol#11)\n\t-approve (tests/incorrect_erc721_interface.sol#12)\n\t-setApprovalForAll (tests/incorrect_erc721_interface.sol#13)\n\t-getApproved (tests/incorrect_erc721_interface.sol#14)\n\t-isApprovedForAll (tests/incorrect_erc721_interface.sol#15)\n", + "elements": [ + { + "type": "function", + "name": "approve", + "source_mapping": { + "start": 549, + "length": 78, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 12 + ], + "starting_column": 5, + "ending_column": 83 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "balanceOf", + "source_mapping": { + "start": 140, + "length": 44, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 7 + ], + "starting_column": 5, + "ending_column": 49 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "getApproved", + "source_mapping": { + "start": 723, + "length": 48, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 14 + ], + "starting_column": 5, + "ending_column": 53 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "isApprovedForAll", + "source_mapping": { + "start": 776, + "length": 70, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 15 + ], + "starting_column": 5, + "ending_column": 75 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "ownerOf", + "source_mapping": { + "start": 189, + "length": 44, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 8 + ], + "starting_column": 5, + "ending_column": 49 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "safeTransferFrom", + "source_mapping": { + "start": 238, + "length": 108, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 9 + ], + "starting_column": 5, + "ending_column": 113 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "safeTransferFrom", + "source_mapping": { + "start": 351, + "length": 96, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 10 + ], + "starting_column": 5, + "ending_column": 101 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "setApprovalForAll", + "source_mapping": { + "start": 632, + "length": 86, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 13 + ], + "starting_column": 5, + "ending_column": 91 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "supportsInterface", + "source_mapping": { + "start": 50, + "length": 56, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 4 + ], + "starting_column": 5, + "ending_column": 61 + }, + "contract": { + "type": "contract", + "name": "IERC165", + "source_mapping": { + "start": 26, + "length": 82, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 3, + 4, + 5 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "function", + "name": "transferFrom", + "source_mapping": { + "start": 452, + "length": 92, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 11 + ], + "starting_column": 5, + "ending_column": 97 + }, + "contract": { + "type": "contract", + "name": "Token", + "source_mapping": { + "start": 109, + "length": 739, + "filename_used": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_relative": "tests/incorrect_erc721_interface.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/incorrect_erc721_interface.sol", + "filename_short": "tests/incorrect_erc721_interface.sol", + "lines": [ + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + ] + } +] \ No newline at end of file diff --git a/tests/expected_json/incorrect_erc721_interface.erc721-interface.txt b/tests/expected_json/incorrect_erc721_interface.erc721-interface.txt new file mode 100644 index 000000000..76530c072 --- /dev/null +++ b/tests/expected_json/incorrect_erc721_interface.erc721-interface.txt @@ -0,0 +1,14 @@ +INFO:Detectors: +Token (tests/incorrect_erc721_interface.sol#6-16) has incorrect ERC721 function interface(s): + -supportsInterface (tests/incorrect_erc721_interface.sol#4) + -balanceOf (tests/incorrect_erc721_interface.sol#7) + -ownerOf (tests/incorrect_erc721_interface.sol#8) + -safeTransferFrom (tests/incorrect_erc721_interface.sol#9) + -safeTransferFrom (tests/incorrect_erc721_interface.sol#10) + -transferFrom (tests/incorrect_erc721_interface.sol#11) + -approve (tests/incorrect_erc721_interface.sol#12) + -setApprovalForAll (tests/incorrect_erc721_interface.sol#13) + -getApproved (tests/incorrect_erc721_interface.sol#14) + -isApprovedForAll (tests/incorrect_erc721_interface.sol#15) +Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-erc721-interface +INFO:Slither:tests/incorrect_erc721_interface.sol analyzed (2 contracts), 1 result(s) found From 3876b2b479afa391f23262591a152f20498479f5 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 7 May 2019 13:45:19 +0100 Subject: [PATCH 5/8] Improvements of the human summary printer. Several of the new features are integrated within slither.core or slither.utils - Add state_variable.signature function(s) to translate a state variable to a function signature - Add several contract.is_erc_* methods - Add contract.is_from_dependency that checks if the contract is from a dependency - Add utils.erc with standard ERCs signatuers - Add utils.standard_libraries with standard libraries Several of these methods require crytic_compile 3ed5160924d08a4d3c86d7cddf9f1e17dfe51808 --- slither/core/declarations/contract.py | 122 +++++++++-- slither/core/variables/state_variable.py | 45 ++++ .../erc/incorrect_erc20_interface.py | 4 +- .../erc/incorrect_erc721_interface.py | 2 +- slither/printers/summary/human_summary.py | 83 +++++++- slither/slither.py | 9 +- slither/utils/erc.py | 71 +++++++ slither/utils/standard_libraries.py | 193 ++++++++++++++++++ slither/utils/type.py | 3 + 9 files changed, 497 insertions(+), 35 deletions(-) create mode 100644 slither/utils/erc.py create mode 100644 slither/utils/standard_libraries.py diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index f53e2a717..f06bc3031 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -5,6 +5,9 @@ import logging from slither.core.children.child_slither import ChildSlither from slither.core.source_mapping.source_mapping import SourceMapping from slither.core.declarations.function import Function +from slither.utils.erc import ERC20_signatures, \ + ERC165_signatures, ERC223_signatures, ERC721_signatures, \ + ERC1820_signatures, ERC777_signatures logger = logging.getLogger("Contract") @@ -35,6 +38,8 @@ class Contract(ChildSlither, SourceMapping): self._using_for = {} self._kind = None + self._signatures = None + self._initial_state_variables = [] # ssa @@ -212,6 +217,20 @@ class Contract(ChildSlither, SourceMapping): ################################################################################### ################################################################################### + @property + def functions_signatures(self): + """ + Return the signatures of all the public/eterxnal functions/state variables + :return: list(string) the signatures of all the functions that can be called + """ + if self._signatures == None: + sigs = [v.full_name for v in self.state_variables if v.visibility in ['public', + 'external']] + + sigs += set([f.full_name for f in self.functions if f.visibility in ['public', 'external']]) + self._signatures = list(set(sigs)) + return self._signatures + @property def functions(self): ''' @@ -534,50 +553,102 @@ class Contract(ChildSlither, SourceMapping): ################################################################################### ################################################################################### + def ercs(self): + """ + Return the ERC implemented + :return: list of string + """ + all = [('ERC20', lambda x: x.is_erc20()), + ('ERC165', lambda x: x.is_erc165()), + ('ERC1820', lambda x: x.is_erc1820()), + ('ERC223', lambda x: x.is_erc223()), + ('ERC721', lambda x: x.is_erc721()), + ('ERC777', lambda x: x.is_erc777())] + + return [erc[0] for erc in all if erc[1](self)] def is_erc20(self): """ Check if the contract is an erc20 token Note: it does not check for correct return values - Returns: - bool + :return: Returns a true if the contract is an erc20 """ - full_names = set([f.full_name for f in self.functions]) - return 'transfer(address,uint256)' in full_names and\ - 'transferFrom(address,address,uint256)' in full_names and\ - 'approve(address,uint256)' in full_names + full_names = self.functions_signatures + return all((s in full_names for s in ERC20_signatures)) + + def is_erc165(self): + """ + Check if the contract is an erc165 token + + Note: it does not check for correct return values + :return: Returns a true if the contract is an erc165 + """ + full_names = self.functions_signatures + return all((s in full_names for s in ERC165_signatures)) + + def is_erc1820(self): + """ + Check if the contract is an erc1820 + + Note: it does not check for correct return values + :return: Returns a true if the contract is an erc165 + """ + full_names = self.functions_signatures + return all((s in full_names for s in ERC1820_signatures)) + + def is_erc223(self): + """ + Check if the contract is an erc223 token + + Note: it does not check for correct return values + :return: Returns a true if the contract is an erc223 + """ + full_names = self.functions_signatures + return all((s in full_names for s in ERC223_signatures)) def is_erc721(self): - full_names = set([f.full_name for f in self.functions]) - return self.is_erc20() and\ - 'ownerOf(uint256)' in full_names and\ - 'safeTransferFrom(address,address,uint256,bytes)' in full_names and\ - 'safeTransferFrom(address,address,uint256)' in full_names and\ - 'setApprovalForAll(address,bool)' in full_names and\ - 'getApproved(uint256)' in full_names and\ - 'isApprovedForAll(address,address)' in full_names + """ + Check if the contract is an erc721 token + + Note: it does not check for correct return values + :return: Returns a true if the contract is an erc721 + """ + full_names = self.functions_signatures + return all((s in full_names for s in ERC721_signatures)) + + def is_erc777(self): + """ + Check if the contract is an erc777 - def has_an_erc20_function(self): + Note: it does not check for correct return values + :return: Returns a true if the contract is an erc165 + """ + full_names = self.functions_signatures + return all((s in full_names for s in ERC777_signatures)) + + def is_possible_erc20(self): """ Checks if the provided contract could be attempting to implement ERC20 standards. :param contract: The contract to check for token compatibility. :return: Returns a boolean indicating if the provided contract met the token standard. """ - full_names = set([f.full_name for f in self.functions]) + # We do not check for all the functions, as name(), symbol(), might give too many FPs + full_names = self.functions_signatures return 'transfer(address,uint256)' in full_names or \ 'transferFrom(address,address,uint256)' in full_names or \ 'approve(address,uint256)' in full_names - def has_an_erc721_function(self): + def is_possible_erc721(self): """ Checks if the provided contract could be attempting to implement ERC721 standards. :param contract: The contract to check for token compatibility. :return: Returns a boolean indicating if the provided contract met the token standard. """ - full_names = set([f.full_name for f in self.functions]) - return self.has_an_erc20_function() and \ - ('ownerOf(uint256)' in full_names or + # We do not check for all the functions, as name(), symbol(), might give too many FPs + full_names = self.functions_signatures + return ('approve(address,uint256)' in full_names or + 'ownerOf(uint256)' in full_names or 'safeTransferFrom(address,address,uint256,bytes)' in full_names or 'safeTransferFrom(address,address,uint256)' in full_names or 'setApprovalForAll(address,bool)' in full_names or @@ -585,6 +656,17 @@ class Contract(ChildSlither, SourceMapping): 'isApprovedForAll(address,address)' in full_names) + # endregion + ################################################################################### + ################################################################################### + # region Dependencies + ################################################################################### + ################################################################################### + + def is_from_dependency(self): + if self.slither.crytic_compile is None: + return False + return self.slither.crytic_compile.is_dependency(self.source_mapping['filename_absolute']) # endregion ################################################################################### diff --git a/slither/core/variables/state_variable.py b/slither/core/variables/state_variable.py index b8f46482e..e85ca2e09 100644 --- a/slither/core/variables/state_variable.py +++ b/slither/core/variables/state_variable.py @@ -1,8 +1,53 @@ from .variable import Variable from slither.core.children.child_contract import ChildContract +from slither.utils.type import export_nested_types_from_variable class StateVariable(ChildContract, Variable): + ################################################################################### + ################################################################################### + # region Signature + ################################################################################### + ################################################################################### + + @property + def signature(self): + """ + Return the signature of the state variable as a function signature + :return: (str, list(str), list(str)), as (name, list parameters type, list return values type) + """ + return self.name, [str(x) for x in export_nested_types_from_variable(self)], self.type + + @property + def signature_str(self): + """ + Return the signature of the state variable as a function signature + :return: str: func_name(type1,type2) returns(type3) + """ + name, parameters, returnVars = self.signature + return name+'('+','.join(parameters)+') returns('+','.join(returnVars)+')' + + # endregion + ################################################################################### + ################################################################################### + # region Name + ################################################################################### + ################################################################################### + @property def canonical_name(self): return '{}:{}'.format(self.contract.name, self.name) + + @property + def full_name(self): + """ + Return the name of the state variable as a function signaure + str: func_name(type1,type2) + :return: the function signature without the return values + """ + name, parameters, _ = self.signature + return name+'('+','.join(parameters)+')' + + # endregion + ################################################################################### + ################################################################################### diff --git a/slither/detectors/erc/incorrect_erc20_interface.py b/slither/detectors/erc/incorrect_erc20_interface.py index ab577fbab..5333f089c 100644 --- a/slither/detectors/erc/incorrect_erc20_interface.py +++ b/slither/detectors/erc/incorrect_erc20_interface.py @@ -62,12 +62,12 @@ contract Token{ list(str) : list of incorrect function signatures """ # Verify this is an ERC20 contract. - if not contract.has_an_erc20_function(): + if not contract.is_possible_erc20(): return [] # If this contract implements a function from ERC721, we can assume it is an ERC721 token. These tokens # offer functions which are similar to ERC20, but are not compatible. - if contract.has_an_erc721_function(): + if contract.is_possible_erc721(): return [] functions = [f for f in contract.functions if IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] diff --git a/slither/detectors/erc/incorrect_erc721_interface.py b/slither/detectors/erc/incorrect_erc721_interface.py index 5ddea37ff..336d549db 100644 --- a/slither/detectors/erc/incorrect_erc721_interface.py +++ b/slither/detectors/erc/incorrect_erc721_interface.py @@ -68,7 +68,7 @@ contract Token{ """ # Verify this is an ERC721 contract. - if not contract.has_an_erc721_function() or not contract.has_an_erc20_function(): + if not contract.is_possible_erc721() or not contract.is_possible_erc20(): return [] functions = [f for f in contract.functions if IncorrectERC721InterfaceDetection.incorrect_erc721_interface(f.signature)] diff --git a/slither/printers/summary/human_summary.py b/slither/printers/summary/human_summary.py index e82b7fef7..625a0aca9 100644 --- a/slither/printers/summary/human_summary.py +++ b/slither/printers/summary/human_summary.py @@ -6,7 +6,7 @@ import logging from slither.printers.abstract_printer import AbstractPrinter from slither.utils.code_complexity import compute_cyclomatic_complexity from slither.utils.colors import green, red, yellow - +from slither.utils.standard_libraries import is_standard_library class PrinterHumanSummary(AbstractPrinter): ARGUMENT = 'human-summary' @@ -88,8 +88,14 @@ class PrinterHumanSummary(AbstractPrinter): issues_informational, issues_low, issues_medium, issues_high = self._get_detectors_result() txt = "Number of informational issues: {}\n".format(green(issues_informational)) txt += "Number of low issues: {}\n".format(green(issues_low)) - txt += "Number of medium issues: {}\n".format(yellow(issues_medium)) - txt += "Number of high issues: {}\n".format(red(issues_high)) + if issues_medium > 0: + txt += "Number of medium issues: {}\n".format(yellow(issues_medium)) + else: + txt += "Number of medium issues: {}\n".format(green(issues_medium)) + if issues_high > 0: + txt += "Number of high issues: {}\n".format(red(issues_high)) + else: + txt += "Number of high issues: {}\n\n".format(green(issues_high)) return txt @@ -119,6 +125,49 @@ class PrinterHumanSummary(AbstractPrinter): def _number_functions(contract): return len(contract.functions) + def _lines_number(self): + if not self.slither.source_code: + return None + total_dep_lines = 0 + total_lines = 0 + for filename, source_code in self.slither.source_code.items(): + lines = len(source_code.splitlines()) + is_dep = False + if self.slither.crytic_compile: + is_dep = self.slither.crytic_compile.is_dependency(filename) + if is_dep: + total_dep_lines += lines + else: + total_lines += lines + return total_lines, total_dep_lines + + 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' + + 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) + + def _standard_libraries(self): + libraries = [] + for contract in self.contracts: + lib = is_standard_library(contract) + if lib: + libraries.append(lib) + + return libraries + + def _ercs(self): + ercs = [] + for contract in self.contracts: + ercs += contract.ercs() + return list(set(ercs)) + def output(self, _filename): """ _filename is not used @@ -126,15 +175,37 @@ class PrinterHumanSummary(AbstractPrinter): _filename(string) """ - txt = "Analyze of {}\n".format(self.slither.filename) + txt = "\n" + txt += self._compilation_type() + + 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' + + number_contracts, number_contracts_deps = self._number_contracts() + txt += f'Number of contracts: {number_contracts} (+ {number_contracts_deps} in dependencies) \n\n' + txt += self.get_detectors_result() + + libs = self._standard_libraries() + if libs: + txt += f'\nUse: {", ".join(libs)}\n' + + ercs = self._ercs() + if ercs: + txt += f'ERCs: {", ".join(ercs)}\n' + 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' is_erc20 = contract.is_erc20() - txt += '\tNumber of functions:{}'.format(self._number_functions(contract)) - txt += "\tIs ERC20 token: {}\n".format(contract.is_erc20()) if is_erc20: + txt += '\tERC20 info:\n' txt += self.get_summary_erc20(contract) self.info(txt) diff --git a/slither/slither.py b/slither/slither.py index d30099426..acb25d619 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -45,9 +45,6 @@ class Slither(SlitherSolc): ''' - truffle_ignore = kwargs.get('truffle_ignore', False) - embark_ignore = kwargs.get('embark_ignore', False) - # list of files provided (see --splitted option) if isinstance(contract, list): self._init_from_list(contract) @@ -56,13 +53,13 @@ class Slither(SlitherSolc): else: super(Slither, self).__init__('') try: - cryticCompile = CryticCompile(contract, **kwargs) - self._crytic_compile = cryticCompile + crytic_compile = CryticCompile(contract, **kwargs) + self._crytic_compile = crytic_compile except InvalidCompilation as e: logger.error('Invalid compilation') logger.error(e) exit(-1) - for path, ast in cryticCompile.asts.items(): + for path, ast in crytic_compile.asts.items(): self._parse_contracts_from_loaded_json(ast, path) self._add_source_code(path) diff --git a/slither/utils/erc.py b/slither/utils/erc.py new file mode 100644 index 000000000..08aa7ad0d --- /dev/null +++ b/slither/utils/erc.py @@ -0,0 +1,71 @@ + +def erc_to_signatures(erc): + return [f'{e[0]}({",".join(e[1])})' for e in erc] + + +# Final +# https://eips.ethereum.org/EIPS/eip-20 +ERC20 = [('name', [], 'string'), + ('symbol', [], 'string'), + ('decimals', [], 'uint8'), + ('totalSupply', [], 'uint256'), + ('balanceOf', ['address'], 'uint256'), + ('transfer', ['address', 'uint256'], 'bool'), + ('transferFrom', ['address', 'address', 'uint256'], 'bool'), + ('approve', ['address', 'uint256'], 'bool'), + ('allowance', ['address', 'address'], 'uint256')] +ERC20_signatures = erc_to_signatures(ERC20) + +# Draft +# https://github.com/ethereum/eips/issues/223 +ERC223 = [('name', [], 'string'), + ('symbol', [], 'string'), + ('decimals', [], 'uint8'), + ('totalSupply', [], 'uint256'), + ('balanceOf', ['address'], 'uint256'), + ('transfer', ['address', 'uint256'], 'bool'), + ('transfer', ['address', 'uint256', 'bytes'], 'bool'), + ('transfer', ['address', 'uint256', 'bytes', 'string'], 'bool')] +ERC223_signatures = erc_to_signatures(ERC223) + +# Final +# https://eips.ethereum.org/EIPS/eip-165 +ERC165 = [('supportsInterface', ['bytes4'], 'bool')] +ERC165_signatures = erc_to_signatures(ERC165) + +# Final +# https://eips.ethereum.org/EIPS/eip-721 +# Must have ERC165 +# name, symbol, tokenURI are optionals +ERC721 = [('balanceOf', ['address'], 'uint256'), + ('ownerOf', ['uint256'], 'address'), + ('safeTransferFrom', ['address', 'address', 'uint256', 'bytes'], ''), + ('safeTransferFrom', ['address', 'address', 'uint256'], ''), + ('transferFrom', ['address', 'address', 'uint256'], ''), + ('approve', ['address', 'uint256'], ''), + ('setApprovalForAll', ['address', 'bool'], ''), + ('getApproved', ['uint256'], 'address'), + ('isApprovedForAll', ['address', 'address'], 'bool')] + ERC165 +ERC721_signatures = erc_to_signatures(ERC721) + +# Final +# https://eips.ethereum.org/EIPS/eip-1820 +ERC1820 = [('canImplementInterfaceForAddress', ['bytes32', 'address'], 'bytes32')] +ERC1820_signatures = erc_to_signatures(ERC1820) + +# Last Call +# https://eips.ethereum.org/EIPS/eip-777 +ERC777 = [('name', [], 'string'), + ('symbol', [], 'string'), + ('totalSupply', [], 'uint256'), + ('balanceOf', ['address'], 'uint256'), + ('granularity', [], 'uint256'), + ('defaultOperators', [], 'address[]'), + ('isOperatorFor', ['address', 'address'], 'bool'), + ('authorizeOperator', ['address'], ''), + ('revokeOperator', ['address'], ''), + ('send', ['address', 'uint256', 'bytes'], ''), + ('operatorSend', ['address', 'address', 'uint256', 'bytes', 'bytes'], ''), + ('burn', ['uint256', 'bytes'] , ''), + ('operatorBurn', ['address', 'uint256', 'bytes', 'bytes'] , '')] +ERC777_signatures = erc_to_signatures(ERC777) \ No newline at end of file diff --git a/slither/utils/standard_libraries.py b/slither/utils/standard_libraries.py new file mode 100644 index 000000000..c73a29b93 --- /dev/null +++ b/slither/utils/standard_libraries.py @@ -0,0 +1,193 @@ +from pathlib import Path + + +libraries = { + 'Openzeppelin-SafeMath': lambda x: is_openzepellin_safemath(x), + 'Openzeppelin-ECRecovery': lambda x: is_openzepellin_ecrecovery(x), + 'Openzeppelin-Ownable': lambda x: is_openzepellin_ownable(x), + 'Openzeppelin-ERC20': lambda x: is_openzepellin_erc20(x), + 'Openzeppelin-ERC721': lambda x: is_openzepellin_erc721(x), + 'Zos-Upgrade': lambda x: is_zos_initializable(x), + 'Dapphub-DSAuth': lambda x: is_dapphub_ds_auth(x), + 'Dapphub-DSMath': lambda x: is_dapphub_ds_math(x), + 'Dapphub-DSToken': lambda x: is_dapphub_ds_token(x), + 'Dapphub-DSProxy': lambda x: is_dapphub_ds_proxy(x), + 'Dapphub-DSGroup': lambda x: is_dapphub_ds_group(x), +} + +def is_standard_library(contract): + for name, is_lib in libraries.items(): + if is_lib(contract): + return name + return None + + +################################################################################### +################################################################################### +# region General libraries +################################################################################### +################################################################################### + + +def is_openzepellin(contract): + if not contract.is_from_dependency(): + return False + return 'openzeppelin-solidity' in Path(contract.source_mapping['filename_absolute']).parts + + +def is_zos(contract): + if not contract.is_from_dependency(): + return False + return 'zos-lib' in Path(contract.source_mapping['filename_absolute']).parts + + +# endregion +################################################################################### +################################################################################### +# region SafeMath +################################################################################### +################################################################################### + + +def is_safemath(contract): + return contract.name == "SafeMath" + + +def is_openzepellin_safemath(contract): + return is_safemath(contract) and is_openzepellin(contract) + +# endregion +################################################################################### +################################################################################### +# region ECRecovery +################################################################################### +################################################################################### + + +def is_ecrecovery(contract): + return contract.name == 'ECRecovery' + + +def is_openzepellin_ecrecovery(contract): + return is_ecrecovery(contract) and is_openzepellin(contract) + + +# endregion +################################################################################### +################################################################################### +# region Ownable +################################################################################### +################################################################################### + + +def is_ownable(contract): + return contract.name == 'Ownable' + + +def is_openzepellin_ownable(contract): + return is_ownable(contract) and is_openzepellin(contract) + + +# endregion +################################################################################### +################################################################################### +# region ERC20 +################################################################################### +################################################################################### + + +def is_erc20(contract): + return contract.name == 'ERC20' + + +def is_openzepellin_erc20(contract): + return is_erc20(contract) and is_openzepellin(contract) + + +# endregion +################################################################################### +################################################################################### +# region ERC721 +################################################################################### +################################################################################### + + +def is_erc721(contract): + return contract.name == 'ERC721' + + +def is_openzepellin_erc721(contract): + return is_erc721(contract) and is_openzepellin(contract) + + +# endregion +################################################################################### +################################################################################### +# region Zos Initializable +################################################################################### +################################################################################### + + +def is_initializable(contract): + return contract.name == 'Initializable' + + +def is_zos_initializable(contract): + return is_initializable(contract) and is_zos(contract) + + +# endregion +################################################################################### +################################################################################### +# region DappHub +################################################################################### +################################################################################### + +dapphubs = { + 'DSAuth': 'ds-auth', + 'DSMath': 'ds-math', + 'DSToken': 'ds-token', + 'DSProxy': 'ds-proxy', + 'DSGroup': 'ds-group', +} + + +def _is_ds(contract, name): + return contract.name == name + +def _is_dappdhub_ds(contract, name): + if not contract.is_from_dependency(): + return False + if not dapphubs[name] in Path(contract.source_mapping['filename_absolute']).parts: + return False + return _is_ds(contract, name) + +def is_ds_auth(contract): + return _is_ds(contract, 'DSAuth') + +def is_dapphub_ds_auth(contract): + return _is_dappdhub_ds(contract, 'DSAuth') + +def is_ds_math(contract): + return _is_ds(contract, 'DSMath') + +def is_dapphub_ds_math(contract): + return _is_dappdhub_ds(contract, 'DSMath') + +def is_ds_token(contract): + return _is_ds(contract, 'DSToken') + +def is_dapphub_ds_token(contract): + return _is_dappdhub_ds(contract, 'DSToken') + +def is_ds_proxy(contract): + return _is_ds(contract, 'DSProxy') + +def is_dapphub_ds_proxy(contract): + return _is_dappdhub_ds(contract, 'DSProxy') + +def is_ds_group(contract): + return _is_ds(contract, 'DSGroup') + +def is_dapphub_ds_group(contract): + return _is_dappdhub_ds(contract, 'DSGroup') diff --git a/slither/utils/type.py b/slither/utils/type.py index d5bca3720..3b0577987 100644 --- a/slither/utils/type.py +++ b/slither/utils/type.py @@ -1,16 +1,19 @@ from slither.core.solidity_types import (ArrayType, MappingType, ElementaryType) + def _add_mapping_parameter(t, l): while isinstance(t, MappingType): l.append(t.type_from) t = t.type_to _add_array_parameter(t, l) + def _add_array_parameter(t, l): while isinstance(t, ArrayType): l.append(ElementaryType('uint256')) t = t.type + def export_nested_types_from_variable(variable): """ Export the list of nested types (mapping/array) From 0beb48146a169779f76943f3ff74d06dba766478 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 7 May 2019 13:51:14 +0100 Subject: [PATCH 6/8] Update travis install --- scripts/travis_install.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/travis_install.sh b/scripts/travis_install.sh index acd47eaba..77d84e296 100755 --- a/scripts/travis_install.sh +++ b/scripts/travis_install.sh @@ -1,4 +1,12 @@ #!/usr/bin/env bash + +# TODO: temporary until the next crytic-compile release +git clone https://github.com/crytic/crytic-compile +cd crytic-compile +git checkout dev +python setup.py install +cd .. + python setup.py install # Used by travis_test.sh pip install deepdiff From e6c9d186e331f82c2d9143eb3f2f195dc8e36983 Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 7 May 2019 17:55:13 +0100 Subject: [PATCH 7/8] Remove optional ERC20 functions --- slither/core/declarations/contract.py | 3 +-- slither/utils/erc.py | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index f06bc3031..cbfe723f7 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -647,8 +647,7 @@ class Contract(ChildSlither, SourceMapping): """ # We do not check for all the functions, as name(), symbol(), might give too many FPs full_names = self.functions_signatures - return ('approve(address,uint256)' in full_names or - 'ownerOf(uint256)' in full_names or + return ('ownerOf(uint256)' in full_names or 'safeTransferFrom(address,address,uint256,bytes)' in full_names or 'safeTransferFrom(address,address,uint256)' in full_names or 'setApprovalForAll(address,bool)' in full_names or diff --git a/slither/utils/erc.py b/slither/utils/erc.py index 08aa7ad0d..21c2c53a5 100644 --- a/slither/utils/erc.py +++ b/slither/utils/erc.py @@ -5,10 +5,8 @@ def erc_to_signatures(erc): # Final # https://eips.ethereum.org/EIPS/eip-20 -ERC20 = [('name', [], 'string'), - ('symbol', [], 'string'), - ('decimals', [], 'uint8'), - ('totalSupply', [], 'uint256'), +# name, symbolc, decimals are optionals +ERC20 = [('totalSupply', [], 'uint256'), ('balanceOf', ['address'], 'uint256'), ('transfer', ['address', 'uint256'], 'bool'), ('transferFrom', ['address', 'address', 'uint256'], 'bool'), From d10c6fbced7000ebfa1131a9bad2cb651823d93b Mon Sep 17 00:00:00 2001 From: Josselin Date: Tue, 7 May 2019 19:12:49 +0100 Subject: [PATCH 8/8] Improve Exception handling --- slither/__main__.py | 7 +++++++ slither/all_exceptions.py | 7 +++++++ slither/core/exceptions.py | 3 +++ .../core/expressions/assignment_operation.py | 8 +++----- slither/core/expressions/binary_operation.py | 8 +++----- slither/core/expressions/unary_operation.py | 11 ++++------- slither/exceptions.py | 3 +++ slither/slither.py | 18 ++++++------------ slither/slithir/convert.py | 7 +++---- slither/slithir/exceptions.py | 3 +++ slither/slithir/operations/binary.py | 7 +++---- slither/slithir/operations/unary.py | 9 +++------ slither/slithir/utils/ssa.py | 4 ++-- slither/solc_parsing/declarations/contract.py | 4 ++-- slither/solc_parsing/declarations/function.py | 10 ++++------ slither/solc_parsing/exceptions.py | 7 +++++++ .../expressions/expression_parsing.py | 12 +++++------- slither/solc_parsing/slitherSolc.py | 14 +++++++------- .../solidity_types/type_parsing.py | 10 ++++------ .../variables/variable_declaration.py | 11 ++++------- slither/visitors/expression/expression.py | 10 ++++------ .../visitors/slithir/expression_to_slithir.py | 6 ++---- 22 files changed, 89 insertions(+), 90 deletions(-) create mode 100644 slither/all_exceptions.py create mode 100644 slither/core/exceptions.py create mode 100644 slither/exceptions.py create mode 100644 slither/slithir/exceptions.py create mode 100644 slither/solc_parsing/exceptions.py diff --git a/slither/__main__.py b/slither/__main__.py index 89678b291..afbdc689c 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -24,6 +24,7 @@ from slither.utils.command_line import (output_detectors, output_results_to_mark output_detectors_json, output_printers, output_to_markdown, output_wiki) from crytic_compile import is_supported +from slither.exceptions import SlitherException logging.basicConfig() logger = logging.getLogger("Slither") @@ -551,6 +552,12 @@ def main_impl(all_detector_classes, all_printer_classes): return exit(results) + except SlitherException as e: + logging.error(red('Error:')) + logging.error(red(e)) + logging.error('Please report an issue to https://github.com/crytic/slither/issues') + sys.exit(-1) + except Exception: logging.error('Error in %s' % args.filename) logging.error(traceback.format_exc()) diff --git a/slither/all_exceptions.py b/slither/all_exceptions.py new file mode 100644 index 000000000..39412971d --- /dev/null +++ b/slither/all_exceptions.py @@ -0,0 +1,7 @@ +""" +This module import all slither exceptions +""" +from slither.slithir.exceptions import SlithIRError +from slither.solc_parsing.exceptions import ParsingError, ParsingContractNotFound, ParsingNameReuse +from slither.core.exceptions import SlitherCoreError +from slither.exceptions import SlitherException \ No newline at end of file diff --git a/slither/core/exceptions.py b/slither/core/exceptions.py new file mode 100644 index 000000000..3f4db4bbf --- /dev/null +++ b/slither/core/exceptions.py @@ -0,0 +1,3 @@ +from slither.exceptions import SlitherException + +class SlitherCoreError(SlitherException): pass diff --git a/slither/core/expressions/assignment_operation.py b/slither/core/expressions/assignment_operation.py index fe7d524bf..749e3843a 100644 --- a/slither/core/expressions/assignment_operation.py +++ b/slither/core/expressions/assignment_operation.py @@ -1,7 +1,7 @@ import logging from slither.core.expressions.expression_typed import ExpressionTyped from slither.core.expressions.expression import Expression - +from slither.core.exceptions import SlitherCoreError logger = logging.getLogger("AssignmentOperation") @@ -43,8 +43,7 @@ class AssignmentOperationType: if operation_type == '%=': return AssignmentOperationType.ASSIGN_MODULO - logger.error('get_type: Unknown operation type {})'.format(operation_type)) - exit(-1) + raise SlitherCoreError('get_type: Unknown operation type {})'.format(operation_type)) @staticmethod def str(operation_type): @@ -71,8 +70,7 @@ class AssignmentOperationType: if operation_type == AssignmentOperationType.ASSIGN_MODULO: return '%=' - logger.error('str: Unknown operation type {})'.format(operation_type)) - exit(-1) + raise SlitherCoreError('str: Unknown operation type {})'.format(operation_type)) class AssignmentOperation(ExpressionTyped): diff --git a/slither/core/expressions/binary_operation.py b/slither/core/expressions/binary_operation.py index 9da7cb5df..8ede21931 100644 --- a/slither/core/expressions/binary_operation.py +++ b/slither/core/expressions/binary_operation.py @@ -1,7 +1,7 @@ import logging from slither.core.expressions.expression_typed import ExpressionTyped from slither.core.expressions.expression import Expression - +from slither.core.exceptions import SlitherCoreError logger = logging.getLogger("BinaryOperation") @@ -67,8 +67,7 @@ class BinaryOperationType: if operation_type == '||': return BinaryOperationType.OROR - logger.error('get_type: Unknown operation type {})'.format(operation_type)) - exit(-1) + raise SlitherCoreError('get_type: Unknown operation type {})'.format(operation_type)) @staticmethod def str(operation_type): @@ -110,8 +109,7 @@ class BinaryOperationType: return '&&' if operation_type == BinaryOperationType.OROR: return '||' - logger.error('str: Unknown operation type {})'.format(operation_type)) - exit(-1) + raise SlitherCoreError('str: Unknown operation type {})'.format(operation_type)) class BinaryOperation(ExpressionTyped): diff --git a/slither/core/expressions/unary_operation.py b/slither/core/expressions/unary_operation.py index 3416cd854..82cb52eb5 100644 --- a/slither/core/expressions/unary_operation.py +++ b/slither/core/expressions/unary_operation.py @@ -1,7 +1,7 @@ import logging from slither.core.expressions.expression_typed import ExpressionTyped from slither.core.expressions.expression import Expression -from slither.core.solidity_types.type import Type +from slither.core.exceptions import SlitherCoreError logger = logging.getLogger("UnaryOperation") @@ -38,8 +38,7 @@ class UnaryOperationType: return UnaryOperationType.PLUSPLUS_POST if operation_type == '--': return UnaryOperationType.MINUSMINUS_POST - logger.error('get_type: Unknown operation type {}'.format(operation_type)) - exit(-1) + raise SlitherCoreError('get_type: Unknown operation type {}'.format(operation_type)) @staticmethod def str(operation_type): @@ -58,8 +57,7 @@ class UnaryOperationType: if operation_type in [UnaryOperationType.MINUSMINUS_PRE, UnaryOperationType.MINUSMINUS_POST]: return '--' - logger.error('str: Unknown operation type {}'.format(operation_type)) - exit(-1) + raise SlitherCoreError('str: Unknown operation type {}'.format(operation_type)) @staticmethod def is_prefix(operation_type): @@ -74,8 +72,7 @@ class UnaryOperationType: elif operation_type in [UnaryOperationType.PLUSPLUS_POST, UnaryOperationType.MINUSMINUS_POST]: return False - logger.error('is_prefix: Unknown operation type {}'.format(operation_type)) - exit(-1) + raise SlitherCoreError('is_prefix: Unknown operation type {}'.format(operation_type)) class UnaryOperation(ExpressionTyped): diff --git a/slither/exceptions.py b/slither/exceptions.py new file mode 100644 index 000000000..85f7e737b --- /dev/null +++ b/slither/exceptions.py @@ -0,0 +1,3 @@ +class SlitherException(Exception): pass + +class SlitherError(SlitherException): pass diff --git a/slither/slither.py b/slither/slither.py index d30099426..bd0690938 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -11,7 +11,7 @@ from crytic_compile import CryticCompile, InvalidCompilation from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification from slither.printers.abstract_printer import AbstractPrinter from .solc_parsing.slitherSolc import SlitherSolc -from .utils.colors import red +from .exceptions import SlitherError logger = logging.getLogger("Slither") logging.basicConfig() @@ -59,9 +59,7 @@ class Slither(SlitherSolc): cryticCompile = CryticCompile(contract, **kwargs) self._crytic_compile = cryticCompile except InvalidCompilation as e: - logger.error('Invalid compilation') - logger.error(e) - exit(-1) + raise SlitherError('Invalid compilation: '+e) for path, ast in cryticCompile.asts.items(): self._parse_contracts_from_loaded_json(ast, path) @@ -81,14 +79,12 @@ class Slither(SlitherSolc): def _init_from_raw_json(self, filename): if not os.path.isfile(filename): - logger.error('{} does not exist (are you in the correct directory?)'.format(filename)) - exit(-1) + raise SlitherError('{} does not exist (are you in the correct directory?)'.format(filename)) assert filename.endswith('json') with open(filename, encoding='utf8') as astFile: stdout = astFile.read() if not stdout: - logger.info('Empty AST file: %s', filename) - sys.exit(-1) + raise SlitherError('Empty AST file: %s', filename) contracts_json = stdout.split('\n=') super(Slither, self).__init__(filename) @@ -176,14 +172,12 @@ class Slither(SlitherSolc): def _run_solc(self, filename, solc, disable_solc_warnings, solc_arguments, ast_format): if not os.path.isfile(filename): - logger.error('{} does not exist (are you in the correct directory?)'.format(filename)) - exit(-1) + raise SlitherError('{} does not exist (are you in the correct directory?)'.format(filename)) assert filename.endswith('json') with open(filename, encoding='utf8') as astFile: stdout = astFile.read() if not stdout: - logger.info('Empty AST file: %s', filename) - sys.exit(-1) + raise SlitherError('Empty AST file: %s', filename) stdout = stdout.split('\n=') return stdout diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 4534ea607..5208e4641 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -33,6 +33,7 @@ from slither.slithir.variables import (Constant, ReferenceVariable, from slither.visitors.slithir.expression_to_slithir import ExpressionToSlithIR from slither.utils.function import get_function_id from slither.utils.type import export_nested_types_from_variable +from slither.slithir.exceptions import SlithIRError logger = logging.getLogger('ConvertToIR') @@ -457,8 +458,7 @@ def propagate_types(ir, node): # temporary operation; they will be removed pass else: - logger.error('Not handling {} during type propgation'.format(type(ir))) - exit(-1) + raise SlithIRError('Not handling {} during type propgation'.format(type(ir))) def extract_tmp_call(ins, contract): assert isinstance(ins, TmpCall) @@ -577,8 +577,7 @@ def convert_to_low_level(ir): new_ir.arguments = ir.arguments new_ir.lvalue.set_type(ElementaryType('bool')) return new_ir - logger.error('Incorrect conversion to low level {}'.format(ir)) - exit(-1) + raise SlithIRError('Incorrect conversion to low level {}'.format(ir)) def convert_to_push(ir, node): """ diff --git a/slither/slithir/exceptions.py b/slither/slithir/exceptions.py new file mode 100644 index 000000000..b0b82341c --- /dev/null +++ b/slither/slithir/exceptions.py @@ -0,0 +1,3 @@ +from slither.exceptions import SlitherException + +class SlithIRError(SlitherException): pass diff --git a/slither/slithir/operations/binary.py b/slither/slithir/operations/binary.py index 6a4269b5d..e7d004ae4 100644 --- a/slither/slithir/operations/binary.py +++ b/slither/slithir/operations/binary.py @@ -4,6 +4,7 @@ from slither.core.variables.variable import Variable from slither.slithir.utils.utils import is_valid_lvalue, is_valid_rvalue from slither.core.solidity_types import ElementaryType from slither.slithir.variables import ReferenceVariable +from slither.slithir.exceptions import SlithIRError logger = logging.getLogger("BinaryOperationIR") @@ -80,8 +81,7 @@ class BinaryType(object): if operation_type == '||': return BinaryType.OROR - logger.error('get_type: Unknown operation type {})'.format(operation_type)) - exit(-1) + raise SlithIRError('get_type: Unknown operation type {})'.format(operation_type)) @staticmethod def str(operation_type): @@ -123,8 +123,7 @@ class BinaryType(object): return '&&' if operation_type == BinaryType.OROR: return '||' - logger.error('str: Unknown operation type {})'.format(operation_type)) - exit(-1) + raise SlithIRError('str: Unknown operation type {})'.format(operation_type)) class Binary(OperationWithLValue): diff --git a/slither/slithir/operations/unary.py b/slither/slithir/operations/unary.py index 9a09f09ba..1d3a7726f 100644 --- a/slither/slithir/operations/unary.py +++ b/slither/slithir/operations/unary.py @@ -1,8 +1,7 @@ import logging from slither.slithir.operations.lvalue import OperationWithLValue -from slither.core.variables.variable import Variable - from slither.slithir.utils.utils import is_valid_lvalue, is_valid_rvalue +from slither.slithir.exceptions import SlithIRError logger = logging.getLogger("BinaryOperationIR") @@ -17,8 +16,7 @@ class UnaryType: return UnaryType.BANG if operation_type == '~': return UnaryType.TILD - logger.error('get_type: Unknown operation type {}'.format(operation_type)) - exit(-1) + raise SlithIRError('get_type: Unknown operation type {}'.format(operation_type)) @staticmethod def str(operation_type): @@ -27,8 +25,7 @@ class UnaryType: if operation_type == UnaryType.TILD: return '~' - logger.error('str: Unknown operation type {}'.format(operation_type)) - exit(-1) + raise SlithIRError('str: Unknown operation type {}'.format(operation_type)) class Unary(OperationWithLValue): diff --git a/slither/slithir/utils/ssa.py b/slither/slithir/utils/ssa.py index 74238589d..64acac84c 100644 --- a/slither/slithir/utils/ssa.py +++ b/slither/slithir/utils/ssa.py @@ -22,6 +22,7 @@ from slither.slithir.variables import (Constant, LocalIRVariable, ReferenceVariable, ReferenceVariableSSA, StateIRVariable, TemporaryVariable, TemporaryVariableSSA, TupleVariable, TupleVariableSSA) +from slither.slithir.exceptions import SlithIRError logger = logging.getLogger('SSA_Conversion') @@ -662,7 +663,6 @@ def copy_ir(ir, *instances): return Length(value, lvalue) - logger.error('Impossible ir copy on {} ({})'.format(ir, type(ir))) - exit(-1) + raise SlithIRError('Impossible ir copy on {} ({})'.format(ir, type(ir))) # endregion diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 953210bf5..ff50df013 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -9,6 +9,7 @@ from slither.solc_parsing.declarations.modifier import ModifierSolc from slither.solc_parsing.declarations.structure import StructureSolc from slither.solc_parsing.solidity_types.type_parsing import parse_type from slither.solc_parsing.variables.state_variable import StateVariableSolc +from slither.solc_parsing.exceptions import ParsingError logger = logging.getLogger("ContractSolcParsing") @@ -186,8 +187,7 @@ class ContractSolc04(Contract): elif item[self.get_key()] == 'UsingForDirective': self._usingForNotParsed.append(item) else: - logger.error('Unknown contract item: '+item[self.get_key()]) - exit(-1) + raise ParsingError('Unknown contract item: '+item[self.get_key()]) return def _parse_struct(self, struct): diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 6b95ca9f9..d862352ad 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -26,6 +26,7 @@ from slither.utils.expression_manipulations import SplitTernaryExpression from slither.utils.utils import unroll from slither.visitors.expression.export_values import ExportValues from slither.visitors.expression.has_conditional import HasConditional +from slither.solc_parsing.exceptions import ParsingError logger = logging.getLogger("FunctionSolc") @@ -725,8 +726,7 @@ class FunctionSolc(Function): link_nodes(node, new_node) node = new_node else: - logger.error('Statement not parsed %s'%name) - exit(-1) + raise ParsingError('Statement not parsed %s'%name) return node @@ -814,8 +814,7 @@ class FunctionSolc(Function): end_node = self._find_end_loop(node, [], 0) if not end_node: - logger.error('Break in no-loop context {}'.format(node)) - exit(-1) + raise ParsingError('Break in no-loop context {}'.format(node)) for son in node.sons: son.remove_father(node) @@ -826,8 +825,7 @@ class FunctionSolc(Function): start_node = self._find_start_loop(node, []) if not start_node: - logger.error('Continue in no-loop context {}'.format(node.nodeId())) - exit(-1) + raise ParsingError('Continue in no-loop context {}'.format(node.nodeId())) for son in node.sons: son.remove_father(node) diff --git a/slither/solc_parsing/exceptions.py b/slither/solc_parsing/exceptions.py new file mode 100644 index 000000000..f4f0e388f --- /dev/null +++ b/slither/solc_parsing/exceptions.py @@ -0,0 +1,7 @@ +from slither.exceptions import SlitherException + +class ParsingError(SlitherException): pass + +class ParsingNameReuse(SlitherException): pass + +class ParsingContractNotFound(SlitherException): pass \ No newline at end of file diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index 968ae446a..8a69a97f4 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -35,7 +35,7 @@ from slither.core.solidity_types import (ArrayType, ElementaryType, FunctionType, MappingType) from slither.solc_parsing.solidity_types.type_parsing import (UnknownType, parse_type) - +from slither.solc_parsing.exceptions import ParsingError logger = logging.getLogger("ExpressionParsing") @@ -78,8 +78,7 @@ def find_variable(var_name, caller_context, referenced_declaration=None): function = caller_context contract = function.contract else: - logger.error('Incorrect caller context') - exit(-1) + raise ParsingError('Incorrect caller context') if function: # We look for variable declared with the referencedDeclaration attr @@ -627,8 +626,7 @@ def parse_expression(expression, caller_context): elif type_name[caller_context.get_key()] == 'FunctionTypeName': array_type = parse_type(type_name, caller_context) else: - logger.error('Incorrect type array {}'.format(type_name)) - exit(-1) + raise ParsingError('Incorrect type array {}'.format(type_name)) array = NewArray(depth, array_type) return array @@ -664,5 +662,5 @@ def parse_expression(expression, caller_context): call = CallExpression(called, arguments, 'Modifier') return call - logger.error('Expression not parsed %s'%name) - exit(-1) + raise ParsingError('Expression not parsed %s'%name) + diff --git a/slither/solc_parsing/slitherSolc.py b/slither/solc_parsing/slitherSolc.py index 5f0e60b13..1b3651f6d 100644 --- a/slither/solc_parsing/slitherSolc.py +++ b/slither/solc_parsing/slitherSolc.py @@ -14,6 +14,7 @@ from slither.core.declarations.import_directive import Import from slither.analyses.data_dependency.data_dependency import compute_dependency from slither.utils.colors import red +from .exceptions import ParsingNameReuse, ParsingContractNotFound class SlitherSolc(Slither): @@ -182,8 +183,7 @@ class SlitherSolc(Slither): info += '\n{} is defined in:'.format(contract.name) info += '\n- {}\n- {}'.format(contract.source_mapping_str, self._contracts[contract.name].source_mapping_str) - logger.error(info) - exit(-1) + raise ParsingNameReuse(info) else: self._contracts_by_id[contract.id] = contract self._contracts[contract.name] = contract @@ -217,11 +217,11 @@ class SlitherSolc(Slither): father_constructors.append(self._contracts_by_id[i]) except KeyError: - logger.error(red('A contract was not found, it is likely that your codebase contains muliple contracts with the same name')) - logger.error(red('Truffle does not handle this case during compilation')) - logger.error(red('Please read https://github.com/trailofbits/slither/wiki#keyerror-or-nonetype-error')) - logger.error(red('And update your code to remove the duplicate')) - exit(-1) + txt = 'A contract was not found, it is likely that your codebase contains muliple contracts with the same name' + txt += 'Truffle does not handle this case during compilation' + txt += 'Please read https://github.com/trailofbits/slither/wiki#keyerror-or-nonetype-error' + txt += 'And update your code to remove the duplicate' + raise ParsingContractNotFound(txt) contract.setInheritance(ancestors, fathers, father_constructors) contracts_to_be_analyzed = self.contracts diff --git a/slither/solc_parsing/solidity_types/type_parsing.py b/slither/solc_parsing/solidity_types/type_parsing.py index 22e8954ab..af9a0bdb4 100644 --- a/slither/solc_parsing/solidity_types/type_parsing.py +++ b/slither/solc_parsing/solidity_types/type_parsing.py @@ -13,6 +13,7 @@ from slither.core.declarations.function import Function from slither.core.expressions.literal import Literal +from slither.solc_parsing.exceptions import ParsingError import re logger = logging.getLogger('TypeParsing') @@ -118,8 +119,7 @@ def _find_from_type_name(name, contract, contracts, structures, enums): return MappingType(from_type, to_type) if not var_type: - logger.error('Type not found '+str(name)) - exit(-1) + raise ParsingError('Type not found '+str(name)) return UserDefinedType(var_type) @@ -134,8 +134,7 @@ def parse_type(t, caller_context): elif isinstance(caller_context, Function): contract = caller_context.contract else: - logger.error('Incorrect caller context') - exit(-1) + raise ParsingError('Incorrect caller context') is_compact_ast = caller_context.is_compact_ast @@ -223,5 +222,4 @@ def parse_type(t, caller_context): return FunctionType(params_vars, return_values_vars) - logger.error('Type name not found '+str(t)) - exit(-1) + raise ParsingError('Type name not found '+str(t)) diff --git a/slither/solc_parsing/variables/variable_declaration.py b/slither/solc_parsing/variables/variable_declaration.py index 0f5f08ee3..72d8fc55e 100644 --- a/slither/solc_parsing/variables/variable_declaration.py +++ b/slither/solc_parsing/variables/variable_declaration.py @@ -6,7 +6,7 @@ from slither.core.variables.variable import Variable from slither.solc_parsing.solidity_types.type_parsing import parse_type, UnknownType from slither.core.solidity_types.elementary_type import ElementaryType, NonElementaryType - +from slither.solc_parsing.exceptions import ParsingError logger = logging.getLogger("VariableDeclarationSolcParsing") class MultipleVariablesDeclaration(Exception): @@ -51,8 +51,7 @@ class VariableDeclarationSolc(Variable): elif nodeType == 'VariableDeclaration': self._init_from_declaration(var, var['value']) else: - logger.error('Incorrect variable declaration type {}'.format(nodeType)) - exit(-1) + raise ParsingError('Incorrect variable declaration type {}'.format(nodeType)) else: nodeType = var['name'] @@ -65,15 +64,13 @@ class VariableDeclarationSolc(Variable): elif len(var['children']) > 2: raise MultipleVariablesDeclaration else: - logger.error('Variable declaration without children?'+var) - exit(-1) + raise ParsingError('Variable declaration without children?'+var) declaration = var['children'][0] self._init_from_declaration(declaration, init) elif nodeType == 'VariableDeclaration': self._init_from_declaration(var, None) else: - logger.error('Incorrect variable declaration type {}'.format(nodeType)) - exit(-1) + raise ParsingError('Incorrect variable declaration type {}'.format(nodeType)) @property def initialized(self): diff --git a/slither/visitors/expression/expression.py b/slither/visitors/expression/expression.py index fe8eeb821..6714508de 100644 --- a/slither/visitors/expression/expression.py +++ b/slither/visitors/expression/expression.py @@ -15,6 +15,7 @@ from slither.core.expressions.new_elementary_type import NewElementaryType from slither.core.expressions.tuple_expression import TupleExpression from slither.core.expressions.type_conversion import TypeConversion from slither.core.expressions.unary_operation import UnaryOperation +from slither.exceptions import SlitherError logger = logging.getLogger("ExpressionVisitor") @@ -86,8 +87,7 @@ class ExpressionVisitor: pass else: - logger.error('Expression not handled: {}'.format(expression)) - exit(-1) + raise SlitherError('Expression not handled: {}'.format(expression)) self._post_visit(expression) @@ -200,8 +200,7 @@ class ExpressionVisitor: pass else: - logger.error('Expression not handled: {}'.format(expression)) - exit(-1) + raise SlitherError('Expression not handled: {}'.format(expression)) # pre_expression_name @@ -302,8 +301,7 @@ class ExpressionVisitor: pass else: - logger.error('Expression not handled: {}'.format(expression)) - exit(-1) + raise SlitherError('Expression not handled: {}'.format(expression)) # post_expression_name diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index c5f275749..b1f5acbce 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -17,8 +17,7 @@ from slither.slithir.variables import (Constant, ReferenceVariable, TemporaryVariable, TupleVariable) from slither.visitors.expression.expression import ExpressionVisitor -#from slither.slithir.variables.state_variable import StateIRVariable -#from slither.slithir.variables.local_variable import LocalIRVariable +from slither.slithir.exceptions import SlithIRError logger = logging.getLogger("VISTIOR:ExpressionToSlithIR") @@ -57,8 +56,7 @@ def convert_assignment(left, right, t, return_type): elif t == AssignmentOperationType.ASSIGN_MODULO: return Binary(left, left, right, BinaryType.MODULO) - logger.error('Missing type during assignment conversion') - exit(-1) + raise SlithIRError('Missing type during assignment conversion') class ExpressionToSlithIR(ExpressionVisitor):