From df1f475a223ca35495b976e41ab418ac4ad3c07d Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Thu, 3 Jan 2019 13:13:35 +0100
Subject: [PATCH 1/4] Improve reentrancy

---
 slither/__main__.py                           |  4 +--
 .../{reentrancy.py => reentrancy_eth.py}      | 26 ++++++++++++-------
 tests/reentrancy.sol                          | 21 +++++++++++++++
 3 files changed, 40 insertions(+), 11 deletions(-)
 rename slither/detectors/reentrancy/{reentrancy.py => reentrancy_eth.py} (92%)

diff --git a/slither/__main__.py b/slither/__main__.py
index 3be86abf6..51f11975f 100644
--- a/slither/__main__.py
+++ b/slither/__main__.py
@@ -117,7 +117,7 @@ def get_detectors_and_printers():
     from slither.detectors.functions.arbitrary_send import ArbitrarySend
     from slither.detectors.functions.suicidal import Suicidal
     from slither.detectors.functions.complex_function import ComplexFunction
-    from slither.detectors.reentrancy.reentrancy import Reentrancy
+    from slither.detectors.reentrancy.reentrancy_eth import ReentrancyEth
     from slither.detectors.variables.unused_state_variables import UnusedStateVars
     from slither.detectors.variables.possible_const_state_variables import ConstCandidateStateVars
     from slither.detectors.statements.tx_origin import TxOrigin
@@ -140,7 +140,7 @@ def get_detectors_and_printers():
                  UninitializedLocalVars,
                  ConstantPragma,
                  OldSolc,
-                 Reentrancy,
+                 ReentrancyEth,
                  LockedEther,
                  ArbitrarySend,
                  Suicidal,
diff --git a/slither/detectors/reentrancy/reentrancy.py b/slither/detectors/reentrancy/reentrancy_eth.py
similarity index 92%
rename from slither/detectors/reentrancy/reentrancy.py
rename to slither/detectors/reentrancy/reentrancy_eth.py
index cdfafe46b..cb5852e05 100644
--- a/slither/detectors/reentrancy/reentrancy.py
+++ b/slither/detectors/reentrancy/reentrancy_eth.py
@@ -15,15 +15,15 @@ from slither.slithir.operations import (HighLevelCall, LowLevelCall,
                                         LibraryCall,
                                         Send, Transfer)
 
-class Reentrancy(AbstractDetector):
+class ReentrancyEth(AbstractDetector):
     ARGUMENT = 'reentrancy'
-    HELP = 'Reentrancy vulnerabilities'
+    HELP = 'Reentrancy vulnerabilities (theft of ethers)'
     IMPACT = DetectorClassification.HIGH
     CONFIDENCE = DetectorClassification.MEDIUM
 
     WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities'
 
-    key = 'REENTRANCY'
+    key = 'REENTRANCY-ETHERS'
 
     @staticmethod
     def _can_callback(node):
@@ -55,7 +55,7 @@ class Reentrancy(AbstractDetector):
                     return True
         return False
 
-    def _check_on_call_returned(self, node):
+    def _filter_if(self, node):
         """
             Check if the node is a condtional node where
             there is an external call checked
@@ -69,7 +69,7 @@ class Reentrancy(AbstractDetector):
         return isinstance(node.expression, UnaryOperation)\
             and node.expression.type == UnaryOperationType.BANG
 
-    def _explore(self, node, visited):
+    def _explore(self, node, visited, skip_father=None):
         """
             Explore the CFG and look for re-entrancy
             Heuristic: There is a re-entrancy if a state variable is written
@@ -93,8 +93,8 @@ class Reentrancy(AbstractDetector):
 
         for father in node.fathers:
             if self.key in father.context:
-                fathers_context['send_eth'] += father.context[self.key]['send_eth']
-                fathers_context['calls'] += father.context[self.key]['calls']
+                fathers_context['send_eth'] += [s for s in father.context[self.key]['send_eth'] if s!=skip_father]
+                fathers_context['calls'] += [c for c in father.context[self.key]['calls'] if c!=skip_father]
                 fathers_context['read'] += father.context[self.key]['read']
 
         # Exclude path that dont bring further information
@@ -147,8 +147,16 @@ class Reentrancy(AbstractDetector):
             self.result[finding_key] = list(set(self.result[finding_key] + finding_vars))
 
         sons = node.sons
-        if contains_call and self._check_on_call_returned(node):
-            sons = sons[1:]
+        if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]:
+            if self._filter_if(node):
+                son = sons[0]
+                self._explore(son, visited, node)
+                sons = sons[1:]
+            else:
+                son = sons[1]
+                self._explore(son, visited, node)
+                sons = [sons[0]]
+
 
         for son in sons:
             self._explore(son, visited)
diff --git a/tests/reentrancy.sol b/tests/reentrancy.sol
index 9f79587a2..077dd57fc 100644
--- a/tests/reentrancy.sol
+++ b/tests/reentrancy.sol
@@ -48,4 +48,25 @@ contract Reentrancy {
             userBalance[msg.sender] = amount;
         }
     }   
+    function withdrawBalance_fixed_4() public{
+        // The state can be changed
+        // But it is fine, as it can only occur if the transaction fails 
+        uint amount = userBalance[msg.sender];
+        userBalance[msg.sender] = 0;
+        if( (msg.sender.call.value(amount)() ) ){
+            return;
+        }
+        else{
+            userBalance[msg.sender] = amount;
+        }
+    }   
+
+    function withdrawBalance_nested() public{
+        uint amount = userBalance[msg.sender];
+        if( ! (msg.sender.call.value(amount/2)() ) ){
+            msg.sender.call.value(amount/2)();
+            userBalance[msg.sender] = 0;
+        }
+    }   
+
 }

From 8828093f1a42acbee2b7b65973b925ffd55fc32b Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Thu, 3 Jan 2019 13:29:25 +0100
Subject: [PATCH 2/4] Update travis testcase

---
 scripts/tests_generate_expected_json_4.sh     | 48 +++++++++----------
 scripts/tests_generate_expected_json_5.sh     | 14 +++---
 .../expected_json/reentrancy.reentrancy.json  |  2 +-
 3 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/scripts/tests_generate_expected_json_4.sh b/scripts/tests_generate_expected_json_4.sh
index 5bedd2f69..0c3e379be 100755
--- a/scripts/tests_generate_expected_json_4.sh
+++ b/scripts/tests_generate_expected_json_4.sh
@@ -14,28 +14,28 @@ generate_expected_json(){
 
 }
 
-generate_expected_json tests/uninitialized.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/uninitialized.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.sol "reentrancy"
-generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
-generate_expected_json tests/tx_origin.sol "tx-origin"
-generate_expected_json tests/unused_state.sol "unused-state"
-generate_expected_json tests/locked_ether.sol "locked-ether"
-generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
-generate_expected_json tests/inline_assembly_contract.sol "assembly"
-generate_expected_json tests/inline_assembly_library.sol "assembly"
-generate_expected_json tests/low_level_calls.sol "low-level-calls"
-generate_expected_json tests/const_state_variables.sol "constable-states"
-generate_expected_json tests/external_function.sol "external-function"
-generate_expected_json tests/naming_convention.sol "naming-convention"
-generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
-generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
-generate_expected_json tests/constant.sol "constant-function"
-generate_expected_json tests/unused_return.sol "unused-return"
-generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
-generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
-generate_expected_json tests/timestamp.sol "timestamp"
-generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
+#generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
+#generate_expected_json tests/tx_origin.sol "tx-origin"
+#generate_expected_json tests/unused_state.sol "unused-state"
+#generate_expected_json tests/locked_ether.sol "locked-ether"
+#generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
+#generate_expected_json tests/inline_assembly_contract.sol "assembly"
+#generate_expected_json tests/inline_assembly_library.sol "assembly"
+#generate_expected_json tests/low_level_calls.sol "low-level-calls"
+#generate_expected_json tests/const_state_variables.sol "constable-states"
+#generate_expected_json tests/external_function.sol "external-function"
+#generate_expected_json tests/naming_convention.sol "naming-convention"
+#generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
+#generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
+#generate_expected_json tests/constant.sol "constant-function"
+#generate_expected_json tests/unused_return.sol "unused-return"
+#generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
+#generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
+#generate_expected_json tests/timestamp.sol "timestamp"
+#generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh
index a6b0feea2..2e4fc6bac 100755
--- a/scripts/tests_generate_expected_json_5.sh
+++ b/scripts/tests_generate_expected_json_5.sh
@@ -14,16 +14,16 @@ generate_expected_json(){
 
 }
 
-generate_expected_json tests/uninitialized-0.5.1.sol "uninitialized-state"
+#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"
 #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/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"
diff --git a/tests/expected_json/reentrancy.reentrancy.json b/tests/expected_json/reentrancy.reentrancy.json
index 151c600f8..e9e7451e0 100644
--- a/tests/expected_json/reentrancy.reentrancy.json
+++ b/tests/expected_json/reentrancy.reentrancy.json
@@ -1 +1 @@
-[{"check": "reentrancy", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance (tests/reentrancy.sol#14-21):\n\tExternal calls:\n\t- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17-19)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#20)\n", "elements": [{"type": "function", "name": "withdrawBalance", "source_mapping": {"start": 299, "length": 314, "filename": "tests/reentrancy.sol", "lines": [14, 15, 16, 17, 18, 19, 20, 21]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 26, "length": 1678, "filename": "tests/reentrancy.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51]}}}, {"type": "external_calls", "expression": "! (msg.sender.call.value(userBalance[msg.sender])())", "source_mapping": {"start": 478, "length": 92, "filename": "tests/reentrancy.sol", "lines": [17, 18, 19]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 579, "length": 27, "filename": "tests/reentrancy.sol", "lines": [20]}}]}]
\ No newline at end of file
+[{"check": "reentrancy", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance (tests/reentrancy.sol#14-21):\n\tExternal calls:\n\t- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17-19)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#20)\n", "elements": [{"type": "function", "name": "withdrawBalance", "source_mapping": {"start": 299, "length": 314, "filename": "tests/reentrancy.sol", "lines": [14, 15, 16, 17, 18, 19, 20, 21]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 26, "length": 2334, "filename": "tests/reentrancy.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72]}}}, {"type": "external_calls", "expression": "! (msg.sender.call.value(userBalance[msg.sender])())", "source_mapping": {"start": 478, "length": 92, "filename": "tests/reentrancy.sol", "lines": [17, 18, 19]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 579, "length": 27, "filename": "tests/reentrancy.sol", "lines": [20]}}]}, {"check": "reentrancy", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance_nested (tests/reentrancy.sol#64-70):\n\tExternal calls:\n\t- msg.sender.call.value(amount / 2)() (tests/reentrancy.sol#67)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#68)\n", "elements": [{"type": "function", "name": "withdrawBalance_nested", "source_mapping": {"start": 2108, "length": 246, "filename": "tests/reentrancy.sol", "lines": [64, 65, 66, 67, 68, 69, 70]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 26, "length": 2334, "filename": "tests/reentrancy.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72]}}}, {"type": "external_calls", "expression": "msg.sender.call.value(amount / 2)()", "source_mapping": {"start": 2263, "length": 33, "filename": "tests/reentrancy.sol", "lines": [67]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 2310, "length": 27, "filename": "tests/reentrancy.sol", "lines": [68]}}]}]
\ No newline at end of file

From 51f3798626728a529f01a8fd6d3b7fd5df71ce2e Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Thu, 3 Jan 2019 13:52:18 +0100
Subject: [PATCH 3/4] Split reentrancy detector into three variants: -
 reentrancy-eth: theft of ether, read before write - reentrancy-no-eth: no
 theft of ether, read before write - reentrancy-benign: no theft of ether, no
 read before write

---
 README.md                                     |  31 +--
 scripts/tests_generate_expected_json_4.sh     |   2 +-
 scripts/tests_generate_expected_json_5.sh     |   2 +-
 scripts/travis_test_4.sh                      |   2 +-
 scripts/travis_test_5.sh                      |   2 +-
 slither/__main__.py                           |   4 +
 .../detectors/reentrancy/reentrancy_benign.py | 216 ++++++++++++++++++
 .../detectors/reentrancy/reentrancy_eth.py    |   4 +-
 .../reentrancy_read_before_write.py           | 216 ++++++++++++++++++
 .../reentrancy-0.5.1.reentrancy-eth.json      |   1 +
 .../reentrancy-0.5.1.reentrancy.json          |   1 -
 .../reentrancy.reentrancy-eth.json            |   1 +
 .../expected_json/reentrancy.reentrancy.json  |   1 -
 13 files changed, 460 insertions(+), 23 deletions(-)
 create mode 100644 slither/detectors/reentrancy/reentrancy_benign.py
 create mode 100644 slither/detectors/reentrancy/reentrancy_read_before_write.py
 create mode 100644 tests/expected_json/reentrancy-0.5.1.reentrancy-eth.json
 delete mode 100644 tests/expected_json/reentrancy-0.5.1.reentrancy.json
 create mode 100644 tests/expected_json/reentrancy.reentrancy-eth.json
 delete mode 100644 tests/expected_json/reentrancy.reentrancy.json

diff --git a/README.md b/README.md
index a28d3aad1..846f88f12 100644
--- a/README.md
+++ b/README.md
@@ -59,24 +59,25 @@ Num | Detector | What it Detects | Impact | Confidence
 4 | `uninitialized-storage` | [Uninitialized storage variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-storage-variables) | High | High
 5 | `arbitrary-send` | [Functions that send ether to arbitrary destinations](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#functions-that-send-ether-to-arbitrary-destinations) | High | Medium
 6 | `controlled-delegatecall` | [Controlled delegatecall destination](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#controlled-delegatecall) | High | Medium
-7 | `reentrancy` | [Reentrancy vulnerabilities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | High | Medium
+7 | `reentrancy-eth` | [Reentrancy vulnerabilities (theft of ethers)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | High | Medium
 8 | `locked-ether` | [Contracts that lock ether](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#contracts-that-lock-ether) | Medium | High
 9 | `shadowing-abstract` | [State variables shadowing from abstract contracts](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variable-shadowing-from-abstract-contracts) | Medium | High
 10 | `constant-function` | [Constant functions changing the state](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#constant-functions-changing-the-state) | Medium | Medium
-11 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin) | Medium | Medium
-12 | `uninitialized-local` | [Uninitialized local variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-local-variables) | Medium | Medium
-13 | `unused-return` | [Unused return values](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-return) | Medium | Medium
-14 | `calls-loop` | [Multiple calls in a loop](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#calls-inside-a-loop) | Low | Medium
-15 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#block-timestamp) | Low | Medium
-16 | `assembly` | [Assembly usage](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage) | Informational | High
-17 | `constable-states` | [State variables that could be declared constant](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant) | Informational | High
-18 | `external-function` | [Public function that could be declared as external](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#public-function-that-could-be-declared-as-external) | Informational | High
-19 | `low-level-calls` | [Low level calls](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls) | Informational | High
-20 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions) | Informational | High
-21 | `pragma` | [If different pragma directives are used](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#different-pragma-directives-are-used) | Informational | High
-22 | `solc-version` | [Old versions of Solidity (< 0.4.23)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity) | Informational | High
-23 | `unused-state` | [Unused state variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-state-variables) | Informational | High
-
+11 | `reentrancy-no-eth` | [Reentrancy vulnerabilities (no theft of ethers)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | Medium | Medium
+12 | `tx-origin` | [Dangerous usage of `tx.origin`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#dangerous-usage-of-txorigin) | Medium | Medium
+13 | `uninitialized-local` | [Uninitialized local variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#uninitialized-local-variables) | Medium | Medium
+14 | `unused-return` | [Unused return values](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-return) | Medium | Medium
+15 | `calls-loop` | [Multiple calls in a loop](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description/_edit#calls-inside-a-loop) | Low | Medium
+16 | `reentrancy-benign` | [Benign reentrancy vulnerabilities](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities) | Low | Medium
+17 | `timestamp` | [Dangerous usage of `block.timestamp`](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#block-timestamp) | Low | Medium
+18 | `assembly` | [Assembly usage](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#assembly-usage) | Informational | High
+19 | `constable-states` | [State variables that could be declared constant](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant) | Informational | High
+20 | `external-function` | [Public function that could be declared as external](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#public-function-that-could-be-declared-as-external) | Informational | High
+21 | `low-level-calls` | [Low level calls](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#low-level-calls) | Informational | High
+22 | `naming-convention` | [Conformance to Solidity naming conventions](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#conformance-to-solidity-naming-conventions) | Informational | High
+23 | `pragma` | [If different pragma directives are used](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#different-pragma-directives-are-used) | Informational | High
+24 | `solc-version` | [Old versions of Solidity (< 0.4.23)](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#old-versions-of-solidity) | Informational | High
+25 | `unused-state` | [Unused state variables](https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#unused-state-variables) | Informational | High
 
 
 [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 0c3e379be..fb87dfc9d 100755
--- a/scripts/tests_generate_expected_json_4.sh
+++ b/scripts/tests_generate_expected_json_4.sh
@@ -19,7 +19,7 @@ generate_expected_json(){
 #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.sol "reentrancy"
+generate_expected_json tests/reentrancy.sol "reentrancy-eth"
 #generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
 #generate_expected_json tests/tx_origin.sol "tx-origin"
 #generate_expected_json tests/unused_state.sol "unused-state"
diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh
index 2e4fc6bac..0bae9b237 100755
--- a/scripts/tests_generate_expected_json_5.sh
+++ b/scripts/tests_generate_expected_json_5.sh
@@ -19,7 +19,7 @@ generate_expected_json(){
 #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"
+generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy-eth"
 #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"
diff --git a/scripts/travis_test_4.sh b/scripts/travis_test_4.sh
index 6395537a8..efc71b457 100755
--- a/scripts/travis_test_4.sh
+++ b/scripts/travis_test_4.sh
@@ -70,7 +70,7 @@ test_slither tests/backdoor.sol "backdoor"
 test_slither tests/backdoor.sol "suicidal"
 test_slither tests/pragma.0.4.24.sol "pragma"
 test_slither tests/old_solc.sol.json "solc-version"
-test_slither tests/reentrancy.sol "reentrancy"
+test_slither tests/reentrancy.sol "reentrancy-eth"
 test_slither tests/uninitialized_storage_pointer.sol "uninitialized-storage"
 test_slither tests/tx_origin.sol "tx-origin"
 test_slither tests/unused_state.sol "unused-state"
diff --git a/scripts/travis_test_5.sh b/scripts/travis_test_5.sh
index d7af73720..1bc0d9206 100755
--- a/scripts/travis_test_5.sh
+++ b/scripts/travis_test_5.sh
@@ -69,7 +69,7 @@ test_slither tests/uninitialized-0.5.1.sol "uninitialized-state"
 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"
+test_slither tests/reentrancy-0.5.1.sol "reentrancy-eth"
 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/__main__.py b/slither/__main__.py
index 51f11975f..6d484ab30 100644
--- a/slither/__main__.py
+++ b/slither/__main__.py
@@ -117,6 +117,8 @@ def get_detectors_and_printers():
     from slither.detectors.functions.arbitrary_send import ArbitrarySend
     from slither.detectors.functions.suicidal import Suicidal
     from slither.detectors.functions.complex_function import ComplexFunction
+    from slither.detectors.reentrancy.reentrancy_benign import ReentrancyBenign
+    from slither.detectors.reentrancy.reentrancy_read_before_write import ReentrancyReadBeforeWritten
     from slither.detectors.reentrancy.reentrancy_eth import ReentrancyEth
     from slither.detectors.variables.unused_state_variables import UnusedStateVars
     from slither.detectors.variables.possible_const_state_variables import ConstCandidateStateVars
@@ -140,6 +142,8 @@ def get_detectors_and_printers():
                  UninitializedLocalVars,
                  ConstantPragma,
                  OldSolc,
+                 ReentrancyBenign,
+                 ReentrancyReadBeforeWritten,
                  ReentrancyEth,
                  LockedEther,
                  ArbitrarySend,
diff --git a/slither/detectors/reentrancy/reentrancy_benign.py b/slither/detectors/reentrancy/reentrancy_benign.py
new file mode 100644
index 000000000..ed653cf07
--- /dev/null
+++ b/slither/detectors/reentrancy/reentrancy_benign.py
@@ -0,0 +1,216 @@
+""""
+    Re-entrancy detection
+
+    Based on heuristics, it may lead to FP and FN
+    Iterate over all the nodes of the graph until reaching a fixpoint
+"""
+
+from slither.core.cfg.node import NodeType
+from slither.core.declarations import Function, SolidityFunction
+from slither.core.expressions import UnaryOperation, UnaryOperationType
+from slither.detectors.abstract_detector import (AbstractDetector,
+                                                 DetectorClassification)
+from slither.visitors.expression.export_values import ExportValues
+from slither.slithir.operations import (HighLevelCall, LowLevelCall,
+                                        LibraryCall,
+                                        Send, Transfer)
+
+class ReentrancyBenign(AbstractDetector):
+    ARGUMENT = 'reentrancy-benign'
+    HELP = 'Benign reentrancy vulnerabilities'
+    IMPACT = DetectorClassification.LOW
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities'
+
+    key = 'REENTRANCY-BENIGN'
+
+    @staticmethod
+    def _can_callback(node):
+        """
+            Detect if the node contains a call that can
+            be used to re-entrance
+
+            Consider as valid target:
+            - low level call
+            - high level call
+
+            Do not consider Send/Transfer as there is not enough gas
+        """
+        for ir in node.irs:
+            if isinstance(ir, LowLevelCall):
+                return True
+            if isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
+                return True
+        return False
+
+    @staticmethod
+    def _can_send_eth(node):
+        """
+            Detect if the node can send eth
+        """
+        for ir in node.irs:
+            if isinstance(ir, (HighLevelCall, LowLevelCall, Transfer, Send)):
+                if ir.call_value:
+                    return True
+        return False
+
+    def _filter_if(self, node):
+        """
+            Check if the node is a condtional node where
+            there is an external call checked
+            Heuristic:
+                - The call is a IF node
+                - It contains a, external call
+                - The condition is the negation (!)
+
+            This will work only on naive implementation
+        """
+        return isinstance(node.expression, UnaryOperation)\
+            and node.expression.type == UnaryOperationType.BANG
+
+    def _explore(self, node, visited, skip_father=None):
+        """
+            Explore the CFG and look for re-entrancy
+            Heuristic: There is a re-entrancy if a state variable is written
+                        after an external call
+
+            node.context will contains the external calls executed
+            It contains the calls executed in father nodes
+
+            if node.context is not empty, and variables are written, a re-entrancy is possible
+        """
+        if node in visited:
+            return
+
+        visited = visited + [node]
+
+        # First we add the external calls executed in previous nodes
+        # send_eth returns the list of calls sending value
+        # calls returns the list of calls that can callback
+        # read returns the variable read
+        fathers_context = {'send_eth':[], 'calls':[], 'read':[]}
+
+        for father in node.fathers:
+            if self.key in father.context:
+                fathers_context['send_eth'] += [s for s in father.context[self.key]['send_eth'] if s!=skip_father]
+                fathers_context['calls'] += [c for c in father.context[self.key]['calls'] if c!=skip_father]
+                fathers_context['read'] += father.context[self.key]['read']
+
+        # Exclude path that dont bring further information
+        if node in self.visited_all_paths:
+            if all(call in self.visited_all_paths[node]['calls'] for call in fathers_context['calls']):
+                if all(send in self.visited_all_paths[node]['send_eth'] for send in fathers_context['send_eth']):
+                    if all(read in self.visited_all_paths[node]['read'] for read in fathers_context['read']):
+                        return
+        else:
+            self.visited_all_paths[node] = {'send_eth':[], 'calls':[], 'read':[]}
+
+        self.visited_all_paths[node]['send_eth'] = list(set(self.visited_all_paths[node]['send_eth'] + fathers_context['send_eth']))
+        self.visited_all_paths[node]['calls'] = list(set(self.visited_all_paths[node]['calls'] + fathers_context['calls']))
+        self.visited_all_paths[node]['read'] = list(set(self.visited_all_paths[node]['read'] + fathers_context['read']))
+
+        node.context[self.key] = fathers_context
+
+        contains_call = False
+        if self._can_callback(node):
+            node.context[self.key]['calls'] = list(set(node.context[self.key]['calls'] + [node]))
+            contains_call = True
+        if self._can_send_eth(node):
+            node.context[self.key]['send_eth'] = list(set(node.context[self.key]['send_eth'] + [node]))
+
+
+        # All the state variables written
+        state_vars_written = node.state_variables_written
+        # Add the state variables written in internal calls
+        for internal_call in node.internal_calls:
+            # Filter to Function, as internal_call can be a solidity call
+            if isinstance(internal_call, Function):
+                state_vars_written += internal_call.all_state_variables_written()
+
+        not_read_then_written = [(v, node) for v in state_vars_written if v not in node.context[self.key]['read']]
+
+        node.context[self.key]['read'] = list(set(node.context[self.key]['read'] + node.state_variables_read))
+        # If a state variables was read and is then written, there is a dangerous call and
+        # ether were sent
+        # We found a potential re-entrancy bug
+        if (not_read_then_written and
+                node.context[self.key]['calls'] and
+                not node.context[self.key]['send_eth']):
+            # calls are ordered
+            finding_key = (node.function,
+                           tuple(set(node.context[self.key]['calls'])))
+            finding_vars = not_read_then_written
+            if finding_key not in self.result:
+                self.result[finding_key] = []
+            self.result[finding_key] = list(set(self.result[finding_key] + finding_vars))
+
+        sons = node.sons
+        if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]:
+            if self._filter_if(node):
+                son = sons[0]
+                self._explore(son, visited, node)
+                sons = sons[1:]
+            else:
+                son = sons[1]
+                self._explore(son, visited, node)
+                sons = [sons[0]]
+
+
+        for son in sons:
+            self._explore(son, visited)
+
+    def detect_reentrancy(self, contract):
+        """
+        """
+        for function in contract.functions:
+            if function.is_implemented:
+                self._explore(function.entry_point, [])
+
+    def detect(self):
+        """
+        """
+        self.result = {}
+
+        # if a node was already visited by another path
+        # we will only explore it if the traversal brings
+        # new variables written
+        # This speedup the exploration through a light fixpoint
+        # Its particular useful on 'complex' functions with several loops and conditions
+        self.visited_all_paths = {}
+
+        for c in self.contracts:
+            self.detect_reentrancy(c)
+
+        results = []
+
+        result_sorted = sorted(list(self.result.items()), key=lambda x:x[0][0].name)
+        for (func, calls), varsWritten in result_sorted:
+            calls = list(set(calls))
+            info = 'Reentrancy in {}.{} ({}):\n'
+            info = info.format(func.contract.name, func.name, func.source_mapping_str)
+            info += '\tExternal calls:\n'
+            for call_info in calls:
+                info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str)
+            info += '\tState variables written after the call(s):\n'
+            for (v, node) in varsWritten:
+                info +=  '\t- {} ({})\n'.format(v, node.source_mapping_str)
+            self.log(info)
+
+            sending_eth_json = []
+
+            json = self.generate_json_result(info)
+            self.add_function_to_json(func, json)
+            json['elements'] += [{'type': 'external_calls',
+                                  'expression': str(call_info.expression),
+                                  'source_mapping': call_info.source_mapping}
+                                 for call_info in calls]
+            json['elements'] += sending_eth_json
+            json['elements'] += [{'type':'variables_written',
+                                   'name': v.name,
+                                   'expression': str(node.expression),
+                                   'source_mapping': node.source_mapping}
+                                  for (v, node) in varsWritten]
+            results.append(json)
+
+        return results
diff --git a/slither/detectors/reentrancy/reentrancy_eth.py b/slither/detectors/reentrancy/reentrancy_eth.py
index cb5852e05..df94590c1 100644
--- a/slither/detectors/reentrancy/reentrancy_eth.py
+++ b/slither/detectors/reentrancy/reentrancy_eth.py
@@ -16,7 +16,7 @@ from slither.slithir.operations import (HighLevelCall, LowLevelCall,
                                         Send, Transfer)
 
 class ReentrancyEth(AbstractDetector):
-    ARGUMENT = 'reentrancy'
+    ARGUMENT = 'reentrancy-eth'
     HELP = 'Reentrancy vulnerabilities (theft of ethers)'
     IMPACT = DetectorClassification.HIGH
     CONFIDENCE = DetectorClassification.MEDIUM
@@ -212,7 +212,7 @@ class ReentrancyEth(AbstractDetector):
                 sending_eth_json = [{'type' : 'external_calls_sending_eth',
                                      'expression': str(call_info.expression),
                                      'source_mapping': call_info.source_mapping}
-                                    for call_info in calls]
+                                    for call_info in send_eth]
 
             json = self.generate_json_result(info)
             self.add_function_to_json(func, json)
diff --git a/slither/detectors/reentrancy/reentrancy_read_before_write.py b/slither/detectors/reentrancy/reentrancy_read_before_write.py
new file mode 100644
index 000000000..508a34393
--- /dev/null
+++ b/slither/detectors/reentrancy/reentrancy_read_before_write.py
@@ -0,0 +1,216 @@
+""""
+    Re-entrancy detection
+
+    Based on heuristics, it may lead to FP and FN
+    Iterate over all the nodes of the graph until reaching a fixpoint
+"""
+
+from slither.core.cfg.node import NodeType
+from slither.core.declarations import Function, SolidityFunction
+from slither.core.expressions import UnaryOperation, UnaryOperationType
+from slither.detectors.abstract_detector import (AbstractDetector,
+                                                 DetectorClassification)
+from slither.visitors.expression.export_values import ExportValues
+from slither.slithir.operations import (HighLevelCall, LowLevelCall,
+                                        LibraryCall,
+                                        Send, Transfer)
+
+class ReentrancyReadBeforeWritten(AbstractDetector):
+    ARGUMENT = 'reentrancy-no-eth'
+    HELP = 'Reentrancy vulnerabilities (no theft of ethers)'
+    IMPACT = DetectorClassification.MEDIUM
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#reentrancy-vulnerabilities'
+
+    key = 'REENTRANCY-NO-ETHER'
+
+    @staticmethod
+    def _can_callback(node):
+        """
+            Detect if the node contains a call that can
+            be used to re-entrance
+
+            Consider as valid target:
+            - low level call
+            - high level call
+
+            Do not consider Send/Transfer as there is not enough gas
+        """
+        for ir in node.irs:
+            if isinstance(ir, LowLevelCall):
+                return True
+            if isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
+                return True
+        return False
+
+    @staticmethod
+    def _can_send_eth(node):
+        """
+            Detect if the node can send eth
+        """
+        for ir in node.irs:
+            if isinstance(ir, (HighLevelCall, LowLevelCall, Transfer, Send)):
+                if ir.call_value:
+                    return True
+        return False
+
+    def _filter_if(self, node):
+        """
+            Check if the node is a condtional node where
+            there is an external call checked
+            Heuristic:
+                - The call is a IF node
+                - It contains a, external call
+                - The condition is the negation (!)
+
+            This will work only on naive implementation
+        """
+        return isinstance(node.expression, UnaryOperation)\
+            and node.expression.type == UnaryOperationType.BANG
+
+    def _explore(self, node, visited, skip_father=None):
+        """
+            Explore the CFG and look for re-entrancy
+            Heuristic: There is a re-entrancy if a state variable is written
+                        after an external call
+
+            node.context will contains the external calls executed
+            It contains the calls executed in father nodes
+
+            if node.context is not empty, and variables are written, a re-entrancy is possible
+        """
+        if node in visited:
+            return
+
+        visited = visited + [node]
+
+        # First we add the external calls executed in previous nodes
+        # send_eth returns the list of calls sending value
+        # calls returns the list of calls that can callback
+        # read returns the variable read
+        fathers_context = {'send_eth':[], 'calls':[], 'read':[]}
+
+        for father in node.fathers:
+            if self.key in father.context:
+                fathers_context['send_eth'] += [s for s in father.context[self.key]['send_eth'] if s!=skip_father]
+                fathers_context['calls'] += [c for c in father.context[self.key]['calls'] if c!=skip_father]
+                fathers_context['read'] += father.context[self.key]['read']
+
+        # Exclude path that dont bring further information
+        if node in self.visited_all_paths:
+            if all(call in self.visited_all_paths[node]['calls'] for call in fathers_context['calls']):
+                if all(send in self.visited_all_paths[node]['send_eth'] for send in fathers_context['send_eth']):
+                    if all(read in self.visited_all_paths[node]['read'] for read in fathers_context['read']):
+                        return
+        else:
+            self.visited_all_paths[node] = {'send_eth':[], 'calls':[], 'read':[]}
+
+        self.visited_all_paths[node]['send_eth'] = list(set(self.visited_all_paths[node]['send_eth'] + fathers_context['send_eth']))
+        self.visited_all_paths[node]['calls'] = list(set(self.visited_all_paths[node]['calls'] + fathers_context['calls']))
+        self.visited_all_paths[node]['read'] = list(set(self.visited_all_paths[node]['read'] + fathers_context['read']))
+
+        node.context[self.key] = fathers_context
+
+        contains_call = False
+        if self._can_callback(node):
+            node.context[self.key]['calls'] = list(set(node.context[self.key]['calls'] + [node]))
+            contains_call = True
+        if self._can_send_eth(node):
+            node.context[self.key]['send_eth'] = list(set(node.context[self.key]['send_eth'] + [node]))
+
+
+        # All the state variables written
+        state_vars_written = node.state_variables_written
+        # Add the state variables written in internal calls
+        for internal_call in node.internal_calls:
+            # Filter to Function, as internal_call can be a solidity call
+            if isinstance(internal_call, Function):
+                state_vars_written += internal_call.all_state_variables_written()
+
+        read_then_written = [(v, node) for v in state_vars_written if v in node.context[self.key]['read']]
+
+        node.context[self.key]['read'] = list(set(node.context[self.key]['read'] + node.state_variables_read))
+        # If a state variables was read and is then written, there is a dangerous call and
+        # ether were sent
+        # We found a potential re-entrancy bug
+        if (read_then_written and
+                node.context[self.key]['calls'] and
+                not node.context[self.key]['send_eth']):
+            # calls are ordered
+            finding_key = (node.function,
+                           tuple(set(node.context[self.key]['calls'])))
+            finding_vars = read_then_written
+            if finding_key not in self.result:
+                self.result[finding_key] = []
+            self.result[finding_key] = list(set(self.result[finding_key] + finding_vars))
+
+        sons = node.sons
+        if contains_call and node.type in [NodeType.IF, NodeType.IFLOOP]:
+            if self._filter_if(node):
+                son = sons[0]
+                self._explore(son, visited, node)
+                sons = sons[1:]
+            else:
+                son = sons[1]
+                self._explore(son, visited, node)
+                sons = [sons[0]]
+
+
+        for son in sons:
+            self._explore(son, visited)
+
+    def detect_reentrancy(self, contract):
+        """
+        """
+        for function in contract.functions:
+            if function.is_implemented:
+                self._explore(function.entry_point, [])
+
+    def detect(self):
+        """
+        """
+        self.result = {}
+
+        # if a node was already visited by another path
+        # we will only explore it if the traversal brings
+        # new variables written
+        # This speedup the exploration through a light fixpoint
+        # Its particular useful on 'complex' functions with several loops and conditions
+        self.visited_all_paths = {}
+
+        for c in self.contracts:
+            self.detect_reentrancy(c)
+
+        results = []
+
+        result_sorted = sorted(list(self.result.items()), key=lambda x:x[0][0].name)
+        for (func, calls), varsWritten in result_sorted:
+            calls = list(set(calls))
+            info = 'Reentrancy in {}.{} ({}):\n'
+            info = info.format(func.contract.name, func.name, func.source_mapping_str)
+            info += '\tExternal calls:\n'
+            for call_info in calls:
+                info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str)
+            info += '\tState variables written after the call(s):\n'
+            for (v, node) in varsWritten:
+                info +=  '\t- {} ({})\n'.format(v, node.source_mapping_str)
+            self.log(info)
+
+            sending_eth_json = []
+
+            json = self.generate_json_result(info)
+            self.add_function_to_json(func, json)
+            json['elements'] += [{'type': 'external_calls',
+                                  'expression': str(call_info.expression),
+                                  'source_mapping': call_info.source_mapping}
+                                 for call_info in calls]
+            json['elements'] += sending_eth_json
+            json['elements'] += [{'type':'variables_written',
+                                   'name': v.name,
+                                   'expression': str(node.expression),
+                                   'source_mapping': node.source_mapping}
+                                  for (v, node) in varsWritten]
+            results.append(json)
+
+        return results
diff --git a/tests/expected_json/reentrancy-0.5.1.reentrancy-eth.json b/tests/expected_json/reentrancy-0.5.1.reentrancy-eth.json
new file mode 100644
index 000000000..8a9a7e8d7
--- /dev/null
+++ b/tests/expected_json/reentrancy-0.5.1.reentrancy-eth.json
@@ -0,0 +1 @@
+[{"check": "reentrancy-eth", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance (tests/reentrancy-0.5.1.sol#14-22):\n\tExternal calls:\n\t- (ret,mem) = msg.sender.call.value(userBalance[msg.sender])() (tests/reentrancy-0.5.1.sol#17)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy-0.5.1.sol#21)\n", "elements": [{"type": "function", "name": "withdrawBalance", "source_mapping": {"start": 298, "length": 357, "filename": "tests/reentrancy-0.5.1.sol", "lines": [14, 15, 16, 17, 18, 19, 20, 21, 22]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 25, "length": 1807, "filename": "tests/reentrancy-0.5.1.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54]}}}, {"type": "external_calls", "expression": "(ret,mem) = msg.sender.call.value(userBalance[msg.sender])()", "source_mapping": {"start": 477, "length": 81, "filename": "tests/reentrancy-0.5.1.sol", "lines": [17]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 621, "length": 27, "filename": "tests/reentrancy-0.5.1.sol", "lines": [21]}}]}, {"check": "reentrancy-eth", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance_fixed_3 (tests/reentrancy-0.5.1.sol#44-53):\n\tExternal calls:\n\t- (ret,mem) = msg.sender.call.value(amount)() (tests/reentrancy-0.5.1.sol#49)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy-0.5.1.sol#51)\n", "elements": [{"type": "function", "name": "withdrawBalance_fixed_3", "source_mapping": {"start": 1434, "length": 393, "filename": "tests/reentrancy-0.5.1.sol", "lines": [44, 45, 46, 47, 48, 49, 50, 51, 52, 53]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 25, "length": 1807, "filename": "tests/reentrancy-0.5.1.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54]}}}, {"type": "external_calls", "expression": "(ret,mem) = msg.sender.call.value(amount)()", "source_mapping": {"start": 1679, "length": 64, "filename": "tests/reentrancy-0.5.1.sol", "lines": [49]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = amount", "source_mapping": {"start": 1778, "length": 32, "filename": "tests/reentrancy-0.5.1.sol", "lines": [51]}}]}]
\ No newline at end of file
diff --git a/tests/expected_json/reentrancy-0.5.1.reentrancy.json b/tests/expected_json/reentrancy-0.5.1.reentrancy.json
deleted file mode 100644
index f2b3785f0..000000000
--- a/tests/expected_json/reentrancy-0.5.1.reentrancy.json
+++ /dev/null
@@ -1 +0,0 @@
-[{"check": "reentrancy", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance (tests/reentrancy-0.5.1.sol#14-22):\n\tExternal calls:\n\t- (ret,mem) = msg.sender.call.value(userBalance[msg.sender])() (tests/reentrancy-0.5.1.sol#17)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy-0.5.1.sol#21)\n", "elements": [{"type": "function", "name": "withdrawBalance", "source_mapping": {"start": 298, "length": 357, "filename": "tests/reentrancy-0.5.1.sol", "lines": [14, 15, 16, 17, 18, 19, 20, 21, 22]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 25, "length": 1807, "filename": "tests/reentrancy-0.5.1.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54]}}}, {"type": "external_calls", "expression": "(ret,mem) = msg.sender.call.value(userBalance[msg.sender])()", "source_mapping": {"start": 477, "length": 81, "filename": "tests/reentrancy-0.5.1.sol", "lines": [17]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 621, "length": 27, "filename": "tests/reentrancy-0.5.1.sol", "lines": [21]}}]}, {"check": "reentrancy", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance_fixed_3 (tests/reentrancy-0.5.1.sol#44-53):\n\tExternal calls:\n\t- (ret,mem) = msg.sender.call.value(amount)() (tests/reentrancy-0.5.1.sol#49)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy-0.5.1.sol#51)\n", "elements": [{"type": "function", "name": "withdrawBalance_fixed_3", "source_mapping": {"start": 1434, "length": 393, "filename": "tests/reentrancy-0.5.1.sol", "lines": [44, 45, 46, 47, 48, 49, 50, 51, 52, 53]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 25, "length": 1807, "filename": "tests/reentrancy-0.5.1.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54]}}}, {"type": "external_calls", "expression": "(ret,mem) = msg.sender.call.value(amount)()", "source_mapping": {"start": 1679, "length": 64, "filename": "tests/reentrancy-0.5.1.sol", "lines": [49]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = amount", "source_mapping": {"start": 1778, "length": 32, "filename": "tests/reentrancy-0.5.1.sol", "lines": [51]}}]}]
\ No newline at end of file
diff --git a/tests/expected_json/reentrancy.reentrancy-eth.json b/tests/expected_json/reentrancy.reentrancy-eth.json
new file mode 100644
index 000000000..93ec6d064
--- /dev/null
+++ b/tests/expected_json/reentrancy.reentrancy-eth.json
@@ -0,0 +1 @@
+[{"check": "reentrancy-eth", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance (tests/reentrancy.sol#14-21):\n\tExternal calls:\n\t- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17-19)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#20)\n", "elements": [{"type": "function", "name": "withdrawBalance", "source_mapping": {"start": 299, "length": 314, "filename": "tests/reentrancy.sol", "lines": [14, 15, 16, 17, 18, 19, 20, 21]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 26, "length": 2334, "filename": "tests/reentrancy.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72]}}}, {"type": "external_calls", "expression": "! (msg.sender.call.value(userBalance[msg.sender])())", "source_mapping": {"start": 478, "length": 92, "filename": "tests/reentrancy.sol", "lines": [17, 18, 19]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 579, "length": 27, "filename": "tests/reentrancy.sol", "lines": [20]}}]}, {"check": "reentrancy-eth", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance_nested (tests/reentrancy.sol#64-70):\n\tExternal calls:\n\t- msg.sender.call.value(amount / 2)() (tests/reentrancy.sol#67)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#68)\n", "elements": [{"type": "function", "name": "withdrawBalance_nested", "source_mapping": {"start": 2108, "length": 246, "filename": "tests/reentrancy.sol", "lines": [64, 65, 66, 67, 68, 69, 70]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 26, "length": 2334, "filename": "tests/reentrancy.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72]}}}, {"type": "external_calls", "expression": "msg.sender.call.value(amount / 2)()", "source_mapping": {"start": 2263, "length": 33, "filename": "tests/reentrancy.sol", "lines": [67]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 2310, "length": 27, "filename": "tests/reentrancy.sol", "lines": [68]}}]}]
\ No newline at end of file
diff --git a/tests/expected_json/reentrancy.reentrancy.json b/tests/expected_json/reentrancy.reentrancy.json
deleted file mode 100644
index e9e7451e0..000000000
--- a/tests/expected_json/reentrancy.reentrancy.json
+++ /dev/null
@@ -1 +0,0 @@
-[{"check": "reentrancy", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance (tests/reentrancy.sol#14-21):\n\tExternal calls:\n\t- ! (msg.sender.call.value(userBalance[msg.sender])()) (tests/reentrancy.sol#17-19)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#20)\n", "elements": [{"type": "function", "name": "withdrawBalance", "source_mapping": {"start": 299, "length": 314, "filename": "tests/reentrancy.sol", "lines": [14, 15, 16, 17, 18, 19, 20, 21]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 26, "length": 2334, "filename": "tests/reentrancy.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72]}}}, {"type": "external_calls", "expression": "! (msg.sender.call.value(userBalance[msg.sender])())", "source_mapping": {"start": 478, "length": 92, "filename": "tests/reentrancy.sol", "lines": [17, 18, 19]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 579, "length": 27, "filename": "tests/reentrancy.sol", "lines": [20]}}]}, {"check": "reentrancy", "impact": "High", "confidence": "Medium", "description": "Reentrancy in Reentrancy.withdrawBalance_nested (tests/reentrancy.sol#64-70):\n\tExternal calls:\n\t- msg.sender.call.value(amount / 2)() (tests/reentrancy.sol#67)\n\tState variables written after the call(s):\n\t- userBalance (tests/reentrancy.sol#68)\n", "elements": [{"type": "function", "name": "withdrawBalance_nested", "source_mapping": {"start": 2108, "length": 246, "filename": "tests/reentrancy.sol", "lines": [64, 65, 66, 67, 68, 69, 70]}, "contract": {"type": "contract", "name": "Reentrancy", "source_mapping": {"start": 26, "length": 2334, "filename": "tests/reentrancy.sol", "lines": [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72]}}}, {"type": "external_calls", "expression": "msg.sender.call.value(amount / 2)()", "source_mapping": {"start": 2263, "length": 33, "filename": "tests/reentrancy.sol", "lines": [67]}}, {"type": "variables_written", "name": "userBalance", "expression": "userBalance[msg.sender] = 0", "source_mapping": {"start": 2310, "length": 27, "filename": "tests/reentrancy.sol", "lines": [68]}}]}]
\ No newline at end of file

From 7956f7f6ae2bc2bb4de17f401b5e9014f482417e Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Thu, 3 Jan 2019 14:03:00 +0100
Subject: [PATCH 4/4] Improve reentrancy bening

---
 .../detectors/reentrancy/reentrancy_benign.py  | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/slither/detectors/reentrancy/reentrancy_benign.py b/slither/detectors/reentrancy/reentrancy_benign.py
index ed653cf07..dbccd86a1 100644
--- a/slither/detectors/reentrancy/reentrancy_benign.py
+++ b/slither/detectors/reentrancy/reentrancy_benign.py
@@ -135,11 +135,11 @@ class ReentrancyBenign(AbstractDetector):
         # ether were sent
         # We found a potential re-entrancy bug
         if (not_read_then_written and
-                node.context[self.key]['calls'] and
-                not node.context[self.key]['send_eth']):
+                node.context[self.key]['calls']):
             # calls are ordered
             finding_key = (node.function,
-                           tuple(set(node.context[self.key]['calls'])))
+                           tuple(set(node.context[self.key]['calls'])),
+                           tuple(set(node.context[self.key]['send_eth'])))
             finding_vars = not_read_then_written
             if finding_key not in self.result:
                 self.result[finding_key] = []
@@ -185,19 +185,29 @@ class ReentrancyBenign(AbstractDetector):
         results = []
 
         result_sorted = sorted(list(self.result.items()), key=lambda x:x[0][0].name)
-        for (func, calls), varsWritten in result_sorted:
+        for (func, calls, send_eth), varsWritten in result_sorted:
             calls = list(set(calls))
+            send_eth = list(set(send_eth))
             info = 'Reentrancy in {}.{} ({}):\n'
             info = info.format(func.contract.name, func.name, func.source_mapping_str)
             info += '\tExternal calls:\n'
             for call_info in calls:
                 info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str)
+            if calls != send_eth:
+                info += '\tExternal calls sending eth:\n'
+                for call_info in send_eth:
+                    info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str)
             info += '\tState variables written after the call(s):\n'
             for (v, node) in varsWritten:
                 info +=  '\t- {} ({})\n'.format(v, node.source_mapping_str)
             self.log(info)
 
             sending_eth_json = []
+            if calls != send_eth:
+                sending_eth_json = [{'type' : 'external_calls_sending_eth',
+                                     'expression': str(call_info.expression),
+                                     'source_mapping': call_info.source_mapping}
+                                    for call_info in send_eth]
 
             json = self.generate_json_result(info)
             self.add_function_to_json(func, json)