Correct the run method and add Math.min check

Author: luchua-bc

java/ql/src/experimental/Security/CWE/CWE-400/ThreadPauseSink.qll

@@ -4,6 +4,17 @@ import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.ExternalFlow
 
+/** `java.lang.Math` data model for value comparison in the new CSV format. */
+private class MathCompDataModel extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "java.lang;Math;false;min;;;Argument[0..1];ReturnValue;taint",
+        "java.lang;Math;false;max;;;Argument[0..1];ReturnValue;taint"
+      ]
+  }
+}
+
 /** Thread pause data model in the new CSV format. */
 private class PauseThreadDataModel extends SinkModelCsv {
   override predicate row(string row) {

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql

@@ -63,19 +63,29 @@ class ThreadResourceAbuse extends TaintTracking::Configuration {
 
   override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
     exists(
-      Method rm, ClassInstanceExpr ce, Argument arg, FieldAccess fa // thread.start() invokes the run() method of thread implementation
+      Method rm, ClassInstanceExpr ce, Argument arg, Parameter p, FieldAccess fa, int i // thread.start() invokes the run() method of thread implementation
     |
       rm.hasName("run") and
       ce.getConstructedType().getSourceDeclaration() = rm.getSourceDeclaration().getDeclaringType() and
       ce.getConstructedType().getASupertype*().hasQualifiedName("java.lang", "Runnable") and
-      ce.getAnArgument() = arg and
-      fa = rm.getAnAccessedField().getAnAccess() and
-      arg.getType() = fa.getField().getType() and
+      ce.getArgument(i) = arg and
+      ce.getConstructor().getParameter(i) = p and
+      fa.getEnclosingCallable() = rm and
+      DataFlow::localExprFlow(p.getAnAccess(), fa.getField().getAnAssignedValue()) and
       node1.asExpr() = arg and
       node2.asExpr() = fa
     )
   }
 
+  override predicate isSanitizer(DataFlow::Node node) {
+    exists(
+      MethodAccess ma // Math.min(sleepTime, MAX_INTERVAL)
+    |
+      ma.getMethod().hasQualifiedName("java.lang", "Math", "min") and
+      node.asExpr() = ma.getAnArgument()
+    )
+  }
+
   override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
     guard instanceof LessThanSanitizer // if (sleepTime > 0 && sleepTime < 5000) { ... }
   }

java/ql/test/experimental/query-tests/security/CWE-400/ThreadResourceAbuse.java

@@ -150,7 +150,7 @@ protected void doHead(HttpServletRequest request, HttpServletResponse response)
 		}
 	}
 
-	int parseReplyAfter(String value) {
+	int parseRetryAfter(String value) {
 		if (value == null || value.isEmpty()) {
 			return DEFAULT_RETRY_AFTER;
 		}
@@ -183,7 +183,7 @@ protected void doHead2(HttpServletRequest request, HttpServletResponse response)
 	protected void doHead3(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
 		// Get thread pause time from request header
 		String header = request.getHeader("Retry-After");
-		int retryAfter = parseReplyAfter(header);
+		int retryAfter = parseRetryAfter(header);
 
 		try {
 			// GOOD: wait for retry-after with input validation