Query to detect uncontrolled thread resource consumption

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

@@ -0,0 +1,21 @@
+/** Provides sink models related to pausing thread operations. */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.ExternalFlow
+
+/** Thread pause data model in the new CSV format. */
+private class PauseThreadDataModel extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "java.lang;Thread;true;sleep;;;Argument[0];thread-pause",
+        "java.util.concurrent;TimeUnit;true;sleep;;;Argument[0];thread-pause"
+      ]
+  }
+}
+
+/** A sink representing methods pausing a thread. */
+class PauseThreadSink extends DataFlow::Node {
+  PauseThreadSink() { sinkNode(this, "thread-pause") }
+}

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

@@ -0,0 +1,40 @@
+public class ThreadResourceAbuse extends HttpServlet {
+	protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		// Get thread pause time from request parameter
+		String delayTimeStr = request.getParameter("DelayTime");
+		try {
+			int delayTime = Integer.valueOf(delayTimeStr);
+			new SyncAction(delayTime).start();
+		} catch (NumberFormatException e) {
+		}
+	}
+
+	class SyncAction extends Thread {
+		int waitTime;
+
+		public SyncAction(int waitTime) {
+			this.waitTime = waitTime;
+		}
+
+		@Override
+		public void run() {
+			try {
+				{
+					// BAD: no boundary check on wait time
+					Thread.sleep(waitTime);
+				}
+
+
+				{
+					// GOOD: enforce an upper limit on wait time
+					if (waitTime > 0 && waitTime < 5000) {
+						Thread.sleep(waitTime);
+					}
+				}
+
+				//Do other updates
+			} catch (InterruptedException e) {
+			}
+		}
+	}
+}

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

@@ -0,0 +1,39 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p><code>Thread.sleep</code> method is used to pause the execution of current thread for specified
+time. When it is used to keep several relevant tasks in synchronization and the sleep time is 
+user-controlled data, especially in the web application context, it can be abused to cause all
+of a server's threads to sleep, leading to denial of service.</p>
+</overview>
+
+<recommendation>
+<p>To guard against this attack, consider specifying an upper range of allowed sleep time or adopting
+the producer/consumer design pattern with <code>Thread.wait</code> method to avoid performance 
+problems or even resource exhaustion.</p>
+</recommendation>
+
+<example>
+<p>The following example shows the bad situation and the good situation respectively. In bad situation,
+a thread is spawned with the sleep time directly from user input. In good situation, an upper range 
+check on maximum allowed sleep time is enforced.</p>
+<sample src="ThreadResourceAbuse.java" />
+</example>
+
+<references>
+<li>
+snyk:
+<a href="https://snyk.io/vuln/SNYK-JAVA-COMGOOGLECODEGWTUPLOAD-569506">Denial of Service (DoS)
+Affecting com.googlecode.gwtupload:gwtupload artifact</a>.
+</li>
+<li>
+gwtupload
+<a href="https://github.com/manolo/gwtupload/issues/33">[Fix DOS issue] Updating the 
+AbstractUploadListener.java file</a>.
+</li>
+</references>
+</qhelp>

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

@@ -0,0 +1,87 @@
+/**
+ * @name Uncontrolled thread resource consumption
+ * @description Use user input directly to control thread sleep time could lead to performance problems
+ *              or even resource exhaustion.
+ * @kind path-problem
+ * @id java/thread-resource-abuse
+ * @tags security
+ *       external/cwe/cwe-400
+ */
+
+import java
+import ThreadPauseSink
+import semmle.code.java.dataflow.FlowSources
+import DataFlow::PathGraph
+
+/** The `getInitParameter` method of servlet or JSF. */
+class GetInitParameter extends Method {
+  GetInitParameter() {
+    (
+      this.getDeclaringType()
+          .getASupertype*()
+          .hasQualifiedName(["javax.servlet", "jakarta.servlet"],
+            ["FilterConfig", "Registration", "ServletConfig", "ServletContext"]) or
+      this.getDeclaringType()
+          .getASupertype*()
+          .hasQualifiedName(["javax.faces.context", "jakarta.faces.context"], "ExternalContext")
+    ) and
+    this.getName() = "getInitParameter"
+  }
+}
+
+/** An access to the `getInitParameter` method. */
+class GetInitParameterAccess extends MethodAccess {
+  GetInitParameterAccess() { this.getMethod() instanceof GetInitParameter }
+}
+
+/* Init parameter input of a Java EE web application. */
+class InitParameterInput extends LocalUserInput {
+  InitParameterInput() { this.asExpr() instanceof GetInitParameterAccess }
+}
+
+private class LessThanSanitizer extends DataFlow::BarrierGuard {
+  LessThanSanitizer() { this instanceof LTExpr }
+
+  override predicate checks(Expr e, boolean branch) {
+    e = this.(LTExpr).getLeftOperand() and
+    branch = true
+  }
+}
+
+/** Taint configuration of uncontrolled thread resource consumption. */
+class ThreadResourceAbuse extends TaintTracking::Configuration {
+  ThreadResourceAbuse() { this = "ThreadResourceAbuse" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source instanceof RemoteFlowSource or source instanceof LocalUserInput
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof PauseThreadSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(ConditionalExpr ce | ce.getAChildExpr() = node1.asExpr() and ce = node2.asExpr()) // request.getParameter("nodelay") != null ? 0 : sleepTime
+    or
+    exists(
+      Method rm, ClassInstanceExpr ce, Argument arg, FieldAccess fa // thread.start() invokes the run() method of thread implementation
+    |
+      rm.hasName("run") and
+      ce.getConstructedType() = 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
+      node1.asExpr() = arg and
+      node2.asExpr() = fa
+    )
+  }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof LessThanSanitizer // if (sleepTime > 0 && sleepTime < 5000) { ... }
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, ThreadResourceAbuse conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "Vulnerability of uncontrolled resource consumption due to $@.", source.getNode(),
+  "user-provided value"

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

@@ -0,0 +1,18 @@
+edges
+| ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) : String | ThreadResourceAbuse.java:18:28:18:36 | delayTime : Number |
+| ThreadResourceAbuse.java:18:28:18:36 | delayTime : Number | ThreadResourceAbuse.java:62:18:62:25 | waitTime |
+| ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) : String | ThreadResourceAbuse.java:28:28:28:36 | delayTime : Number |
+| ThreadResourceAbuse.java:28:28:28:36 | delayTime : Number | ThreadResourceAbuse.java:62:18:62:25 | waitTime |
+| ThreadResourceAbuse.java:97:27:97:43 | getValue(...) : String | ThreadResourceAbuse.java:100:34:100:42 | delayTime |
+nodes
+| ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| ThreadResourceAbuse.java:18:28:18:36 | delayTime : Number | semmle.label | delayTime : Number |
+| ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) : String | semmle.label | getInitParameter(...) : String |
+| ThreadResourceAbuse.java:28:28:28:36 | delayTime : Number | semmle.label | delayTime : Number |
+| ThreadResourceAbuse.java:62:18:62:25 | waitTime | semmle.label | waitTime |
+| ThreadResourceAbuse.java:97:27:97:43 | getValue(...) : String | semmle.label | getValue(...) : String |
+| ThreadResourceAbuse.java:100:34:100:42 | delayTime | semmle.label | delayTime |
+#select
+| ThreadResourceAbuse.java:62:18:62:25 | waitTime | ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) : String | ThreadResourceAbuse.java:62:18:62:25 | waitTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:15:25:15:57 | getParameter(...) | user-provided value |
+| ThreadResourceAbuse.java:62:18:62:25 | waitTime | ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) : String | ThreadResourceAbuse.java:62:18:62:25 | waitTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:25:25:25:73 | getInitParameter(...) | user-provided value |
+| ThreadResourceAbuse.java:100:34:100:42 | delayTime | ThreadResourceAbuse.java:97:27:97:43 | getValue(...) : String | ThreadResourceAbuse.java:100:34:100:42 | delayTime | Vulnerability of uncontrolled resource consumption due to $@. | ThreadResourceAbuse.java:97:27:97:43 | getValue(...) | user-provided value |

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

@@ -0,0 +1,108 @@
+package test.cwe400.cwe.examples;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+public class ThreadResourceAbuse extends HttpServlet {
+	protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		// Get thread pause time from request parameter
+		String delayTimeStr = request.getParameter("DelayTime");
+		try {
+			int delayTime = Integer.valueOf(delayTimeStr);
+			new UncheckedSyncAction(delayTime).start();
+		} catch (NumberFormatException e) {
+		}
+	}
+
+	protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		// Get thread pause time from init container parameter
+		String delayTimeStr = getServletContext().getInitParameter("DelayTime");
+		try {
+			int delayTime = Integer.valueOf(delayTimeStr);
+			new UncheckedSyncAction(delayTime).start();
+		} catch (NumberFormatException e) {
+		}
+	}
+
+	protected void doPut(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		// Get thread pause time from request cookie
+		Cookie[] cookies = request.getCookies();
+
+		for ( int i=0; i<cookies.length; i++) {
+			Cookie cookie = cookies[i];
+
+			if (cookie.getName().equals("DelayTime")) {
+				String delayTimeStr = cookie.getValue();
+				try {
+					int delayTime = Integer.valueOf(delayTimeStr);
+					new CheckedSyncAction(delayTime).start();
+				} catch (NumberFormatException e) {
+				}
+			}
+		}
+	}
+
+	class UncheckedSyncAction extends Thread {
+		int waitTime;
+
+		public UncheckedSyncAction(int waitTime) {
+			this.waitTime = waitTime;
+		}
+
+		@Override
+		// BAD: no boundary check on wait time
+		public void run() {
+			try {
+				Thread.sleep(waitTime);
+				// Do other updates
+			} catch (InterruptedException e) {
+			}
+		}
+	}
+
+	class CheckedSyncAction extends Thread {
+		int waitTime;
+
+		public CheckedSyncAction(int waitTime) {
+			this.waitTime = waitTime;
+		}
+
+		// GOOD: enforce an upper limit on wait time
+		@Override
+		public void run() {
+			try {
+				if (waitTime > 0 && waitTime < 5000) {
+				Thread.sleep(waitTime);
+				// Do other updates
+			}
+		  } catch (InterruptedException e) {
+		  }
+		}
+	}
+
+	protected void doHead(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		// Get thread pause time from request cookie
+		Cookie[] cookies = request.getCookies();
+
+		for ( int i=0; i<cookies.length; i++) {
+			Cookie cookie = cookies[i];
+
+			if (cookie.getName().equals("DelayTime")) {
+				String delayTimeStr = cookie.getValue();
+				try {
+					int delayTime = Integer.valueOf(delayTimeStr);
+					TimeUnit.MILLISECONDS.sleep(delayTime);
+					// Do other updates
+				} catch (NumberFormatException ne) {
+				} catch (InterruptedException ie) {
+				}
+			}
+		}
+	}
+}

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

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql

java/ql/test/experimental/query-tests/security/CWE-400/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4