-
Notifications
You must be signed in to change notification settings - Fork 988
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #230 from crytic/dev-uncheck-return
Improve unused return value + add unchecked send/low-level calls detector
- Loading branch information
Showing
19 changed files
with
417 additions
and
28 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
slither/detectors/operations/unchecked_low_level_return_values.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
|
|
||
|
|
||
|
|
||
|
|
40 changes: 40 additions & 0 deletions
40
slither/detectors/operations/unchecked_send_return_value.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
77 changes: 77 additions & 0 deletions
77
tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
5 changes: 5 additions & 0 deletions
5
tests/expected_json/unchecked_lowlevel-0.5.1.unchecked-lowlevel.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| INFO:Detectors:[93m | ||
| 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[0m | ||
| INFO:Slither:tests/unchecked_lowlevel-0.5.1.sol analyzed (1 contracts), 1 result(s) found |
Oops, something went wrong.