Merge pull request #706 from crytic/dev-divive_before_multiply

Remove recursion in divide-before-multiplity

slither/detectors/statements/divide_before_multiply.py

@@ -50,37 +50,13 @@ def is_assert(node):
     return False
 
 
-class DivideBeforeMultiply(AbstractDetector):
+def _explore(to_explore, f_results, divisions):  # pylint: disable=too-many-branches
-    """
+    explored = set()
-    Divide before multiply
+    while to_explore:  # pylint: disable=too-many-nested-blocks
-    """
+        node = to_explore.pop()
-
-    ARGUMENT = "divide-before-multiply"
-    HELP = "Imprecise arithmetic operations order"
-    IMPACT = DetectorClassification.MEDIUM
-    CONFIDENCE = DetectorClassification.MEDIUM
-
-    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply"
-
-    WIKI_TITLE = "Divide before multiply"
-    WIKI_DESCRIPTION = """Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision."""
-    WIKI_EXPLOIT_SCENARIO = """
-```solidity
-contract A {
-	function f(uint n) public {
-        coins = (oldSupply / n) * interest;
-    }
-}
-```
-If `n` is greater than `oldSupply`, `coins` will be zero. For example, with `oldSupply = 5; n = 10, interest = 2`, coins will be zero.  
-If `(oldSupply * interest / n)` was used, `coins` would have been `1`.   
-In general, it's usually a good idea to re-arrange arithmetic to perform multiplication before division, unless the limit of a smaller type makes this dangerous."""
-
-    WIKI_RECOMMENDATION = """Consider ordering multiplication before division."""
 
-    def _explore(self, node, explored, f_results, divisions):  # pylint: disable=too-many-branches
         if node in explored:
-            return
+            continue
         explored.add(node)
 
         equality_found = False
@@ -121,47 +97,75 @@ In general, it's usually a good idea to re-arrange arithmetic to perform multipl
             # We do not track the case where the multiplication is done in a require() or assert()
             # Which also contains a ==, to prevent FP due to the form
             # assert(a == b * c + a % b)
-            if is_assert(node) and equality_found:
+            if not (is_assert(node) and equality_found):
-                pass
-            else:
                 f_results.append(node_results)
 
         for son in node.sons:
-            self._explore(son, explored, f_results, divisions)
+            to_explore.add(son)
 
-    def detect_divide_before_multiply(self, contract):
-        """
-        Detects and returns all nodes with multiplications of division results.
-        :param contract: Contract to detect assignment within.
-        :return: A list of nodes with multiplications of divisions.
-        """
 
-        # Create our result set.
+def detect_divide_before_multiply(contract):
-        # List of tuple (function -> list(list(nodes)))
+    """
-        # Each list(nodes) of the list is one bug instances
+    Detects and returns all nodes with multiplications of division results.
-        # Each node in the list(nodes) is involved in the bug
+    :param contract: Contract to detect assignment within.
-        results = []
+    :return: A list of nodes with multiplications of divisions.
+    """
 
-        # Loop for each function and modifier.
+    # Create our result set.
-        for function in contract.functions_declared:
+    # List of tuple (function -> list(list(nodes)))
-            if not function.entry_point:
+    # Each list(nodes) of the list is one bug instances
-                continue
+    # Each node in the list(nodes) is involved in the bug
+    results = []
 
-            # List of list(nodes)
+    # Loop for each function and modifier.
-            # Each list(nodes) is one bug instances
+    for function in contract.functions_declared:
-            f_results = []
+        if not function.entry_point:
+            continue
 
-            # lvalue -> node
+        # List of list(nodes)
-            # track all the division results (and the assignment of the division results)
+        # Each list(nodes) is one bug instances
-            divisions = defaultdict(list)
+        f_results = []
 
-            self._explore(function.entry_point, set(), f_results, divisions)
+        # lvalue -> node
+        # track all the division results (and the assignment of the division results)
+        divisions = defaultdict(list)
 
-            for f_result in f_results:
+        _explore({function.entry_point}, f_results, divisions)
-                results.append((function, f_result))
 
-        # Return the resulting set of nodes with divisions before multiplications
+        for f_result in f_results:
-        return results
+            results.append((function, f_result))
+
+    # Return the resulting set of nodes with divisions before multiplications
+    return results
+
+
+class DivideBeforeMultiply(AbstractDetector):
+    """
+    Divide before multiply
+    """
+
+    ARGUMENT = "divide-before-multiply"
+    HELP = "Imprecise arithmetic operations order"
+    IMPACT = DetectorClassification.MEDIUM
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply"
+
+    WIKI_TITLE = "Divide before multiply"
+    WIKI_DESCRIPTION = """Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision."""
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract A {
+	function f(uint n) public {
+        coins = (oldSupply / n) * interest;
+    }
+}
+```
+If `n` is greater than `oldSupply`, `coins` will be zero. For example, with `oldSupply = 5; n = 10, interest = 2`, coins will be zero.  
+If `(oldSupply * interest / n)` was used, `coins` would have been `1`.   
+In general, it's usually a good idea to re-arrange arithmetic to perform multiplication before division, unless the limit of a smaller type makes this dangerous."""
+
+    WIKI_RECOMMENDATION = """Consider ordering multiplication before division."""
 
     def _detect(self):
         """
@@ -169,7 +173,7 @@ In general, it's usually a good idea to re-arrange arithmetic to perform multipl
         """
         results = []
         for contract in self.contracts:
-            divisions_before_multiplications = self.detect_divide_before_multiply(contract)
+            divisions_before_multiplications = detect_divide_before_multiply(contract)
             if divisions_before_multiplications:
                 for (func, nodes) in divisions_before_multiplications: