Add detection for Recursion in Java

Author: DarkaMaul

.codeqlmanifest.json

@@ -1,6 +1,7 @@
 {
     "provide": [
         "cpp/*/qlpack.yml",
-        "go/*/qlpack.yml"
+        "go/*/qlpack.yml",
+        "java/*/qlpack.yml"
     ]
 }

.github/workflows/test.yml

@@ -16,3 +16,4 @@ jobs:
         run: |
           ${{ steps.init.outputs.codeql-path }} test run ./cpp/test/
           ${{ steps.init.outputs.codeql-path }} test run ./go/test/
+          ${{ steps.init.outputs.codeql-path }} test run ./java/test/

README.md

@@ -66,6 +66,14 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
 |[Missing MinVersion in tls.Config](./go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md)|This rule finds cases when you do not set the `tls.Config.MinVersion` explicitly for servers. By default version 1.0 is used, which is considered insecure. This rule does not mark explicitly set insecure versions|error|medium|
 |[Trim functions misuse](./go/src/docs/security/TrimMisuse/TrimMisuse.md)|Finds calls to `string.{Trim,TrimLeft,TrimRight}` with the 2nd argument not being a cutset but a continuous substring to be trimmed|error|low|
 
+### Java-kotlin
+
+#### Security
+
+| Name | Description | Severity | Precision  |
+| ---  | ----------- | :----:   | :--------: |
+|[Recursive functions](./java-kotlin/src/docs/security/Recursion/Recursion.md)|Detects recursive calls|warning|low|
+
 ## Query suites
 
 CodeQL queries are grouped into "suites". To execute queries from a specific suit add its name after a colon: `trailofbits/cpp-queries:codeql-suites/tob-cpp-full.qls`.
@@ -89,7 +97,7 @@ echo "--search-path '$PWD/codeql-queries'" > "${HOME}/.config/codeql/config"
 
 Check that CodeQL CLI detects the new qlpacks:
 ```sh
-codeql resolve qlpacks | grep trailofbits
+codeql resolve packs | grep trailofbits
 ```
 
 #### Before committing
@@ -99,6 +107,7 @@ Run tests:
 cd codeql-queries
 codeql test run ./cpp/test
 codeql test run ./go/test
+codeql test run ./java/test
 ```
 
 Update dependencies:
@@ -115,4 +124,5 @@ Generate markdown query help files
 ```sh
 codeql generate query-help ./cpp/src/ --format=markdown --output ./cpp/src/docs
 codeql generate query-help ./go/src/ --format=markdown --output ./go/src/docs
+codeql generate query-help ./java/src/ --format=markdown --output ./java/src/docs
 ```

java/src/codeql-pack.lock.yml

@@ -0,0 +1,28 @@
+---
+lockVersion: 1.0.0
+dependencies:
+  codeql/dataflow:
+    version: 1.1.5
+  codeql/java-all:
+    version: 4.2.0
+  codeql/mad:
+    version: 1.0.11
+  codeql/rangeanalysis:
+    version: 1.0.11
+  codeql/regex:
+    version: 1.0.11
+  codeql/ssa:
+    version: 1.0.11
+  codeql/threat-models:
+    version: 1.0.11
+  codeql/tutorial:
+    version: 1.0.11
+  codeql/typeflow:
+    version: 1.0.11
+  codeql/typetracking:
+    version: 1.0.11
+  codeql/util:
+    version: 1.0.11
+  codeql/xml:
+    version: 1.0.11
+compiled: false

java/src/codeql-suites/tob-java-code-scanning.qls

@@ -0,0 +1,5 @@
+- description: Security queries for Java
+- queries: 'security'
+  from: trailofbits/java-queries
+- exclude:
+    tags contain: experimental

java/src/codeql-suites/tob-java-full.qls

@@ -0,0 +1,3 @@
+- description: Queries for Java
+- queries: '.'
+  from: trailofbits/java-queries

java/src/docs/security/Recursion/Recursion.md

@@ -0,0 +1,39 @@
+# Recursive functions
+Recursive functions are methods that call themselves either directly or indirectly through other functions. While recursion can be a powerful programming technique, unbounded recursion on user inputs can lead to stack overflow errors and program crashes, potentially enabling denial of service attacks. This query detects recursive patterns up to order 4.
+
+
+## Recommendation
+Review recursive functions and ensure that they are either: - Not processing user-controlled data - The data has been properly sanitized before recursing - The recursion has an explicit depth limit
+
+Consider replacing recursion with iterative alternatives where possible.
+
+
+## Example
+
+```java
+// From https://github.com/x-stream/xstream/blob/dfa1d35462fe84412ee72a9b0cf5b5c633086520/xstream/src/java/com/thoughtworks/xstream/io/binary/BinaryStreamReader.java#L165
+private Token readToken() {
+    // ...
+    try {
+        final Token token = tokenFormatter.read(in);
+        switch (token.getType()) {
+        case Token.TYPE_MAP_ID_TO_VALUE: // 0x2
+            idRegistry.put(token.getId(), token.getValue());
+            return readToken(); // Next one please.
+        default:
+            return token;
+        }
+    } catch (final IOException e) {
+        throw new StreamException(e);
+    }
+    // ...
+}
+```
+In this example, a binary stream reader processes tokens recursively.
+
+For each new token \`0x2\`, the parser will create a new recursive call. If this stream is user-controlled, an attacker can generate too many stackframes and crash the application with a `StackOverflow` error.
+
+
+## References
+* Trail Of Bits Blog: [Low-effort denial of service with recursion](https://blog.trailofbits.com/2024/05/16/TODO/)
+* CWE-674: [Uncontrolled Recursion](https://cwe.mitre.org/data/definitions/674.html)

java/src/qlpack.yml

@@ -0,0 +1,12 @@
+---
+name: trailofbits/java-queries
+description: CodeQL queries for Java developed by Trail of Bits
+authors: Trail of Bits
+version: 0.0.1
+license: AGPL
+library: false
+extractor: java-kotlin
+dependencies:
+  codeql/java-all: "*"
+suites: codeql-suites
+defaultSuiteFile: codeql-suites/tob-java-code-scanning.qls

java/src/security/Recursion/Recursion.qhelp

@@ -0,0 +1,39 @@
+<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+        Recursive functions are methods that call themselves either directly or indirectly through other functions.
+        While recursion can be a powerful programming technique, unbounded recursion on user inputs can lead
+        to stack overflow errors and program crashes, potentially enabling denial of service attacks.
+
+        This query detects recursive patterns up to order 4.
+        </p>
+
+    </overview>
+    <recommendation>
+        <p>
+        Review recursive functions and ensure that they are either:
+            - Not processing user-controlled data
+            - The data has been properly sanitized before recursing
+            - The recursion has an explicit depth limit
+        </p>
+        <p>
+        Consider replacing recursion with iterative alternatives where possible.
+        </p>
+    </recommendation>
+    <example>
+        <sample src="RecursiveCall.java" />
+        <p>In this example, a binary stream reader processes tokens recursively.</p>
+        <p>For each new token `0x2`, the parser will create a new recursive call.
+        If this stream is user-controlled, an attacker can generate too many stackframes
+        and crash the application with a <code>StackOverflow</code> error.</p>
+    </example>
+    <references>
+        <li>
+            Trail Of Bits Blog: <a href="https://blog.trailofbits.com/2024/05/16/TODO/">Low-effort denial of service with recursion</a>
+        </li>
+        <li>
+            CWE-674: <a href="https://cwe.mitre.org/data/definitions/674.html">Uncontrolled Recursion</a>
+        </li>
+    </references>
+</qhelp>

java/src/security/Recursion/Recursion.ql

@@ -0,0 +1,96 @@
+/**
+ * @name Recursive functions
+ * @id tob/java/recursion
+ * @description Detects recursive calls
+ * @kind problem
+ * @tags security
+ * @precision low
+ * @problem.severity warning
+ * @security-severity 3.0
+ * @group security
+ */
+
+import java
+
+predicate isTestPackage(RefType referenceType) {
+  referenceType.getPackage().getName().toLowerCase().matches("%test%") or
+  referenceType.getPackage().getName().toLowerCase().matches("%benchmark%") or
+  referenceType.getName().toLowerCase().matches("%test%")
+}
+
+predicate isBefore(Method a, Method b) {
+  a.getLocation().getStartLine() < b.getLocation().getStartLine()
+}
+
+abstract class RecursiveCall extends MethodCall {
+  Method m1;
+  Method m2;
+  Method m3;
+  Method m4;
+
+  abstract string asString();
+
+  string toString(Method m) {
+    result = m.getName() + "(" + m.getLocation().getStartLine() + ")" + " -> "
+  }
+}
+
+class RecursiveCallOrder1 extends RecursiveCall {
+  RecursiveCallOrder1() {
+    this.getMethod() = this.getEnclosingCallable() and
+    m1 = this.getMethod()
+  }
+
+  override string asString() { result = "Recursion(1): " + toString(m1) + m1.getName() }
+}
+
+class RecursiveCallOrder2 extends RecursiveCall {
+  //RecursiveCallOrder1 {
+  RecursiveCallOrder2() {
+    m1 = this.getMethod() and
+    m2 = m1.getACallee() and
+    m2.getACallee() = m1 and
+    isBefore(m1, m2)
+  }
+
+  override string asString() {
+    result = "Recursion(2): " + toString(m1) + toString(m2) + m1.getName()
+  }
+}
+
+class RecursiveCallOrder3 extends RecursiveCall {
+  RecursiveCallOrder3() {
+    m1 = this.getMethod() and
+    m2 = m1.getACallee() and
+    m3 = m2.getACallee() and
+    m3.getACallee() = m1 and
+    isBefore(m1, m2) and
+    isBefore(m1, m3)
+  }
+
+  override string asString() {
+    result = "Recursion(3): " + toString(m1) + toString(m2) + toString(m3) + m1.getName()
+  }
+}
+
+class RecursiveCallOrder4 extends RecursiveCall {
+  RecursiveCallOrder4() {
+    m1 = this.getMethod() and
+    m2 = m1.getACallee() and
+    m3 = m2.getACallee() and
+    m4 = m3.getACallee() and
+    m4.getACallee() = m1 and
+    isBefore(m1, m2) and
+    isBefore(m1, m3) and
+    isBefore(m1, m4)
+  }
+
+  override string asString() {
+    result =
+      "Recursion(4): " + toString(m1) + toString(m2) + toString(m3) + toString(m4) + m1.getName()
+  }
+}
+
+from RecursiveCall recursiveCall
+where not isTestPackage(recursiveCall.getMethod().getDeclaringType())
+select recursiveCall, "$@", recursiveCall, recursiveCall.asString()