Merge branch 'main' into henrymercer/merge-back-rc-3.13

java/ql/src/Metrics/Summaries/LinesOfCode.ql

@@ -7,6 +7,7 @@
  * @kind metric
  * @tags summary
  *       lines-of-code
+ *       debug
  */
 
 import java

java/ql/src/Security/CWE/CWE-601/UrlRedirect.java

@@ -1,14 +0,0 @@
-public class UrlRedirect extends HttpServlet {
-	private static final String VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";
-
-	protected void doGet(HttpServletRequest request, HttpServletResponse response)
-	throws ServletException, IOException {
-		// BAD: a request parameter is incorporated without validation into a URL redirect
-		response.sendRedirect(request.getParameter("target"));
-
-		// GOOD: the request parameter is validated against a known fixed string
-		if (VALID_REDIRECT.equals(request.getParameter("target"))) {
-			response.sendRedirect(VALID_REDIRECT);
-		}
-	}
-}

java/ql/src/Security/CWE/CWE-601/UrlRedirect.qhelp

@@ -16,21 +16,53 @@ controlled by the attacker.</p>
 <p>To guard against untrusted URL redirection, it is advisable to avoid putting user input
 directly into a redirect URL. Instead, maintain a list of authorized
 redirects on the server; then choose from that list based on the user input provided.</p>
-
+<p>
+If this is not possible, then the user input should be validated in some other way,
+for example, by verifying that the target URL is on the same host as the current page.
+</p>
 </recommendation>
-<example>
 
-<p>The following example shows an HTTP request parameter being used directly in a URL redirect
-without validating the input, which facilitates phishing attacks.
-It also shows how to remedy the problem by validating the user input against a known fixed string.
+
+<example>
+<p>
+The following example shows an HTTP request parameter being used directly in a URL redirect
+without validating the input, which facilitates phishing attacks:
 </p>
 
-<sample src="UrlRedirect.java" />
+<sample src="examples/UrlRedirect.java"/>
+
+<p>
+One way to remedy the problem is to validate the user input against a known fixed string
+before doing the redirection:
+</p>
+
+<sample src="examples/UrlRedirectGood.java"/>
+
+<p>
+Alternatively, we can check that the target URL does not redirect to a different host
+by checking that the URL is either relative or on a known good host:
+</p>
+
+<sample src="examples/UrlRedirectGoodDomain.java"/>
+
+<p>
+Note that as written, the above code will allow redirects to URLs on <code>example.com</code>,
+which is harmless but perhaps not intended. You can substitute your own domain (if known) for
+<code>example.com</code> to prevent this.
+</p>
 
 </example>
+
 <references>
 
-
+<li>
+OWASP:
+<a href="https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html">
+Unvalidated Redirects and Forwards Cheat Sheet</a>.
+</li>
+<li>
+Microsoft Docs: <a href="https://docs.microsoft.com/en-us/aspnet/mvc/overview/security/preventing-open-redirection-attacks">Preventing Open Redirection Attacks (C#)</a>.
+</li>
 
 </references>
 </qhelp>

java/ql/src/Security/CWE/CWE-601/examples/UrlRedirect.java

@@ -0,0 +1,6 @@
+public class UrlRedirect extends HttpServlet {
+  protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+    // BAD: a request parameter is incorporated without validation into a URL redirect
+    response.sendRedirect(request.getParameter("target"));
+  }
+}

java/ql/src/Security/CWE/CWE-601/examples/UrlRedirectGood.java

@@ -0,0 +1,16 @@
+public class UrlRedirect extends HttpServlet {
+  private static final List<String> VALID_REDIRECTS = Arrays.asList(
+    "http://cwe.mitre.org/data/definitions/601.html",
+    "http://cwe.mitre.org/data/definitions/79.html"
+  );
+
+  protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+    // GOOD: the request parameter is validated against a known list of strings
+    String target = request.getParameter("target");
+    if (VALID_REDIRECTS.contains(target)) {
+        response.sendRedirect(target);
+    } else {
+        response.sendRedirect("/error.html");
+    }
+  }
+}

java/ql/src/Security/CWE/CWE-601/examples/UrlRedirectGoodDomain.java

@@ -0,0 +1,18 @@
+public class UrlRedirect extends HttpServlet {
+  protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+    try {
+      String urlString = request.getParameter("page");
+      URI url = new URI(urlString);
+
+      if (!url.isAbsolute()) {
+        response.sendRedirect(url.toString()); // GOOD: The redirect is to a relative URL
+      }
+
+      if ("example.org".equals(url.getHost())) {
+        response.sendRedirect(url.toString()); // GOOD: The redirect is to a known host
+      }
+    } catch (URISyntaxException e) {
+        // handle exception
+    }
+  }
+}

java/ql/src/change-notes/2024-03-12-request-sanitizers.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added sanitizers for relative URLs, `List.contains()`, and checking the host of a URI to the `java/ssrf` and `java/unvalidated-url-redirection` queries.

java/ql/src/change-notes/2024-03-24-sensitive-log-whitelist-tokenimage.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Variables named `tokenImage` are no longer sources for  the `java/sensitive-log` query. This is because this variable name is used in parsing code generated by JavaCC, so it causes a large number of false positive alerts.

java/ql/src/utils/modeleditor/FrameworkModeEndpointsQuery.qll

@@ -8,7 +8,7 @@ private import ModelEditor
  * A class of effectively public callables from source code.
  */
 class PublicEndpointFromSource extends Endpoint, ModelApi {
-  override predicate isSource() { SourceSinkInterpretationInput::sourceElement(this, _, _) }
+  override predicate isSource() { SourceSinkInterpretationInput::sourceElement(this, _, _, _) }
 
-  override predicate isSink() { SourceSinkInterpretationInput::sinkElement(this, _, _) }
+  override predicate isSink() { SourceSinkInterpretationInput::sinkElement(this, _, _, _) }
 }

java/ql/src/utils/modelgenerator/internal/CaptureModels.qll

@@ -72,11 +72,11 @@ string captureQualifierFlow(TargetApiSpecific api) {
   result = ModelPrinting::asValueModel(api, qualifierString(), "ReturnValue")
 }
 
-private int accessPathLimit() { result = 2 }
+private int accessPathLimit0() { result = 2 }
 
 private newtype TTaintState =
-  TTaintRead(int n) { n in [0 .. accessPathLimit()] } or
-  TTaintStore(int n) { n in [1 .. accessPathLimit()] }
+  TTaintRead(int n) { n in [0 .. accessPathLimit0()] } or
+  TTaintStore(int n) { n in [1 .. accessPathLimit0()] }
 
 abstract private class TaintState extends TTaintState {
   abstract string toString();