Merge pull request #15895 from erik-krogh/url-java-qhelp

Java: update the url-redirection in the same style as the C# qhelp

Author: erik-krogh

csharp/ql/src/Security Features/CWE-601/UrlRedirect.qhelp

@@ -57,7 +57,7 @@ which is harmless but perhaps not intended. You can substitute your own domain (
 
 <li>
 OWASP:
-<a href="https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html">XSS
+<a href="https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html">
 Unvalidated Redirects and Forwards Cheat Sheet</a>.
 </li>
 <li>

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

@@ -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:
+</p>
+
+<sample src="examples/UrlRedirect.java"/>
 
-<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.
+<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="UrlRedirect.java" />
+<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>
 
+<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/UrlRedirect.qhelp

@@ -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/UrlRedirect.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/UrlRedirectGood.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
+    }
+  }
+}