From 8d2a2ce09c33f75b3262184e11ce3c82e6d6fb60 Mon Sep 17 00:00:00 2001 From: David Pokora Date: Fri, 26 Apr 2019 16:43:17 -0400 Subject: [PATCH 1/9] 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/9] -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 ec732509811ffedf17765247a4f27d45a4c602e0 Mon Sep 17 00:00:00 2001 From: Gilles de Bordeaux Date: Sat, 27 Apr 2019 19:55:42 -0700 Subject: [PATCH 3/9] =?UTF-8?q?Added=20the=20new=20=E2=80=9Ctoo-many-digit?= =?UTF-8?q?s=E2=80=9D=20detector?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- slither/detectors/all_detectors.py | 1 + .../detectors/statements/too_many_digits.py | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 slither/detectors/statements/too_many_digits.py diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 36c35496c..732bee3b9 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -32,5 +32,6 @@ from .erc20.incorrect_interface import IncorrectERC20InterfaceDetection from .erc20.unindexed_event_parameters import UnindexedERC20EventParameters from .statements.deprecated_calls import DeprecatedStandards from .source.rtlo import RightToLeftOverride +from .statements.too_many_digits import TooManyDigits # # diff --git a/slither/detectors/statements/too_many_digits.py b/slither/detectors/statements/too_many_digits.py new file mode 100644 index 000000000..6049c1371 --- /dev/null +++ b/slither/detectors/statements/too_many_digits.py @@ -0,0 +1,54 @@ +""" +Module detecting numbers with too many digits. +""" + +from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification + +class TooManyDigits(AbstractDetector): + """ + Detect numbers with too many digits + """ + + ARGUMENT = 'too_many_digits' # slither will launch the detector with slither.py --too_many_digits + HELP = 'Numbers with too many digits (human readability risk)' + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = 'https://github.com/ethereum/web3.js/blob/0.15.0/lib/utils/utils.js' + WIKI_TITLE = 'too_many_digits' + WIKI_DESCRIPTION = 'Plugin too_many_digits' + WIKI_EXPLOIT_SCENARIO = 'An error or malicious intent may have caused a number with many digits to be different than the number originally intended. This kind of error is usually difficult to find visually.' + WIKI_RECOMMENDATION = 'The number used in the contract has too many digits. Try to use an Ether denomination instead' + + def _detect(self): + results = [] + + from slither import Slither + from slither.slithir.variables import Constant + + # iterate over all contracts + for contract in self.slither.contracts_derived: + # iterate over all functions + for f in contract.functions: + # iterate over all the nodes + for node in f.nodes: + # each node contains a list of IR instruction + for ir in node.irs: + # iterate over all the variables read by the IR + for read in ir.read: + # if the variable is a constant + if isinstance(read, Constant): + # read.value can return an int or a str. Convert it to str + value_as_str = str(read.value) + #name_as_str = str(read.name) + line_of_code = str(node.expression) + if '00000' in value_as_str: + # Info to be printed + info = 'In {}.{} ({}), too many digits used in expression:\n\t- {}\n\tPlease use the proper Ether denomination instead\n' + info = info.format(contract.name, f.name, f.source_mapping_str, line_of_code) + + # Add the result in result + json = self.generate_json_result(info) + self.add_function_to_json(f, json) + results.append(json) + return results From 7e56501a39b4603172d5fa7359b16fa2b6915a35 Mon Sep 17 00:00:00 2001 From: Gilles de Bordeaux Date: Sat, 27 Apr 2019 20:11:59 -0700 Subject: [PATCH 4/9] =?UTF-8?q?Added=20a=20test=20for=20=E2=80=9Ctoo-many-?= =?UTF-8?q?digits=E2=80=9D=20detector?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/travis_test_4.sh | 1 + tests/too_many_digits.sol | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 tests/too_many_digits.sol diff --git a/scripts/travis_test_4.sh b/scripts/travis_test_4.sh index 5150a92f6..e940ea86b 100755 --- a/scripts/travis_test_4.sh +++ b/scripts/travis_test_4.sh @@ -102,3 +102,4 @@ test_slither tests/shadowing_builtin_symbols.sol "shadowing-builtin" test_slither tests/shadowing_local_variable.sol "shadowing-local" test_slither tests/solc_version_incorrect.sol "solc-version" test_slither tests/right_to_left_override.sol "rtlo" +test_slither tests/too_many_digits.sol "too-many-digits" diff --git a/tests/too_many_digits.sol b/tests/too_many_digits.sol new file mode 100644 index 000000000..409030e3d --- /dev/null +++ b/tests/too_many_digits.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.5.7; + +contract C { + uint balance; + + /** + * @dev Variables are not Ok - using too many digits in place of the Ether denomination. + */ + function f() external { + uint x1 = 0x000001; + uint x2 = 0x0000000000001; + uint x3 = 1000000000000000000; + uint x4 = 100000; + balance += x1 + x2 + x3 + x4; + } + + /** + * @dev Variables are Ok - not using too many digits. + */ + function h() external { + uint x1 = 1000; + uint x2 = 100000; + balance += x1 + x2 + 100; + } + + /** + * @dev Variables are Ok - Using Ether denominations. + */ + function i() external { + uint x1 = 1 wei + 10 wei + 100 wei + 1000 wei + 10000 wei; + uint x2 = 1 szabo + 10 szabo + 100 szabo + 1000 szabo + 10000 szabo; + balance += x1 + x2; + } + +} From fbd1ddb5fc7199aaff0a6f1dd73440b1d29f046b Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 6 May 2019 14:59:15 +0100 Subject: [PATCH 5/9] - Improve literals parsing and Constant conversion: - add bool type to constant - convert integer using 'e' (1e10) - convert constant uint -> int when possible - Api added: - Constant.original_value: str version of the constant expression - Structure.elems_ordered: keep original structure declaration order --- slither/core/declarations/function.py | 3 +- slither/core/declarations/structure.py | 9 +++ slither/core/expressions/literal.py | 7 +- slither/core/solidity_types/array_type.py | 4 +- slither/core/variables/state_variable.py | 1 - slither/slithir/convert.py | 74 ++++++++++++++++++- slither/slithir/variables/constant.py | 48 ++++++++++-- .../solc_parsing/declarations/structure.py | 2 + .../expressions/expression_parsing.py | 16 +++- .../solidity_types/type_parsing.py | 4 +- slither/utils/type.py | 31 ++++++++ .../visitors/expression/constants_folding.py | 8 +- .../visitors/slithir/expression_to_slithir.py | 3 +- 13 files changed, 189 insertions(+), 21 deletions(-) create mode 100644 slither/utils/type.py diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index 5c8849880..1a99f90e3 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -86,6 +86,7 @@ class Function(ChildContract, SourceMapping): self._reachable_from_nodes = set() self._reachable_from_functions = set() + ################################################################################### ################################################################################### # region General properties @@ -1070,4 +1071,4 @@ class Function(ChildContract, SourceMapping): def __str__(self): return self._name - # endregion + # endregion \ No newline at end of file diff --git a/slither/core/declarations/structure.py b/slither/core/declarations/structure.py index b11fb7e35..6584cbe19 100644 --- a/slither/core/declarations/structure.py +++ b/slither/core/declarations/structure.py @@ -10,6 +10,8 @@ class Structure(ChildContract, SourceMapping): self._name = None self._canonical_name = None self._elems = None + # Name of the elements in the order of declaration + self._elems_ordered = None @property def canonical_name(self): @@ -23,5 +25,12 @@ class Structure(ChildContract, SourceMapping): def elems(self): return self._elems + @property + def elems_ordered(self): + ret = [] + for e in self._elems_ordered: + ret.append(self._elems[e]) + return ret + def __str__(self): return self.name diff --git a/slither/core/expressions/literal.py b/slither/core/expressions/literal.py index e0c9ce6b0..c1923eff4 100644 --- a/slither/core/expressions/literal.py +++ b/slither/core/expressions/literal.py @@ -2,14 +2,19 @@ from slither.core.expressions.expression import Expression class Literal(Expression): - def __init__(self, value): + def __init__(self, value, type): super(Literal, self).__init__() self._value = value + self._type = type @property def value(self): return self._value + @property + def type(self): + return self._type + def __str__(self): # be sure to handle any character return str(self._value) diff --git a/slither/core/solidity_types/array_type.py b/slither/core/solidity_types/array_type.py index 918ed9355..ad8061b74 100644 --- a/slither/core/solidity_types/array_type.py +++ b/slither/core/solidity_types/array_type.py @@ -10,7 +10,7 @@ class ArrayType(Type): assert isinstance(t, Type) if length: if isinstance(length, int): - length = Literal(length) + length = Literal(length, 'uint256') assert isinstance(length, Expression) super(ArrayType, self).__init__() self._type = t @@ -18,7 +18,7 @@ class ArrayType(Type): if length: if not isinstance(length, Literal): - cf = ConstantFolding(length) + cf = ConstantFolding(length, "uint256") length = cf.result() self._length_value = length else: diff --git a/slither/core/variables/state_variable.py b/slither/core/variables/state_variable.py index adca10c19..b8f46482e 100644 --- a/slither/core/variables/state_variable.py +++ b/slither/core/variables/state_variable.py @@ -3,7 +3,6 @@ from slither.core.children.child_contract import ChildContract class StateVariable(ChildContract, Variable): - @property def canonical_name(self): return '{}:{}'.format(self.contract.name, self.name) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 7b9607df0..4534ea607 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -7,7 +7,9 @@ from slither.core.expressions import Identifier, Literal from slither.core.solidity_types import (ArrayType, ElementaryType, FunctionType, MappingType, UserDefinedType) +from slither.core.solidity_types.elementary_type import Int as ElementaryTypeInt from slither.core.variables.variable import Variable +from slither.core.variables.state_variable import StateVariable from slither.slithir.operations import (Assignment, Balance, Binary, BinaryType, Call, Condition, Delete, EventCall, HighLevelCall, Index, @@ -30,6 +32,7 @@ from slither.slithir.variables import (Constant, ReferenceVariable, TemporaryVariable) 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 logger = logging.getLogger('ConvertToIR') @@ -39,7 +42,8 @@ def convert_expression(expression, node): from slither.core.cfg.node import NodeType if isinstance(expression, Literal) and node.type in [NodeType.IF, NodeType.IFLOOP]: - result = [Condition(Constant(expression.value))] + cst = Constant(expression.value, expression.type) + result = [Condition(cst)] return result if isinstance(expression, Identifier) and node.type in [NodeType.IF, NodeType.IFLOOP]: result = [Condition(expression.value)] @@ -599,7 +603,7 @@ def convert_to_push(ir, node): ir = Push(ir.destination, val) - length = Literal(len(operation.init_values)) + length = Literal(len(operation.init_values), 'uint256') t = operation.init_values[0].type ir.lvalue.set_type(ArrayType(t, length)) @@ -822,6 +826,71 @@ def remove_unused(result): result = [i for i in result if not i in to_remove] return result +# endregion +################################################################################### +################################################################################### +# region Constant type conversioh +################################################################################### +################################################################################### + +def convert_constant_types(irs): + """ + late conversion of uint -> type for constant (Literal) + :param irs: + :return: + """ + # TODO: implement instances lookup for events, NewContract + was_changed = True + while was_changed: + was_changed = False + for ir in irs: + if isinstance(ir, Assignment): + if isinstance(ir.lvalue.type, ElementaryType): + if ir.lvalue.type.type in ElementaryTypeInt: + if ir.rvalue.type.type != 'int256': + ir.rvalue.set_type(ElementaryType('int256')) + was_changed = True + if isinstance(ir, Binary): + if isinstance(ir.lvalue.type, ElementaryType): + if ir.lvalue.type.type in ElementaryTypeInt: + for r in ir.read: + if r.type.type != 'int256': + r.set_type(ElementaryType('int256')) + was_changed = True + if isinstance(ir, (HighLevelCall, InternalCall)): + func = ir.function + if isinstance(func, StateVariable): + types = export_nested_types_from_variable(func) + else: + types = [p.type for p in func.parameters] + for idx, arg in enumerate(ir.arguments): + t = types[idx] + if isinstance(t, ElementaryType): + if t.type in ElementaryTypeInt: + if arg.type.type != 'int256': + arg.set_type(ElementaryType('int256')) + was_changed = True + if isinstance(ir, NewStructure): + st = ir.structure + for idx, arg in enumerate(ir.arguments): + e = st.elems_ordered[idx] + if isinstance(e.type, ElementaryType): + if e.type.type in ElementaryTypeInt: + if arg.type.type != 'int256': + arg.set_type(ElementaryType('int256')) + was_changed = True + if isinstance(ir, InitArray): + if isinstance(ir.lvalue.type, ArrayType): + if isinstance(ir.lvalue.type.type, ElementaryType): + if ir.lvalue.type.type.type in ElementaryTypeInt: + for r in ir.read: + if r.type.type != 'int256': + r.set_type(ElementaryType('int256')) + was_changed = True + + + + # endregion ################################################################################### ################################################################################### @@ -839,6 +908,7 @@ def apply_ir_heuristics(irs, node): irs = propagate_type_and_convert_call(irs, node) irs = remove_unused(irs) find_references_origin(irs) + convert_constant_types(irs) return irs diff --git a/slither/slithir/variables/constant.py b/slither/slithir/variables/constant.py index e7fa55716..ba19279e1 100644 --- a/slither/slithir/variables/constant.py +++ b/slither/slithir/variables/constant.py @@ -1,17 +1,41 @@ from .variable import SlithIRVariable -from slither.core.solidity_types.elementary_type import ElementaryType +from slither.core.solidity_types.elementary_type import ElementaryType, Int, Uint + class Constant(SlithIRVariable): - def __init__(self, val): + def __init__(self, val, type=None): super(Constant, self).__init__() assert isinstance(val, str) - if val.isdigit(): - self._type = ElementaryType('uint256') - self._val = int(val) + + self._original_value = val + + if type: + assert isinstance(type, ElementaryType) + self._type = type + if type.type in Int + Uint: + if val.startswith('0x'): + self._val = int(val, 16) + else: + if 'e' in val: + base, expo = val.split('e') + self._val = int(float(base)* (10 ** int(expo))) + elif 'E' in val: + base, expo = val.split('E') + self._val = int(float(base) * (10 ** int(expo))) + else: + self._val = int(val) + elif type.type == 'bool': + self._val = val == 'true' + else: + self._val = val else: - self._type = ElementaryType('string') - self._val = val + if val.isdigit(): + self._type = ElementaryType('uint256') + self._val = int(val) + else: + self._type = ElementaryType('string') + self._val = val @property def value(self): @@ -20,10 +44,18 @@ class Constant(SlithIRVariable): If the expression was an hexadecimal delcared as hex'...' return a str Returns: - (str, int) + (str | int | bool) ''' return self._val + @property + def original_value(self): + ''' + Return the string representation of the value + :return: str + ''' + return self._original_value + def __str__(self): return str(self.value) diff --git a/slither/solc_parsing/declarations/structure.py b/slither/solc_parsing/declarations/structure.py index 0026f451e..825e85c98 100644 --- a/slither/solc_parsing/declarations/structure.py +++ b/slither/solc_parsing/declarations/structure.py @@ -16,6 +16,7 @@ class StructureSolc(Structure): self._name = name self._canonical_name = canonicalName self._elems = {} + self._elems_ordered = [] self._elemsNotParsed = elems @@ -28,5 +29,6 @@ class StructureSolc(Structure): elem.analyze(self.contract) self._elems[elem.name] = elem + self._elems_ordered.append(elem.name) self._elemsNotParsed = [] diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index 39b2334d6..968ae446a 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -479,6 +479,12 @@ def parse_expression(expression, caller_context): value = str(convert_subdenomination(value, expression['subdenomination'])) elif not value and value != "": value = '0x'+expression['hexValue'] + type = expression['typeDescriptions']['typeString'] + + # Length declaration for array was None until solc 0.5.5 + if type is None: + if expression['kind'] == 'number': + type = 'int_const' else: value = expression['attributes']['value'] if value: @@ -489,7 +495,15 @@ def parse_expression(expression, caller_context): # see https://solidity.readthedocs.io/en/v0.4.25/types.html?highlight=hex#hexadecimal-literals assert 'hexvalue' in expression['attributes'] value = '0x'+expression['attributes']['hexvalue'] - literal = Literal(value) + type = expression['attributes']['type'] + + if type.startswith('int_const '): + type = ElementaryType('uint256') + elif type.startswith('bool'): + type = ElementaryType('bool') + else: + type = ElementaryType('string') + literal = Literal(value, type) return literal elif name == 'Identifier': diff --git a/slither/solc_parsing/solidity_types/type_parsing.py b/slither/solc_parsing/solidity_types/type_parsing.py index 2b5057dda..22e8954ab 100644 --- a/slither/solc_parsing/solidity_types/type_parsing.py +++ b/slither/solc_parsing/solidity_types/type_parsing.py @@ -32,7 +32,7 @@ def _find_from_type_name(name, contract, contracts, structures, enums): if name_elementary in ElementaryTypeName: depth = name.count('[') if depth: - return ArrayType(ElementaryType(name_elementary), Literal(depth)) + return ArrayType(ElementaryType(name_elementary), Literal(depth, 'uint256')) else: return ElementaryType(name_elementary) # We first look for contract @@ -78,7 +78,7 @@ def _find_from_type_name(name, contract, contracts, structures, enums): depth+=1 var_type = next((st for st in all_structures if st.contract.name+"."+st.name == name_struct), None) if var_type: - return ArrayType(UserDefinedType(var_type), Literal(depth)) + return ArrayType(UserDefinedType(var_type), Literal(depth, 'uint256')) if not var_type: var_type = next((f for f in contract.functions if f.name == name), None) diff --git a/slither/utils/type.py b/slither/utils/type.py new file mode 100644 index 000000000..d5bca3720 --- /dev/null +++ b/slither/utils/type.py @@ -0,0 +1,31 @@ +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) + :param variable: + :return: list(Type) + """ + l = [] + if isinstance(variable.type, MappingType): + t = variable.type + _add_mapping_parameter(t, l) + + if isinstance(variable.type, ArrayType): + v = variable + _add_array_parameter(v.type, l) + + return l + + diff --git a/slither/visitors/expression/constants_folding.py b/slither/visitors/expression/constants_folding.py index bca1bfe04..26ce38906 100644 --- a/slither/visitors/expression/constants_folding.py +++ b/slither/visitors/expression/constants_folding.py @@ -20,8 +20,12 @@ def set_val(expression, val): class ConstantFolding(ExpressionVisitor): + def __init__(self, expression, type): + super(ConstantFolding, self).__init__(expression) + self._type = type + def result(self): - return Literal(int(get_val(self._expression))) + return Literal(int(get_val(self._expression)), self._type) def _post_identifier(self, expression): if not expression.value.is_constant: @@ -29,7 +33,7 @@ class ConstantFolding(ExpressionVisitor): expr = expression.value.expression # assumption that we won't have infinite loop if not isinstance(expr, Literal): - cf = ConstantFolding(expr) + cf = ConstantFolding(expr, self._type) expr = cf.result() set_val(expression, int(expr.value)) diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index a1c1c5e5d..c5f275749 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -173,7 +173,8 @@ class ExpressionToSlithIR(ExpressionVisitor): set_val(expression, val) def _post_literal(self, expression): - set_val(expression, Constant(expression.value)) + cst = Constant(expression.value, expression.type) + set_val(expression, cst) def _post_member_access(self, expression): expr = get(expression.expression) From d1dbef483d10b3102568e9db830615ef56f82617 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 6 May 2019 16:53:04 +0100 Subject: [PATCH 6/9] Improve too many digits detector. Fix travis test --- scripts/tests_generate_expected_json_5.sh | 32 +-- scripts/travis_test_4.sh | 1 - scripts/travis_test_5.sh | 2 + .../detectors/statements/too_many_digits.py | 84 +++++--- .../too_many_digits.too-many-digits.json | 196 ++++++++++++++++++ .../too_many_digits.too-many-digits.txt | 19 ++ tests/too_many_digits.sol | 2 +- 7 files changed, 288 insertions(+), 48 deletions(-) create mode 100644 tests/expected_json/too_many_digits.too-many-digits.json create mode 100644 tests/expected_json/too_many_digits.too-many-digits.txt diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh index c509acb48..b6107458e 100755 --- a/scripts/tests_generate_expected_json_5.sh +++ b/scripts/tests_generate_expected_json_5.sh @@ -19,18 +19,20 @@ generate_expected_json(){ sed "s|$CURRENT_PATH|$TRAVIS_PATH|g" "$output_filename" -i } -generate_expected_json tests/uninitialized-0.5.1.sol "uninitialized-state" -generate_expected_json tests/backdoor.sol "backdoor" -generate_expected_json tests/backdoor.sol "suicidal" -generate_expected_json tests/pragma.0.4.24.sol "pragma" -generate_expected_json tests/old_solc.sol.json "solc-version" -generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy-eth" -generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy" -generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage" -generate_expected_json tests/tx_origin-0.5.1.sol "tx-origin" -generate_expected_json tests/locked_ether-0.5.1.sol "locked-ether" -generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send" -generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly" -generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly" -generate_expected_json tests/constant-0.5.1.sol "constant-function" -generate_expected_json tests/incorrect_equality.sol "incorrect-equality" +#generate_expected_json tests/uninitialized-0.5.1.sol "uninitialized-state" +#generate_expected_json tests/backdoor.sol "backdoor" +#generate_expected_json tests/backdoor.sol "suicidal" +#generate_expected_json tests/pragma.0.4.24.sol "pragma" +#generate_expected_json tests/old_solc.sol.json "solc-version" +#generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy-eth" +#generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy" +#generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage" +#generate_expected_json tests/tx_origin-0.5.1.sol "tx-origin" +#generate_expected_json tests/locked_ether-0.5.1.sol "locked-ether" +#generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send" +#generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly" +#generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly" +#generate_expected_json tests/constant-0.5.1.sol "constant-function" +#generate_expected_json tests/incorrect_equality.sol "incorrect-equality" +#generate_expected_json tests/too_many_digits.sol "too-many-digits" + diff --git a/scripts/travis_test_4.sh b/scripts/travis_test_4.sh index e940ea86b..5150a92f6 100755 --- a/scripts/travis_test_4.sh +++ b/scripts/travis_test_4.sh @@ -102,4 +102,3 @@ test_slither tests/shadowing_builtin_symbols.sol "shadowing-builtin" test_slither tests/shadowing_local_variable.sol "shadowing-local" test_slither tests/solc_version_incorrect.sol "solc-version" test_slither tests/right_to_left_override.sol "rtlo" -test_slither tests/too_many_digits.sol "too-many-digits" diff --git a/scripts/travis_test_5.sh b/scripts/travis_test_5.sh index feb4f5a59..da5455db0 100755 --- a/scripts/travis_test_5.sh +++ b/scripts/travis_test_5.sh @@ -91,6 +91,8 @@ test_slither tests/constant-0.5.1.sol "constant-function" test_slither tests/unused_return.sol "unused-return" test_slither tests/timestamp.sol "timestamp" test_slither tests/incorrect_equality.sol "incorrect-equality" +test_slither tests/too_many_digits.sol "too-many-digits" + ### Test scripts diff --git a/slither/detectors/statements/too_many_digits.py b/slither/detectors/statements/too_many_digits.py index 6049c1371..145fef6bc 100644 --- a/slither/detectors/statements/too_many_digits.py +++ b/slither/detectors/statements/too_many_digits.py @@ -3,52 +3,74 @@ Module detecting numbers with too many digits. """ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.slithir.variables import Constant class TooManyDigits(AbstractDetector): """ Detect numbers with too many digits """ - ARGUMENT = 'too_many_digits' # slither will launch the detector with slither.py --too_many_digits - HELP = 'Numbers with too many digits (human readability risk)' - IMPACT = DetectorClassification.MEDIUM + ARGUMENT = 'too-many-digits' + HELP = 'Detect large digits usage' + IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.MEDIUM - WIKI = 'https://github.com/ethereum/web3.js/blob/0.15.0/lib/utils/utils.js' - WIKI_TITLE = 'too_many_digits' - WIKI_DESCRIPTION = 'Plugin too_many_digits' - WIKI_EXPLOIT_SCENARIO = 'An error or malicious intent may have caused a number with many digits to be different than the number originally intended. This kind of error is usually difficult to find visually.' - WIKI_RECOMMENDATION = 'The number used in the contract has too many digits. Try to use an Ether denomination instead' + WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits' + WIKI_TITLE = 'Too many digits' + WIKI_DESCRIPTION = 'Detect large digits that do not follow best practices' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract MyContract{ + uint 1_ether = 10000000000000000000; +``` + +While `1_ether` looks like `1 ether`, it is `10 ether`.''' + WIKI_RECOMMENDATION = ''' +Use: +- [Ether suffix](https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#ether-units), +- [Time suffix](https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#time-units) or, +- [The scientific notation](https://solidity.readthedocs.io/en/latest/types.html#rational-and-integer-literals) +''' + + @staticmethod + def _detect_too_many_digits(f): + ret = [] + for node in f.nodes: + # each node contains a list of IR instruction + for ir in node.irs: + # iterate over all the variables read by the IR + for read in ir.read: + # if the variable is a constant + if isinstance(read, Constant): + # read.value can return an int or a str. Convert it to str + value_as_str = read.original_value + line_of_code = str(node.expression) + if '00000' in value_as_str: + # Info to be printed + ret.append(node) + return ret def _detect(self): results = [] - from slither import Slither - from slither.slithir.variables import Constant - # iterate over all contracts for contract in self.slither.contracts_derived: # iterate over all functions for f in contract.functions: # iterate over all the nodes - for node in f.nodes: - # each node contains a list of IR instruction - for ir in node.irs: - # iterate over all the variables read by the IR - for read in ir.read: - # if the variable is a constant - if isinstance(read, Constant): - # read.value can return an int or a str. Convert it to str - value_as_str = str(read.value) - #name_as_str = str(read.name) - line_of_code = str(node.expression) - if '00000' in value_as_str: - # Info to be printed - info = 'In {}.{} ({}), too many digits used in expression:\n\t- {}\n\tPlease use the proper Ether denomination instead\n' - info = info.format(contract.name, f.name, f.source_mapping_str, line_of_code) - - # Add the result in result - json = self.generate_json_result(info) - self.add_function_to_json(f, json) - results.append(json) + ret = self._detect_too_many_digits(f) + if ret: + info = '{}.{} ({}) uses literals with too many digits:'.format(f.contract.name, + f.name, + f.source_mapping_str) + for node in ret: + info += '\n\t- {}'.format(node.expression) + info += '\n\tUse the proper denomination (ether-unit, time-unit,' + info += 'or the scientific notation\n' + + # Add the result in result + json = self.generate_json_result(info) + self.add_nodes_to_json(ret, json) + results.append(json) + return results diff --git a/tests/expected_json/too_many_digits.too-many-digits.json b/tests/expected_json/too_many_digits.too-many-digits.json new file mode 100644 index 000000000..96768b95e --- /dev/null +++ b/tests/expected_json/too_many_digits.too-many-digits.json @@ -0,0 +1,196 @@ +[ + { + "check": "too-many-digits", + "impact": "Informational", + "confidence": "Medium", + "description": "C.f (tests/too_many_digits.sol#9-15) uses literals with too many digits:\n\t- x1 = 0x000001\n\t- x2 = 0x0000000000001\n\t- x3 = 1000000000000000000\n\t- x4 = 100000\n\tUse the proper denomination (ether-unit, time-unit,or the scientific notation\n", + "elements": [ + { + "type": "expression", + "expression": "x1 = 0x000001", + "source_mapping": { + "start": 206, + "length": 18, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 10 + ], + "starting_column": 9, + "ending_column": 27 + } + }, + { + "type": "expression", + "expression": "x2 = 0x0000000000001", + "source_mapping": { + "start": 234, + "length": 25, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 11 + ], + "starting_column": 9, + "ending_column": 34 + } + }, + { + "type": "expression", + "expression": "x3 = 1000000000000000000", + "source_mapping": { + "start": 269, + "length": 29, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 12 + ], + "starting_column": 9, + "ending_column": 38 + } + }, + { + "type": "expression", + "expression": "x4 = 100000", + "source_mapping": { + "start": 308, + "length": 16, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 13 + ], + "starting_column": 9, + "ending_column": 25 + } + } + ] + }, + { + "check": "too-many-digits", + "impact": "Informational", + "confidence": "Medium", + "description": "C.h (tests/too_many_digits.sol#20-24) uses literals with too many digits:\n\t- x2 = 100000\n\tUse the proper denomination (ether-unit, time-unit,or the scientific notation\n", + "elements": [ + { + "type": "expression", + "expression": "x2 = 100000", + "source_mapping": { + "start": 509, + "length": 16, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 22 + ], + "starting_column": 9, + "ending_column": 25 + } + } + ] + }, + { + "check": "too-many-digits", + "impact": "Informational", + "confidence": "Medium", + "description": "C.i (tests/too_many_digits.sol#29-33) uses literals with too many digits:\n\t- x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000\n\t- x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000\n\t- x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000\n\t- x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000\n\t- x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000\n\tUse the proper denomination (ether-unit, time-unit,or the scientific notation\n", + "elements": [ + { + "type": "expression", + "expression": "x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000", + "source_mapping": { + "start": 749, + "length": 67, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 31 + ], + "starting_column": 9, + "ending_column": 76 + } + }, + { + "type": "expression", + "expression": "x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000", + "source_mapping": { + "start": 749, + "length": 67, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 31 + ], + "starting_column": 9, + "ending_column": 76 + } + }, + { + "type": "expression", + "expression": "x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000", + "source_mapping": { + "start": 749, + "length": 67, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 31 + ], + "starting_column": 9, + "ending_column": 76 + } + }, + { + "type": "expression", + "expression": "x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000", + "source_mapping": { + "start": 749, + "length": 67, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 31 + ], + "starting_column": 9, + "ending_column": 76 + } + }, + { + "type": "expression", + "expression": "x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000", + "source_mapping": { + "start": 749, + "length": 67, + "filename_used": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_relative": "tests/too_many_digits.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/too_many_digits.sol", + "filename_short": "tests/too_many_digits.sol", + "lines": [ + 31 + ], + "starting_column": 9, + "ending_column": 76 + } + } + ] + } +] \ No newline at end of file diff --git a/tests/expected_json/too_many_digits.too-many-digits.txt b/tests/expected_json/too_many_digits.too-many-digits.txt new file mode 100644 index 000000000..a80d381ca --- /dev/null +++ b/tests/expected_json/too_many_digits.too-many-digits.txt @@ -0,0 +1,19 @@ +INFO:Detectors: +C.f (tests/too_many_digits.sol#9-15) uses literals with too many digits: + - x1 = 0x000001 + - x2 = 0x0000000000001 + - x3 = 1000000000000000000 + - x4 = 100000 + Use the proper denomination (ether-unit, time-unit,or the scientific notation +C.h (tests/too_many_digits.sol#20-24) uses literals with too many digits: + - x2 = 100000 + Use the proper denomination (ether-unit, time-unit,or the scientific notation +C.i (tests/too_many_digits.sol#29-33) uses literals with too many digits: + - x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000 + - x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000 + - x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000 + - x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000 + - x2 = 1000000000000 + 10000000000000 + 100000000000000 + 1000000000000000 + 10000000000000000 + Use the proper denomination (ether-unit, time-unit,or the scientific notation +Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits +INFO:Slither:tests/too_many_digits.sol analyzed (1 contracts), 3 result(s) found diff --git a/tests/too_many_digits.sol b/tests/too_many_digits.sol index 409030e3d..84e930056 100644 --- a/tests/too_many_digits.sol +++ b/tests/too_many_digits.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.5.7; +pragma solidity ^0.5.1; contract C { uint balance; From cda075ac2f5f0562874617b6d12a110d250b9330 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 6 May 2019 17:01:44 +0100 Subject: [PATCH 7/9] Minor --- README.md | 1 + slither/detectors/statements/too_many_digits.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index cf7f577d8..880fd30e2 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,7 @@ Num | Detector | What it Detects | Impact | Confidence 30 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High 31 | `solc-version` | [Incorrect Solidity version (< 0.4.24 or complex pragma)](https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-version-of-solidity) | Informational | High 32 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variables) | Informational | High +33 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium [Contact us](https://www.trailofbits.com/contact/) to get access to additional detectors. diff --git a/slither/detectors/statements/too_many_digits.py b/slither/detectors/statements/too_many_digits.py index 145fef6bc..ed80e7467 100644 --- a/slither/detectors/statements/too_many_digits.py +++ b/slither/detectors/statements/too_many_digits.py @@ -11,24 +11,28 @@ class TooManyDigits(AbstractDetector): """ ARGUMENT = 'too-many-digits' - HELP = 'Detect large digits usage' + HELP = 'Conformance to numeric notation best practices' IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.MEDIUM WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits' WIKI_TITLE = 'Too many digits' - WIKI_DESCRIPTION = 'Detect large digits that do not follow best practices' + WIKI_DESCRIPTION = ''' +Literals with many digits are difficult to read and review. +''' WIKI_EXPLOIT_SCENARIO = ''' ```solidity contract MyContract{ uint 1_ether = 10000000000000000000; +} ``` -While `1_ether` looks like `1 ether`, it is `10 ether`.''' +While `1_ether` looks like `1 ether`, it is `10 ether`. As a result, its usage is likely to be incorrect. +''' WIKI_RECOMMENDATION = ''' Use: -- [Ether suffix](https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#ether-units), -- [Time suffix](https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#time-units) or, +- [Ether suffix](https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#ether-units) +- [Time suffix](https://solidity.readthedocs.io/en/latest/units-and-global-variables.html#time-units), or - [The scientific notation](https://solidity.readthedocs.io/en/latest/types.html#rational-and-integer-literals) ''' From 5e1eb423113d5524ad044f0b59e82dd700fa2593 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 6 May 2019 17:53:51 +0100 Subject: [PATCH 8/9] 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 9/9] 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