diff --git a/README.md b/README.md index e98264ae6c..abb7361966 100644 --- a/README.md +++ b/README.md @@ -55,24 +55,26 @@ Num | Detector | What it Detects | Impact | Confidence 14 | `constant-function` | [Constant functions changing the state](https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state) | Medium | Medium 15 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1) | Medium | Medium 16 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-usage-of-txorigin) | Medium | Medium -17 | `uninitialized-local` | [Uninitialized local variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables) | Medium | Medium -18 | `unused-return` | [Unused return values](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return) | Medium | Medium -19 | `shadowing-builtin` | [Built-in symbol shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#builtin-symbol-shadowing) | Low | High -20 | `shadowing-local` | [Local variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#local-variable-shadowing) | Low | High -21 | `calls-loop` | [Multiple calls in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/_edit#calls-inside-a-loop) | Low | Medium -22 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2) | Low | Medium -23 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp) | Low | Medium -24 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High -25 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Informational | High -26 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High -27 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High -28 | `external-function` | [Public function that could be declared as external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-as-external) | Informational | High -29 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High -30 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High -31 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High -32 | `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 -33 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variables) | Informational | High -34 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium +17 | `unchecked-lowlevel` | [Unchecked low-level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level) | Medium | Medium +18 | `unchecked-send` | [Unchecked send](https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-send) | Medium | Medium +19 | `uninitialized-local` | [Uninitialized local variables](https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables) | Medium | Medium +20 | `unused-return` | [Unused return values](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return) | Medium | Medium +21 | `shadowing-builtin` | [Built-in symbol shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#builtin-symbol-shadowing) | Low | High +22 | `shadowing-local` | [Local variables shadowing](https://github.com/crytic/slither/wiki/Detector-Documentation#local-variable-shadowing) | Low | High +23 | `calls-loop` | [Multiple calls in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation/_edit#calls-inside-a-loop) | Low | Medium +24 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2) | Low | Medium +25 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp) | Low | Medium +26 | `assembly` | [Assembly usage](https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage) | Informational | High +27 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Informational | High +28 | `deprecated-standards` | [Deprecated Solidity Standards](https://github.com/crytic/slither/wiki/Detector-Documentation#deprecated-standards) | Informational | High +29 | `erc20-indexed` | [Un-indexed ERC20 event parameters](https://github.com/crytic/slither/wiki/Detector-Documentation#unindexed-erc20-event-parameters) | Informational | High +30 | `external-function` | [Public function that could be declared as external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-as-external) | Informational | High +31 | `low-level-calls` | [Low level calls](https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls) | Informational | High +32 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions) | Informational | High +33 | `pragma` | [If different pragma directives are used](https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used) | Informational | High +34 | `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 +35 | `unused-state` | [Unused state variables](https://github.com/crytic/slither/wiki/Detector-Documentation#unused-state-variables) | Informational | High +36 | `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/scripts/tests_generate_expected_json_4.sh b/scripts/tests_generate_expected_json_4.sh index c87b181de8..4e1def80c9 100755 --- a/scripts/tests_generate_expected_json_4.sh +++ b/scripts/tests_generate_expected_json_4.sh @@ -55,3 +55,4 @@ generate_expected_json(){ #generate_expected_json tests/shadowing_local_variable.sol "shadowing-local" #generate_expected_json tests/solc_version_incorrect.sol "solc-version" #generate_expected_json tests/right_to_left_override.sol "rtlo" +#generate_expected_json tests/unchecked_lowlevel.sol "unchecked-lowlevel" diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh index f8857daf5f..db19580573 100755 --- a/scripts/tests_generate_expected_json_5.sh +++ b/scripts/tests_generate_expected_json_5.sh @@ -35,4 +35,6 @@ generate_expected_json(){ #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" +#generate_expected_json tests/unchecked_lowlevel-0.5.1.sol "unchecked-lowlevel" +#generate_expected_json tests/unchecked_send-0.5.1.sol "unchecked-send" diff --git a/scripts/travis_test_4.sh b/scripts/travis_test_4.sh index 171079d9e0..1b43f2a5de 100755 --- a/scripts/travis_test_4.sh +++ b/scripts/travis_test_4.sh @@ -69,6 +69,7 @@ test_slither(){ } +test_slither tests/unchecked_lowlevel.sol "unchecked-lowlevel" 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" diff --git a/scripts/travis_test_5.sh b/scripts/travis_test_5.sh index da5455db07..5f12d765d1 100755 --- a/scripts/travis_test_5.sh +++ b/scripts/travis_test_5.sh @@ -69,6 +69,8 @@ test_slither(){ } +test_slither tests/unchecked_lowlevel-0.5.1.sol "unchecked-lowlevel" +test_slither tests/unchecked_send-0.5.1.sol "unchecked-send" test_slither tests/uninitialized-0.5.1.sol "uninitialized-state" test_slither tests/backdoor.sol "backdoor" test_slither tests/backdoor.sol "suicidal" diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 6d0d94b3e6..9fd21ac838 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -34,5 +34,7 @@ from .statements.deprecated_calls import DeprecatedStandards from .source.rtlo import RightToLeftOverride from .statements.too_many_digits import TooManyDigits +from .operations.unchecked_low_level_return_values import UncheckedLowLevel +from .operations.unchecked_send_return_value import UncheckedSend # # diff --git a/slither/detectors/operations/unchecked_low_level_return_values.py b/slither/detectors/operations/unchecked_low_level_return_values.py new file mode 100644 index 0000000000..6f85600a71 --- /dev/null +++ b/slither/detectors/operations/unchecked_low_level_return_values.py @@ -0,0 +1,43 @@ +""" +Module detecting unused return values from low level +""" +from slither.detectors.abstract_detector import DetectorClassification +from .unused_return_values import UnusedReturnValues +from slither.slithir.operations import LowLevelCall + +class UncheckedLowLevel(UnusedReturnValues): + """ + If the return value of a send is not checked, it might lead to losing ether + """ + + ARGUMENT = 'unchecked-lowlevel' + HELP = 'Unchecked low-level calls' + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level' + + WIKI_TITLE = 'Unchecked low-level calls' + WIKI_DESCRIPTION = 'The return value of a low-level call is not checked.' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract MyConc{ + function my_func(address payable dst) public payable{ + dst.call.value(msg.value)(""); + } +} +``` +The return value of the low-level call is not checked. As a result if the callfailed, the ether will be locked in the contract. +If the low level is used to prevent blocking operations, consider logging failed calls. + ''' + + WIKI_RECOMMENDATION = 'Ensure that the return value of low-level call is checked or logged.' + + _txt_description = "low-level calls" + + def _is_instance(self, ir): + return isinstance(ir, LowLevelCall) + + + + diff --git a/slither/detectors/operations/unchecked_send_return_value.py b/slither/detectors/operations/unchecked_send_return_value.py new file mode 100644 index 0000000000..6c5a593242 --- /dev/null +++ b/slither/detectors/operations/unchecked_send_return_value.py @@ -0,0 +1,40 @@ +""" +Module detecting unused return values from send +""" + +from slither.detectors.abstract_detector import DetectorClassification +from .unused_return_values import UnusedReturnValues +from slither.slithir.operations import Send + +class UncheckedSend(UnusedReturnValues): + """ + If the return value of a send is not checked, it might lead to losing ether + """ + + ARGUMENT = 'unchecked-send' + HELP = 'Unchecked send' + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-send' + + WIKI_TITLE = 'Unchecked Send' + WIKI_DESCRIPTION = 'The return value of a send is not checked.' + WIKI_EXPLOIT_SCENARIO = ''' +```solidity +contract MyConc{ + function my_func(address payable dst) public payable{ + dst.send(msg.value); + } +} +``` +The return value of `send` is not checked. As a result if the send failed, the ether will be locked in the contract. +If `send` is used to prevent blocking operations, consider logging the failed sent. + ''' + + WIKI_RECOMMENDATION = 'Ensure that the return value of send is checked or logged.' + + _txt_description = "send calls" + + def _is_instance(self, ir): + return isinstance(ir, Send) \ No newline at end of file diff --git a/slither/detectors/operations/unused_return_values.py b/slither/detectors/operations/unused_return_values.py index 04e50398b8..6e521c8c85 100644 --- a/slither/detectors/operations/unused_return_values.py +++ b/slither/detectors/operations/unused_return_values.py @@ -2,9 +2,8 @@ Module detecting unused return values from external calls """ -from collections import defaultdict from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -from slither.slithir.operations.high_level_call import HighLevelCall +from slither.slithir.operations import HighLevelCall, InternalCall, InternalDynamicCall from slither.core.variables.state_variable import StateVariable class UnusedReturnValues(AbstractDetector): @@ -19,7 +18,6 @@ class UnusedReturnValues(AbstractDetector): WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return' - WIKI_TITLE = 'Unused return' WIKI_DESCRIPTION = 'The return value of an external call is not stored in a local or state variable.' WIKI_EXPLOIT_SCENARIO = ''' @@ -33,7 +31,12 @@ class UnusedReturnValues(AbstractDetector): ``` `MyConc` calls `add` of SafeMath, but does not store the result in `a`. As a result, the computation has no effect.''' - WIKI_RECOMMENDATION = 'Ensure that all the return values of the function calls are stored in a local or state variable.' + WIKI_RECOMMENDATION = 'Ensure that all the return values of the function calls are used.' + + _txt_description = "external calls" + + def _is_instance(self, ir): + return isinstance(ir, HighLevelCall) def detect_unused_return_values(self, f): """ @@ -47,7 +50,7 @@ def detect_unused_return_values(self, f): nodes_origin = {} for n in f.nodes: for ir in n.irs: - if isinstance(ir, HighLevelCall): + if self._is_instance(ir): # if a return value is stored in a state variable, it's ok if ir.lvalue and not isinstance(ir.lvalue, StateVariable): values_returned.append(ir.lvalue) @@ -68,11 +71,12 @@ def _detect(self): continue unused_return = self.detect_unused_return_values(f) if unused_return: - info = "{}.{} ({}) does not use the value returned by external calls:\n" + info = "{}.{} ({}) does not use the value returned by {}:\n" info = info.format(f.contract.name, f.name, - f.source_mapping_str) - for node in unused_return: + f.source_mapping_str, + self._txt_description) + for node in sorted(unused_return, key=lambda x:x.node_id): info += "\t-{} ({})\n".format(node.expression, node.source_mapping_str) json = self.generate_json_result(info) @@ -81,3 +85,4 @@ def _detect(self): results.append(json) return results + diff --git a/slither/slither.py b/slither/slither.py index 7b025952b6..617b8b81fc 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -55,7 +55,7 @@ def __init__(self, contract, **kwargs): crytic_compile = CryticCompile(contract, **kwargs) self._crytic_compile = crytic_compile except InvalidCompilation as e: - raise SlitherError('Invalid compilation: '+e) + raise SlitherError('Invalid compilation: \n'+str(e)) for path, ast in crytic_compile.asts.items(): self._parse_contracts_from_loaded_json(ast, path) self._add_source_code(path) @@ -160,7 +160,7 @@ def _check_common_things(self, thing_name, cls, base_cls, instances_list): ) ) - if any(isinstance(obj, cls) for obj in instances_list): + if any(type(obj) == cls for obj in instances_list): raise Exception( "You can't register {!r} twice.".format(cls) ) diff --git a/tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.json b/tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.json new file mode 100644 index 0000000000..581549f312 --- /dev/null +++ b/tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.json @@ -0,0 +1,77 @@ +{ + "success": true, + "error": null, + "results": [ + { + "check": "unchecked-lowlevel", + "impact": "Medium", + "confidence": "Medium", + "description": "MyConc.bad (tests/unchecked_lowlevel-0.5.1.sol#2-4) does not use the value returned by low-level calls:\n\t-dst.call.value(msg.value)() (tests/unchecked_lowlevel-0.5.1.sol#3)\n", + "elements": [ + { + "type": "function", + "name": "bad", + "source_mapping": { + "start": 21, + "length": 96, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel-0.5.1.sol", + "filename_relative": "tests/unchecked_lowlevel-0.5.1.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel-0.5.1.sol", + "filename_short": "tests/unchecked_lowlevel-0.5.1.sol", + "lines": [ + 2, + 3, + 4 + ], + "starting_column": 5, + "ending_column": 6 + }, + "contract": { + "type": "contract", + "name": "MyConc", + "source_mapping": { + "start": 0, + "length": 274, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel-0.5.1.sol", + "filename_relative": "tests/unchecked_lowlevel-0.5.1.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel-0.5.1.sol", + "filename_short": "tests/unchecked_lowlevel-0.5.1.sol", + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "expression", + "expression": "dst.call.value(msg.value)()", + "source_mapping": { + "start": 81, + "length": 29, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel-0.5.1.sol", + "filename_relative": "tests/unchecked_lowlevel-0.5.1.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel-0.5.1.sol", + "filename_short": "tests/unchecked_lowlevel-0.5.1.sol", + "lines": [ + 3 + ], + "starting_column": 9, + "ending_column": 38 + } + } + ] + } + ] +} \ No newline at end of file diff --git a/tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.txt b/tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.txt new file mode 100644 index 0000000000..ca447cf4ae --- /dev/null +++ b/tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.txt @@ -0,0 +1,5 @@ +INFO:Detectors: +MyConc.bad (tests/unchecked_lowlevel-0.5.1.sol#2-4) does not use the value returned by low-level calls: + -dst.call.value(msg.value)() (tests/unchecked_lowlevel-0.5.1.sol#3) +Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level +INFO:Slither:tests/unchecked_lowlevel-0.5.1.sol analyzed (1 contracts), 1 result(s) found diff --git a/tests/expected_json/unchecked_lowlevel.unchecked-lowlevel.json b/tests/expected_json/unchecked_lowlevel.unchecked-lowlevel.json new file mode 100644 index 0000000000..f02479fe5b --- /dev/null +++ b/tests/expected_json/unchecked_lowlevel.unchecked-lowlevel.json @@ -0,0 +1,76 @@ +{ + "success": true, + "error": null, + "results": [ + { + "check": "unchecked-lowlevel", + "impact": "Medium", + "confidence": "Medium", + "description": "MyConc.bad (tests/unchecked_lowlevel.sol#2-4) does not use the value returned by low-level calls:\n\t-dst.call.value(msg.value)() (tests/unchecked_lowlevel.sol#3)\n", + "elements": [ + { + "type": "function", + "name": "bad", + "source_mapping": { + "start": 21, + "length": 88, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel.sol", + "filename_relative": "tests/unchecked_lowlevel.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel.sol", + "filename_short": "tests/unchecked_lowlevel.sol", + "lines": [ + 2, + 3, + 4 + ], + "starting_column": 5, + "ending_column": 6 + }, + "contract": { + "type": "contract", + "name": "MyConc", + "source_mapping": { + "start": 0, + "length": 214, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel.sol", + "filename_relative": "tests/unchecked_lowlevel.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel.sol", + "filename_short": "tests/unchecked_lowlevel.sol", + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "expression", + "expression": "dst.call.value(msg.value)()", + "source_mapping": { + "start": 73, + "length": 29, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel.sol", + "filename_relative": "tests/unchecked_lowlevel.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_lowlevel.sol", + "filename_short": "tests/unchecked_lowlevel.sol", + "lines": [ + 3 + ], + "starting_column": 9, + "ending_column": 38 + } + } + ] + } + ] +} \ No newline at end of file diff --git a/tests/expected_json/unchecked_lowlevel.unchecked-lowlevel.txt b/tests/expected_json/unchecked_lowlevel.unchecked-lowlevel.txt new file mode 100644 index 0000000000..c221234be4 --- /dev/null +++ b/tests/expected_json/unchecked_lowlevel.unchecked-lowlevel.txt @@ -0,0 +1,5 @@ +INFO:Detectors: +MyConc.bad (tests/unchecked_lowlevel.sol#2-4) does not use the value returned by low-level calls: + -dst.call.value(msg.value)() (tests/unchecked_lowlevel.sol#3) +Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level +INFO:Slither:tests/unchecked_lowlevel.sol analyzed (1 contracts), 1 result(s) found diff --git a/tests/expected_json/unchecked_send-0.5.1.unchecked-send.json b/tests/expected_json/unchecked_send-0.5.1.unchecked-send.json new file mode 100644 index 0000000000..a0b1c74e77 --- /dev/null +++ b/tests/expected_json/unchecked_send-0.5.1.unchecked-send.json @@ -0,0 +1,84 @@ +{ + "success": true, + "error": null, + "results": [ + { + "check": "unchecked-send", + "impact": "Medium", + "confidence": "Medium", + "description": "MyConc.bad (tests/unchecked_send-0.5.1.sol#2-4) does not use the value returned by send calls:\n\t-dst.send(msg.value) (tests/unchecked_send-0.5.1.sol#3)\n", + "elements": [ + { + "type": "function", + "name": "bad", + "source_mapping": { + "start": 21, + "length": 86, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_send-0.5.1.sol", + "filename_relative": "tests/unchecked_send-0.5.1.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_send-0.5.1.sol", + "filename_short": "tests/unchecked_send-0.5.1.sol", + "lines": [ + 2, + 3, + 4 + ], + "starting_column": 5, + "ending_column": 6 + }, + "contract": { + "type": "contract", + "name": "MyConc", + "source_mapping": { + "start": 0, + "length": 419, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_send-0.5.1.sol", + "filename_relative": "tests/unchecked_send-0.5.1.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_send-0.5.1.sol", + "filename_short": "tests/unchecked_send-0.5.1.sol", + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18 + ], + "starting_column": 1, + "ending_column": 2 + } + } + }, + { + "type": "expression", + "expression": "dst.send(msg.value)", + "source_mapping": { + "start": 81, + "length": 19, + "filename_used": "/home/travis/build/crytic/slither/tests/unchecked_send-0.5.1.sol", + "filename_relative": "tests/unchecked_send-0.5.1.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/unchecked_send-0.5.1.sol", + "filename_short": "tests/unchecked_send-0.5.1.sol", + "lines": [ + 3 + ], + "starting_column": 9, + "ending_column": 28 + } + } + ] + } + ] +} \ No newline at end of file diff --git a/tests/expected_json/unchecked_send-0.5.1.unchecked-send.txt b/tests/expected_json/unchecked_send-0.5.1.unchecked-send.txt new file mode 100644 index 0000000000..7b7a029e05 --- /dev/null +++ b/tests/expected_json/unchecked_send-0.5.1.unchecked-send.txt @@ -0,0 +1,5 @@ +INFO:Detectors: +MyConc.bad (tests/unchecked_send-0.5.1.sol#2-4) does not use the value returned by send calls: + -dst.send(msg.value) (tests/unchecked_send-0.5.1.sol#3) +Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-send +INFO:Slither:tests/unchecked_send-0.5.1.sol analyzed (1 contracts), 1 result(s) found diff --git a/tests/unchecked_lowlevel-0.5.1.sol b/tests/unchecked_lowlevel-0.5.1.sol new file mode 100644 index 0000000000..99bd1f822c --- /dev/null +++ b/tests/unchecked_lowlevel-0.5.1.sol @@ -0,0 +1,11 @@ +contract MyConc{ + function bad(address payable dst) external payable{ + dst.call.value(msg.value)(""); + } + + function good(address payable dst) external payable{ + (bool ret, bytes memory _) = dst.call.value(msg.value)(""); + require(ret); + } + +} diff --git a/tests/unchecked_lowlevel.sol b/tests/unchecked_lowlevel.sol new file mode 100644 index 0000000000..da13139d68 --- /dev/null +++ b/tests/unchecked_lowlevel.sol @@ -0,0 +1,10 @@ +contract MyConc{ + function bad(address dst) external payable{ + dst.call.value(msg.value)(""); + } + + function good(address dst) external payable{ + require(dst.call.value(msg.value)()); + } + +} diff --git a/tests/unchecked_send-0.5.1.sol b/tests/unchecked_send-0.5.1.sol new file mode 100644 index 0000000000..8e27089382 --- /dev/null +++ b/tests/unchecked_send-0.5.1.sol @@ -0,0 +1,18 @@ +contract MyConc{ + function bad(address payable dst) external payable{ + dst.send(msg.value); + } + + function good(address payable dst) external payable{ + require(dst.send(msg.value)); + } + + function good2(address payable dst) external payable{ + bool res = dst.send(msg.value); + if(!res){ + emit Failed(dst, msg.value); + } + } + + event Failed(address, uint); +}