From 1d067def92223ac4322e1453b8a1db1bb0918011 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 25 Nov 2019 16:25:12 +0100 Subject: [PATCH 1/3] Fix FPs in reentrancy-events --- scripts/tests_generate_expected_json_5.sh | 1 + scripts/travis_test_5.sh | 1 + slither/detectors/reentrancy/reentrancy.py | 9 +++------ 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh index 620fc0b0c..aac7a2428 100755 --- a/scripts/tests_generate_expected_json_5.sh +++ b/scripts/tests_generate_expected_json_5.sh @@ -27,6 +27,7 @@ generate_expected_json(){ #generate_expected_json tests/backdoor.sol "suicidal" #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-events.sol "reentrancy-events" #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" diff --git a/scripts/travis_test_5.sh b/scripts/travis_test_5.sh index 4b3928b5c..92163a0f7 100755 --- a/scripts/travis_test_5.sh +++ b/scripts/travis_test_5.sh @@ -78,6 +78,7 @@ test_slither tests/backdoor.sol "backdoor" test_slither tests/backdoor.sol "suicidal" test_slither tests/old_solc.sol.json "solc-version" test_slither tests/reentrancy-0.5.1.sol "reentrancy-eth" +test_slither tests/reentrancy-0.5.1-events.sol "reentrancy-events" test_slither tests/tx_origin-0.5.1.sol "tx-origin" test_slither tests/unused_state.sol "unused-state" test_slither tests/locked_ether-0.5.1.sol "locked-ether" diff --git a/slither/detectors/reentrancy/reentrancy.py b/slither/detectors/reentrancy/reentrancy.py index d46510f9d..fb1578e73 100644 --- a/slither/detectors/reentrancy/reentrancy.py +++ b/slither/detectors/reentrancy/reentrancy.py @@ -87,7 +87,7 @@ class Reentrancy(AbstractDetector): # calls returns the list of calls that can callback # read returns the variable read # read_prior_calls returns the variable read prior a call - fathers_context = {'send_eth': set(), 'calls': set(), 'read': set(), 'read_prior_calls': {}, 'events': set()} + fathers_context = {'send_eth': set(), 'calls': set(), 'read': set(), 'read_prior_calls': {}} for father in node.fathers: if self.KEY in father.context: @@ -97,7 +97,6 @@ class Reentrancy(AbstractDetector): fathers_context['read'] |= set(father.context[self.KEY]['read']) fathers_context['read_prior_calls'] = union_dict(fathers_context['read_prior_calls'], father.context[self.KEY]['read_prior_calls']) - fathers_context['events'] |= set(father.context[self.KEY]['events']) # Exclude path that dont bring further information if node in self.visited_all_paths: @@ -106,8 +105,7 @@ class Reentrancy(AbstractDetector): if fathers_context['read'].issubset(self.visited_all_paths[node]['read']): if dict_are_equal(self.visited_all_paths[node]['read_prior_calls'], fathers_context['read_prior_calls']): - if fathers_context['events'].issubset(self.visited_all_paths[node]['events']): - return + return else: self.visited_all_paths[node] = {'send_eth': set(), 'calls': set(), 'read': set(), 'read_prior_calls': {}, 'events': set()} @@ -117,7 +115,6 @@ class Reentrancy(AbstractDetector): self.visited_all_paths[node]['read'] |= fathers_context['read'] self.visited_all_paths[node]['read_prior_calls'] = union_dict(self.visited_all_paths[node]['read_prior_calls'], fathers_context['read_prior_calls']) - self.visited_all_paths[node]['events'] |= fathers_context['events'] node.context[self.KEY] = fathers_context @@ -147,7 +144,7 @@ class Reentrancy(AbstractDetector): node.context[self.KEY]['read'] |= state_vars_read - node.context[self.KEY]['events'] |= set([ir for ir in node.irs if isinstance(ir, EventCall)]) + node.context[self.KEY]['events'] = set([ir for ir in node.irs if isinstance(ir, EventCall)]) sons = node.sons if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]: From 14ec175f81a6f9ba62f9dedddbe506aa4bc92b04 Mon Sep 17 00:00:00 2001 From: Josselin Date: Thu, 28 Nov 2019 11:55:36 +0100 Subject: [PATCH 2/3] Add missing test files --- ...trancy-0.5.1-events.reentrancy-events.json | 151 ++++++++++++++++++ ...ntrancy-0.5.1-events.reentrancy-events.txt | 9 ++ tests/reentrancy-0.5.1-events.sol | 24 +++ 3 files changed, 184 insertions(+) create mode 100644 tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.json create mode 100644 tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.txt create mode 100644 tests/reentrancy-0.5.1-events.sol diff --git a/tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.json b/tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.json new file mode 100644 index 000000000..58cf6a067 --- /dev/null +++ b/tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.json @@ -0,0 +1,151 @@ +{ + "success": true, + "error": null, + "results": { + "detectors": [ + { + "elements": [ + { + "type": "function", + "name": "bug", + "source_mapping": { + "start": 86, + "length": 68, + "filename_used": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_relative": "tests/reentrancy-0.5.1-events.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_short": "tests/reentrancy-0.5.1-events.sol", + "is_dependency": false, + "lines": [ + 14, + 15, + 16, + 17 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Test", + "source_mapping": { + "start": 51, + "length": 193, + "filename_used": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_relative": "tests/reentrancy-0.5.1-events.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_short": "tests/reentrancy-0.5.1-events.sol", + "is_dependency": false, + "lines": [ + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "bug(C)" + } + }, + { + "type": "node", + "name": "c.f()", + "source_mapping": { + "start": 120, + "length": 5, + "filename_used": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_relative": "tests/reentrancy-0.5.1-events.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_short": "tests/reentrancy-0.5.1-events.sol", + "is_dependency": false, + "lines": [ + 15 + ], + "starting_column": 9, + "ending_column": 14 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "bug", + "source_mapping": { + "start": 86, + "length": 68, + "filename_used": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_relative": "tests/reentrancy-0.5.1-events.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_short": "tests/reentrancy-0.5.1-events.sol", + "is_dependency": false, + "lines": [ + 14, + 15, + 16, + 17 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Test", + "source_mapping": { + "start": 51, + "length": 193, + "filename_used": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_relative": "tests/reentrancy-0.5.1-events.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/reentrancy-0.5.1-events.sol", + "filename_short": "tests/reentrancy-0.5.1-events.sol", + "is_dependency": false, + "lines": [ + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "bug(C)" + } + } + }, + "additional_fields": { + "underlying_type": "external_calls" + } + } + ], + "description": "Reentrancy in Test.bug(C) (tests/reentrancy-0.5.1-events.sol#14-17):\n\tExternal calls:\n\t- c.f() (tests/reentrancy-0.5.1-events.sol#15)\n\tEvent emitted after the call(s):\n\t- E() (tests/reentrancy-0.5.1-events.sol#16)\n", + "markdown": "Reentrancy in [Test.bug(C)](tests/reentrancy-0.5.1-events.sol#L14-L17):\n\tExternal calls:\n\t- [c.f()](tests/reentrancy-0.5.1-events.sol#L15)\n\tEvent emitted after the call(s):\n\t- [E()](tests/reentrancy-0.5.1-events.sol#L16)\n", + "id": "9654da7d8b8d85c90bc2ee1ddaea365f98f14d9981149b354f8a3d84f98ea576", + "check": "reentrancy-events", + "impact": "Low", + "confidence": "Medium" + } + ] + } +} \ No newline at end of file diff --git a/tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.txt b/tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.txt new file mode 100644 index 000000000..c42c64ee0 --- /dev/null +++ b/tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.txt @@ -0,0 +1,9 @@ + +Reentrancy in Test.bug(C) (tests/reentrancy-0.5.1-events.sol#14-17): + External calls: + - c.f() (tests/reentrancy-0.5.1-events.sol#15) + Event emitted after the call(s): + - E() (tests/reentrancy-0.5.1-events.sol#16) +Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-3 +tests/reentrancy-0.5.1-events.sol analyzed (2 contracts with 1 detectors), 1 result(s) found +Use https://crytic.io/ to get access to additional detectors and Github integration diff --git a/tests/reentrancy-0.5.1-events.sol b/tests/reentrancy-0.5.1-events.sol new file mode 100644 index 000000000..f67409d45 --- /dev/null +++ b/tests/reentrancy-0.5.1-events.sol @@ -0,0 +1,24 @@ +contract C{ + + + function f() public{ + + } + +} + + +contract Test{ + event E(); + + function bug(C c) public{ + c.f(); + emit E(); + } + + function ok(C c) public{ + emit E(); + c.f(); + c.f(); + } +} From 146cc446b392d627c9180cf8a6904881f7aaec9a Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 9 Dec 2019 15:47:05 +0100 Subject: [PATCH 3/3] Update etherscan test --- scripts/travis_test_etherscan.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/travis_test_etherscan.sh b/scripts/travis_test_etherscan.sh index 3bdb49107..57b026cc5 100755 --- a/scripts/travis_test_etherscan.sh +++ b/scripts/travis_test_etherscan.sh @@ -18,7 +18,7 @@ fi slither rinkeby:0xFe05820C5A92D9bc906D4A46F662dbeba794d3b7 --solc "./solc-0.4.25" -if [ $? -ne 71 ] +if [ $? -ne 70 ] then echo "Etherscan test failed" exit -1