Merge pull request #4880 from luchua-bc/java/sensitive-query-with-get

Java: Sensitive GET Query

java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.java

@@ -0,0 +1,13 @@
+public class SensitiveGetQuery extends HttpServlet {
+	// BAD - Tests sending sensitive information in a GET request.
+	public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
+		String password = request.getParameter("password");
+		System.out.println("password = " + password);
+	}
+	
+	// GOOD - Tests sending sensitive information in a POST request.
+	public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
+		String password = request.getParameter("password");
+		System.out.println("password = " + password);
+	}
+}

java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>Sensitive information such as user passwords should not be transmitted within the query string of the requested URL. Sensitive information within URLs may be logged in various locations, including the user's browser, the web server, and any forward or reverse proxy servers between the two endpoints. URLs may also be displayed on-screen, bookmarked or emailed around by users. They may be disclosed to third parties via the Referer header when any off-site links are followed. Placing passwords into the URL therefore increases the risk that they will be captured by an attacker.</p>
+  </overview>
+
+  <recommendation>
+    <p>Use HTTP POST to send sensitive information as part of the request body; for example, as form data.</p>
+  </recommendation>
+
+  <example>
+    <p>The following example shows two ways of sending sensitive information. In the 'BAD' case, a password is transmitted using the GET method. In the 'GOOD' case, the password is transmitted using the POST method.</p>
+    <sample src="SensitiveGetQuery.java" />
+  </example>
+
+  <references>
+    <li>
+      CWE:
+      <a href="https://cwe.mitre.org/data/definitions/598.html">CWE-598: Use of GET Request Method with Sensitive Query Strings</a>
+    </li>
+    <li>
+      PortSwigger (Burp):
+      <a href="https://portswigger.net/kb/issues/00400300_password-submitted-using-get-method">Password Submitted using GET Method</a>
+    </li>
+    <li>
+      OWASP:
+      <a href="https://owasp.org/www-community/vulnerabilities/Information_exposure_through_query_strings_in_url">Information Exposure through Query Strings in URL</a>
+    </li>
+  </references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.ql

@@ -0,0 +1,77 @@
+/**
+ * @name Sensitive GET Query
+ * @description Use of GET request method with sensitive query strings.
+ * @kind path-problem
+ * @id java/sensitive-query-with-get
+ * @tags security
+ *       external/cwe-598
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.security.SensitiveActions
+import DataFlow::PathGraph
+
+/** A variable that holds sensitive information judging by its name. */
+class SensitiveInfoExpr extends Expr {
+  SensitiveInfoExpr() {
+    exists(Variable v | this = v.getAnAccess() |
+      v.getName().regexpMatch(getCommonSensitiveInfoRegex()) and
+      not v.getName().regexpMatch("token.*") // exclude ^token.* since sensitive tokens are usually in the form of accessToken, authToken, ...
+    )
+  }
+}
+
+/** Holds if `m` is a method of some override of `HttpServlet.doGet`. */
+private predicate isGetServletMethod(Method m) {
+  isServletRequestMethod(m) and m.getName() = "doGet"
+}
+
+/** The `doGet` method of `HttpServlet`. */
+class DoGetServletMethod extends Method {
+  DoGetServletMethod() { isGetServletMethod(this) }
+}
+
+/** Holds if `ma` is (perhaps indirectly) called from the `doGet` method of `HttpServlet`. */
+predicate isReachableFromServletDoGet(MethodAccess ma) {
+  ma.getEnclosingCallable() instanceof DoGetServletMethod
+  or
+  exists(Method pm, MethodAccess pma |
+    ma.getEnclosingCallable() = pm and
+    pma.getMethod() = pm and
+    isReachableFromServletDoGet(pma)
+  )
+}
+
+/** Source of GET servlet requests. */
+class RequestGetParamSource extends DataFlow::ExprNode {
+  RequestGetParamSource() {
+    exists(MethodAccess ma |
+      isRequestGetParamMethod(ma) and
+      ma = this.asExpr() and
+      isReachableFromServletDoGet(ma)
+    )
+  }
+}
+
+/** A taint configuration tracking flow from the `ServletRequest` of a GET request handler to an expression whose name suggests it holds security-sensitive data. */
+class SensitiveGetQueryConfiguration extends TaintTracking::Configuration {
+  SensitiveGetQueryConfiguration() { this = "SensitiveGetQueryConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RequestGetParamSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveInfoExpr }
+
+  /** Holds if the node is in a servlet method other than `doGet`. */
+  override predicate isSanitizer(DataFlow::Node node) {
+    isServletRequestMethod(node.getEnclosingCallable()) and
+    not isGetServletMethod(node.getEnclosingCallable())
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveGetQueryConfiguration c
+where c.hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "$@ uses the GET request method to transmit sensitive information.", source.getNode(),
+  "This request"

java/ql/src/semmle/code/java/frameworks/Servlets.qll

@@ -322,3 +322,18 @@ class ServletWebXMLListenerType extends RefType {
     //  - `HttpSessionBindingListener`
   }
 }
+
+/** Holds if `m` is a request handler method (for example `doGet` or `doPost`). */
+predicate isServletRequestMethod(Method m) {
+  m.getDeclaringType() instanceof ServletClass and
+  m.getNumberOfParameters() = 2 and
+  m.getParameter(0).getType() instanceof ServletRequest and
+  m.getParameter(1).getType() instanceof ServletResponse
+}
+
+/** Holds if `ma` is a call that gets a request parameter. */
+predicate isRequestGetParamMethod(MethodAccess ma) {
+  ma.getMethod() instanceof ServletRequestGetParameterMethod or
+  ma.getMethod() instanceof ServletRequestGetParameterMapMethod or
+  ma.getMethod() instanceof HttpServletRequestGetQueryStringMethod
+}