Merge pull request #2628 from crytic/dev-fix-modifier-detector

incorrect-modifier: Fix infinite loop

slither/detectors/functions/modifier.py

@@ -67,21 +67,30 @@ class ModifierDefaultDetection(AbstractDetector):
 
     def _detect(self) -> List[Output]:
         results = []
+        # pylint: disable=too-many-nested-blocks
         for c in self.contracts:
             for mod in c.modifiers:
                 if mod.contract_declarer != c:
                     continue
                 # Walk down the tree, only looking at nodes in the outer scope
                 node = mod.entry_point
+                node_seen = []
                 while node is not None:
+                    node_seen.append(node)
                     # If any node in the outer scope executes _; or reverts,
                     # we will never return a default value
                     if node.type == NodeType.PLACEHOLDER or is_revert(node):
                         break
 
                     # Move down, staying on the outer scope in branches
                     if len(node.sons) > 0:
-                        node = _get_false_son(node) if node.contains_if() else node.sons[0]
+                        if node.contains_if():
+                            node = _get_false_son(node)
+                        else:
+                            if node.sons[0] in node_seen:
+                                node = node.sons[1]
+                            else:
+                                node = node.sons[0]
                     else:
                         node = None
                 else:

tests/e2e/detectors/test_data/incorrect-modifier/0.4.25/modifier_default.sol

@@ -49,4 +49,10 @@ contract Test {
             _;
         } while (i < 1);
     }
+
+    modifier issue2619(bool b) {
+        while (b ? false : b) {}
+        _;
+    }
+
 }

tests/e2e/detectors/test_data/incorrect-modifier/0.5.16/modifier_default.sol

@@ -49,4 +49,10 @@ contract Test {
             _;
         } while (i < 1);
     }
+
+    modifier issue2619(bool b) {
+        while (b ? false : b) {}
+        _;
+    }
+
 }

tests/e2e/detectors/test_data/incorrect-modifier/0.6.11/modifier_default.sol

@@ -49,4 +49,10 @@ contract Test {
             _;
         } while (i < 1);
     }
+
+    modifier issue2619(bool b) {
+        while (b ? false : b) {}
+        _;
+    }
+
 }

tests/e2e/detectors/test_data/incorrect-modifier/0.7.6/modifier_default.sol

@@ -49,4 +49,9 @@ contract Test {
             _;
         } while (i < 1);
     }
+
+    modifier issue2619(bool b) {
+        while (b ? false : b) {}
+        _;
+    }
 }