Add detection for Recursion in Java

.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,37 @@
+/**
+ * @name Recursive functions
+ * @id tob/java/unbounded-recursion
+ * @description Detects possibly unbounded 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%")
+}
+
+class RecursiveMethod extends Method {
+  RecursiveMethod() {
+    exists(Method m | this.calls+(m) and this = m)
+  }
+}
+
+/**
+ * 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()

java/src/security/Recursion/RecursiveCall.java

@@ -0,0 +1,17 @@
+// 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);
+    }
+    // ...
+}

java/test/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/test/qlpack.yml

@@ -0,0 +1,8 @@
+---
+name: trailofbits/java-tests
+authors: Trail of Bits
+license: AGPL
+extractor: java-kotlin
+tests: .
+dependencies:
+  trailofbits/java-queries: "*"

java/test/query-tests/security/Recursion/Recursion.expected

@@ -0,0 +1,3 @@
+| Recursion.java:8:16:8:20 | bar(...) | $@ | Recursion.java:8:16:8:20 | bar(...) | Recursion(2): bar(2) -> foo(7) -> bar |
+| Recursion.java:12:16:12:32 | directRecursive(...) | $@ | Recursion.java:12:16:12:32 | directRecursive(...) | Recursion(1): directRecursive(11) -> directRecursive |
+| Recursion.java:31:16:31:23 | level0(...) | $@ | Recursion.java:31:16:31:23 | level0(...) | Recursion(3): level0(15) -> level1(21) -> level2(27) -> level0 |