From c3213e4a2ee66766a163fc214a861f47650d334b Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Mon, 18 Nov 2024 14:03:24 +0100
Subject: [PATCH] update recursion query to path-problem, deduplicate results

---
 java/src/security/Recursion/Recursion.ql      | 40 +++++++++++++++----
 .../security/Recursion/Recursion.java         | 33 +++++++++++++++
 2 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/java/src/security/Recursion/Recursion.ql b/java/src/security/Recursion/Recursion.ql
index 23bb16c..c85a60f 100644
--- a/java/src/security/Recursion/Recursion.ql
+++ b/java/src/security/Recursion/Recursion.ql
@@ -2,7 +2,7 @@
  * @name Recursive functions
  * @id tob/java/unbounded-recursion
  * @description Detects possibly unbounded recursive calls
- * @kind problem
+ * @kind path-problem
  * @tags security
  * @precision low
  * @problem.severity warning
@@ -12,6 +12,7 @@
 
 import java
 
+
 predicate isTestPackage(RefType referenceType) {
   referenceType.getPackage().getName().toLowerCase().matches("%test%") or
   referenceType.getPackage().getName().toLowerCase().matches("%benchmark%") or
@@ -19,19 +20,44 @@ predicate isTestPackage(RefType referenceType) {
 }
 
 class RecursiveMethod extends Method {
+  Call entryCall;
+  Call lastCall;
+
   RecursiveMethod() {
-    exists(Method m | this.calls+(m) and this = m)
+    not isTestPackage(this.getDeclaringType())
+    and entryCall.getEnclosingCallable() = this
+    and edges+(entryCall, lastCall) and lastCall.getCallee() = this
   }
+
+  Call getEntryCall() { result = entryCall}
+
+  Call getLastCall() { result = lastCall}
+}
+
+query predicate edges(Call a, Call b) {
+  exists(Callable c |
+    a.getCallee() = c and b.getCaller() = c
+  )
 }
 
+from RecursiveMethod recursiveMethod
+where
+  /* for a single recursion loop we would return multiple results so we deduplicate redundant findings
+     returning only the one starting from a method with "greatest" name
+  */
+  not exists(RecursiveMethod rm2 |
+    edges+(rm2.getEntryCall(), recursiveMethod.getLastCall())
+    and exists(Call x | edges(recursiveMethod.getLastCall(), x) and edges(x, rm2.getEntryCall()))
+    and rm2 != recursiveMethod
+    and rm2.getQualifiedName() > recursiveMethod.getQualifiedName()
+  )
+select recursiveMethod.getLastCall(), recursiveMethod.getEntryCall(), recursiveMethod.getLastCall(),
+  "Found a recursion path from/to method $@", recursiveMethod, recursiveMethod.toString()
+
 /**
  * TODO ideas:
  *   - limit results to methods that take any user input 
  *   - check if recursive calls are bounded (takes argument that is decremented for every call)
  *   - do not return methods that have calls to self (or unbounded recursion) that are conditional
  *   - gather and print whole call graph (list of calls from recursiveMethod to itself) 
- */
-from RecursiveMethod recursiveMethod
-where
-  not isTestPackage(recursiveMethod.getDeclaringType())
-select recursiveMethod, "Method $@ has unbounded, possibly infinite recursive calls", recursiveMethod, recursiveMethod.toString()
+ */
\ No newline at end of file
diff --git a/java/test/query-tests/security/Recursion/Recursion.java b/java/test/query-tests/security/Recursion/Recursion.java
index 4c70b5b..c15d52e 100644
--- a/java/test/query-tests/security/Recursion/Recursion.java
+++ b/java/test/query-tests/security/Recursion/Recursion.java
@@ -106,6 +106,39 @@ private boolean someCondition() {
     }
 }
 
+class RecursiveCallNonLinear {
+    // finding: level0->...->level0
+    public boolean level0() {
+        if (someOtherCondition()) {
+            return true;
+        }
+        if (someCondition()) {
+            return level1();
+        }        
+        return level2();
+    }
+    public boolean level1() {
+        if (someCondition()) {
+            return true;
+        }        
+        return level2();
+    }
+    public boolean level2() {
+        if (someCondition()) {
+            return level1();
+        }
+        return level0();
+    }
+
+    private boolean someCondition() {
+        return false;
+    }
+
+    private boolean someOtherCondition() {
+        return true;
+    }
+}
+
 class RecursiveCallWronglyLimited {
     // finding: recursion is not limited
     public boolean directRecursiveNoDepth(int anything, int depth) {