Update query

trailofbits · Nov 19, 2024 · 512a659 · 512a659
1 parent e4c26e3
commit 512a659
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 162 deletions.
diff --git a/java/src/security/Recursion/Recursion.ql b/java/src/security/Recursion/Recursion.ql
@@ -13,7 +13,6 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 
-
 predicate isTestPackage(RefType referenceType) {
   referenceType.getPackage().getName().toLowerCase().matches("%test%") or
   referenceType.getPackage().getName().toLowerCase().matches("%benchmark%") or
@@ -24,11 +23,25 @@ class RecursionSource extends MethodCall {
   RecursionSource() { not isTestPackage(this.getCaller().getDeclaringType()) }
 
   override string toString() {
-    result = this.getCaller().toString() + " calls " + this.getCallee().toString()
+    result = this.getCaller().toString() + " clls " + this.getCallee().toString()
   }
 
 }
 
+/**
+ * Check if the Expr uses directly an argument of the enclosing function
+ */
+class ParameterOperation extends Expr {
+  ParameterOperation() {
+      this instanceof BinaryExpr or this instanceof UnaryAssignExpr
+      and exists(
+        VarAccess va |
+        va.getVariable() = this.getEnclosingCallable().getAParameter() |
+        this.getAChildExpr+() = va
+      )
+  }
+}
+
 module RecursiveConfig implements DataFlow::StateConfigSig {
   class FlowState = Method;
 
@@ -44,12 +57,26 @@ module RecursiveConfig implements DataFlow::StateConfigSig {
   }
 
   predicate isBarrier(DataFlow::Node node) {
-    node.asExpr() instanceof MethodCall and
-    exists(Expr arg | arg = node.asExpr().(MethodCall).getAnArgument() |
-      arg instanceof BinaryExpr or
-      exists(BinaryExpr b | DataFlow::localFlow(DataFlow::exprNode(b), DataFlow::exprNode(arg)))
+    exists(MethodCall ma  |
+      ma = node.asExpr()
+      and (
+        exists(Expr e | e = ma.getAnArgument() and e instanceof ParameterOperation)
+        // or exists(
+        //   VarAccess e| 
+        //   e = ma.getAnArgument() |
+        //   e.getVariable().getAnAssignedValue().getAChildExpr() instanceof ParameterOperation
+        // )
+      )
     )
   }
+
+  /**
+   * Weird but useful deduplication logic
+   */
+  predicate isBarrierIn(DataFlow::Node node, FlowState state) {
+    not node.asExpr() instanceof MethodCall
+    or node.asExpr().(MethodCall).getCaller().getLocation().getStartLine() > state.getLocation().getStartLine()
+  }
 }
 
 module RecursiveFlow = DataFlow::GlobalWithState<RecursiveConfig>;
@@ -66,4 +93,4 @@ import RecursiveFlow::PathGraph
 from RecursiveFlow::PathNode source, RecursiveFlow::PathNode sink
 where RecursiveFlow::flowPath(source, sink)
 // TODO(dm): de-duplicate results
-select sink.getNode(), source, sink, "Found a recursion: "
+select sink.getNode(), source, sink, "Found a recursion: "