KNOX-3132: Improve URL checks for originalUrl

bonampak · bonampak · commit 7c7605de65ce · 2025-04-23T14:59:49.000+02:00
diff --git a/gateway-applications/pom.xml b/gateway-applications/pom.xml
@@ -58,6 +58,10 @@
             <groupId>org.apache.knox</groupId>
             <artifactId>knox-webshell-ui</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-text</artifactId>
+        </dependency>
     </dependencies>
 
     <build>
diff --git a/gateway-applications/src/main/resources/applications/knoxauth/app/logout.jsp b/gateway-applications/src/main/resources/applications/knoxauth/app/logout.jsp
@@ -21,6 +21,7 @@
 <%@ page import="org.apache.knox.gateway.config.GatewayConfig" %>
 <%@ page import="java.net.MalformedURLException" %>
 <%@ page import="org.apache.knox.gateway.util.Urls" %>
+<%@ page import="org.apache.commons.text.StringEscapeUtils" %>
 
 <!DOCTYPE html>
 <!--[if lt IE 7]><html class="no-js lt-ie9 lt-ie8 lt-ie7"><![endif]-->
@@ -93,6 +94,9 @@
         if (origUrl != null) {
           validRedirect = RegExUtils.checkWhitelist(whitelist, origUrl);
         }
+        if (validRedirect) {
+            validRedirect = Urls.isValidURL(originalUrl);
+        }
         if (("1".equals(request.getParameter("returnToApp")))) {
           if (validRedirect) {
             response.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
@@ -178,7 +182,7 @@
                 the application. If your previously established SSO session is still valid then
                 you will likely be automatically logged into your application. Otherwise, you
                 will be required to login again.
-                <a href="?returnToApp=1&originalUrl=<%= originalUrl %>" >Return to Application</a>
+                <a href="?returnToApp=1&originalUrl=<%= StringEscapeUtils.escapeHtml4(originalUrl) %>" >Return to Application</a>
               </p>
         <%
             if (globalLogoutPageURL != null && !globalLogoutPageURL.isEmpty()) {
diff --git a/gateway-applications/src/main/resources/applications/knoxauth/app/redirecting.jsp b/gateway-applications/src/main/resources/applications/knoxauth/app/redirecting.jsp
@@ -20,6 +20,7 @@
 <%@ page import="org.apache.knox.gateway.util.RegExUtils" %>
 <%@ page import="org.apache.knox.gateway.util.Urls" %>
 <%@ page import="org.apache.knox.gateway.util.WhitelistUtils" %>
+<%@ page import="org.apache.commons.text.StringEscapeUtils" %>
 
 <!DOCTYPE html>
 <!--[if lt IE 7]><html class="no-js lt-ie9 lt-ie8 lt-ie7"><![endif]-->
@@ -57,6 +58,9 @@
           // if not a well formed URL then not a valid redirect
           validRedirect = false;
         }
+        if (validRedirect) {
+            validRedirect = Urls.isValidURL(originalUrl);
+        }
         if (validRedirect) {
 	      Topology topology = (Topology)request.getSession().getServletContext().getAttribute("org.apache.knox.gateway.topology");
           String whitelist = null;
@@ -82,7 +86,7 @@
     document.addEventListener("load", redirectOnLoad());
 
     function redirectOnLoad() {
-      var originalUrl = "<%= originalUrl %>";
+      var originalUrl = "<%= StringEscapeUtils.escapeEcmaScript(originalUrl) %>";
       if (originalUrl != null) {
         redirect(originalUrl);
       }
@@ -103,7 +107,7 @@
           <div style="background: white;" class="l-logo">
                 <img src="images/loading.gif" alt="Knox logo" style="text-align:center;width: 2%; height: 2%">
             </div>
-              <p style="color: white;display: block">Loading should complete in few a seconds. If not, click <a href="<%= originalUrl %>">here</a></p>
+              <p style="color: white;display: block">Loading should complete in few a seconds. If not, click <a href="<%= StringEscapeUtils.escapeHtml4(originalUrl) %>">here</a></p>
         <%
         } else {
         %>
diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/Urls.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/Urls.java
@@ -19,6 +19,7 @@
 
 import java.io.UnsupportedEncodingException;
 import java.net.MalformedURLException;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.util.regex.Matcher;
@@ -81,6 +82,20 @@ public static boolean containsUserInfo(String url) throws MalformedURLException
     return (new URL(url).getUserInfo() != null);
   }
 
+  /**
+   * Checks if a given URL is valid according to RFC2396.
+   * @param url a URL
+   * @return true if URL conforms to RFC2396, false otherwise.
+   */
+  public static boolean isValidURL(String url) {
+    try {
+      new URL(url).toURI();
+      return true;
+    } catch (MalformedURLException | URISyntaxException e) {
+      return false;
+    }
+  }
+
   /**
    * Compute the domain name from an URL.
    *
diff --git a/gateway-util-common/src/test/java/org/apache/knox/gateway/util/UrlsTest.java b/gateway-util-common/src/test/java/org/apache/knox/gateway/util/UrlsTest.java
@@ -26,6 +26,14 @@
 
 public class UrlsTest {
 
+  private static final String INVALID_ORIGINAL_URL = "https://localhost:8443/gateway/homepage/homeisyr\">";
+  private static final String VALID_LOCAL_ORIGINAL_URL = "https://localhost:8443/gateway/homepage/home";
+  private static final String VALID_ORIGINAL_URL = "https://knoxhost.site:8443/gateway/homepage/home";
+  private static final String VALID_ORIGINAL_URL_SOLR = "https://knoxhost.site:8443/gateway/cdp-proxy/solr/";
+  private static final String LOGOUT_LOCAL_ORIGINAL_URL = "https://localhost:8443/gateway/homepage/home?profile=token";
+  private static final String LOGOUT_ORIGINAL_URL_WITH_PARAMETERS = "https://knoxhost.site:8443/gateway/homepage/session/api/v1/sessioninfo?logoutPageProfile=token&logoutPageTopologies=cdp-proxy-token";
+  private static final String LOGOUT_ORIGINAL_URL_WITH_PARAMETERS_ENCODED = "https://knoxhost.site:8443/gateway/homepage/session/api/v1/sessioninfo%3FlogoutPageProfile=token%26logoutPageTopologies=cdp-proxy-token";
+
   /*
    * Domain name creation follows the following algorithm:
    * 1. if the incoming request hostname endsWith a configured domain suffix return the suffix - with prefixed dot
@@ -101,4 +109,16 @@ public void testContainsUserInfo() throws Exception {
     assertTrue(Urls.containsUserInfo( "https://www.local.com:8443aa@google.com"));
     assertFalse(Urls.containsUserInfo( "https://www.local.com:8443/google.com"));
   }
+
+  @Test
+  public void testValidUrl() throws Exception {
+    assertFalse(Urls.isValidURL(INVALID_ORIGINAL_URL));
+    assertTrue(Urls.isValidURL(VALID_LOCAL_ORIGINAL_URL));
+    assertTrue(Urls.isValidURL(VALID_ORIGINAL_URL));
+    assertTrue(Urls.isValidURL(VALID_ORIGINAL_URL_SOLR));
+    assertTrue(Urls.isValidURL(LOGOUT_LOCAL_ORIGINAL_URL));
+    assertTrue(Urls.isValidURL(LOGOUT_ORIGINAL_URL_WITH_PARAMETERS));
+    assertTrue(Urls.isValidURL(LOGOUT_ORIGINAL_URL_WITH_PARAMETERS_ENCODED));
+  }
+
 }
