diff --git a/jspwiki-util/src/main/java/org/apache/wiki/util/HttpUtil.java b/jspwiki-util/src/main/java/org/apache/wiki/util/HttpUtil.java index 71d8dbfe89..fc77cce033 100644 --- a/jspwiki-util/src/main/java/org/apache/wiki/util/HttpUtil.java +++ b/jspwiki-util/src/main/java/org/apache/wiki/util/HttpUtil.java @@ -292,8 +292,12 @@ public static void clearCookie( final HttpServletResponse response, final String /** * Generates an absolute URL based on the given HttpServletRequest and a relative URL. - * This method takes into account various headers like X-Forwarded-Host, X-Forwarded-Proto, - * and X-Forwarded-Server to construct the absolute URL. + * Only the container-resolved scheme, server name and port are used; forwarded headers + * such as {@code X-Forwarded-Host}, {@code X-Forwarded-Proto} and {@code X-Forwarded-Server} + * are deliberately ignored, because they are client-supplied and must only be honored by + * the container itself (e.g. Tomcat's RemoteIpValve) after validating that they were set + * by a trusted proxy. Trusting them here allowed any client to poison generated URLs + * (e.g. the shared RSS feed cache and e-mailed login links). * * @param request The HttpServletRequest object, used to obtain scheme, server name, and port. * @param relativeUrl The relative URL to be appended to the base URL. Can be null. @@ -301,31 +305,21 @@ public static void clearCookie( final HttpServletResponse response, final String * @since 2.12.2 */ public static String getAbsoluteUrl(final HttpServletRequest request, final String relativeUrl) { - StringBuilder baseUrl = new StringBuilder(); + final StringBuilder baseUrl = new StringBuilder(); - // Check for proxy headers - final String forwardedHost = request.getHeader("X-Forwarded-Host"); - final String forwardedProto = request.getHeader("X-Forwarded-Proto"); - final String forwardedServer = request.getHeader("X-Forwarded-Server"); + // Use only container-resolved values. X-Forwarded-* headers are attacker-controlled + // unless validated against a trusted proxy list, which is the container's job. + final String scheme = request.getScheme(); + final String serverName = request.getServerName(); + final int port = request.getServerPort(); - if (forwardedHost != null && forwardedProto != null) { - baseUrl.append(forwardedProto).append("://").append(forwardedHost); - } else if (forwardedServer != null && forwardedProto != null) { - baseUrl.append(forwardedProto).append("://").append(forwardedServer); - } else { - // Fallback to HttpServletRequest - final String scheme = request.getScheme(); - final String serverName = request.getServerName(); - final int port = request.getServerPort(); - - baseUrl.append(scheme).append("://").append(serverName); - - // Include port only if it's not the default port for the scheme - if ((URIScheme.HTTP.same(scheme) && port != 80) - || (URIScheme.HTTPS.same(scheme) && port != 443)) { - baseUrl.append(':'); - baseUrl.append(port); - } + baseUrl.append(scheme).append("://").append(serverName); + + // Include port only if it's not the default port for the scheme + if ((URIScheme.HTTP.same(scheme) && port != 80) + || (URIScheme.HTTPS.same(scheme) && port != 443)) { + baseUrl.append(':'); + baseUrl.append(port); } if (relativeUrl != null) { diff --git a/jspwiki-util/src/test/java/org/apache/wiki/util/HttpUtilTest.java b/jspwiki-util/src/test/java/org/apache/wiki/util/HttpUtilTest.java index 551464cdf2..becd4b27f1 100644 --- a/jspwiki-util/src/test/java/org/apache/wiki/util/HttpUtilTest.java +++ b/jspwiki-util/src/test/java/org/apache/wiki/util/HttpUtilTest.java @@ -127,26 +127,32 @@ public void testGetAbsoluteUrlWithDefaultHttpsPort() { } @Test - public void testGetAbsoluteUrlWithForwardedHostAndProto() { + public void testGetAbsoluteUrlIgnoresForwardedHostAndProto() { HttpServletRequest request = mock(HttpServletRequest.class); - when(request.getHeader("X-Forwarded-Host")).thenReturn("proxyhost"); + when(request.getHeader("X-Forwarded-Host")).thenReturn("evil.example.com"); when(request.getHeader("X-Forwarded-Proto")).thenReturn("https"); + when(request.getScheme()).thenReturn("http"); + when(request.getServerName()).thenReturn("localhost"); + when(request.getServerPort()).thenReturn(8080); String relativeUrl = "/login"; - String expected = "https://proxyhost/login"; + String expected = "http://localhost:8080/login"; String actual = HttpUtil.getAbsoluteUrl(request, relativeUrl); assertEquals(expected, actual); } @Test - public void testGetAbsoluteUrlWithForwardedServerAndProto() { + public void testGetAbsoluteUrlIgnoresForwardedServerAndProto() { HttpServletRequest request = mock(HttpServletRequest.class); - when(request.getHeader("X-Forwarded-Server")).thenReturn("proxyserver"); + when(request.getHeader("X-Forwarded-Server")).thenReturn("evil.example.com"); when(request.getHeader("X-Forwarded-Proto")).thenReturn("https"); + when(request.getScheme()).thenReturn("http"); + when(request.getServerName()).thenReturn("localhost"); + when(request.getServerPort()).thenReturn(8080); String relativeUrl = "/login"; - String expected = "https://proxyserver/login"; + String expected = "http://localhost:8080/login"; String actual = HttpUtil.getAbsoluteUrl(request, relativeUrl); assertEquals(expected, actual); @@ -176,7 +182,7 @@ public void testGetAbsoluteUrlWithAllHeaders() { when(request.getServerName()).thenReturn("localhost"); when(request.getServerPort()).thenReturn(443); - String expected = "forwardedProto://forwardedHost"; + String expected = "https://localhost"; String actual = HttpUtil.getAbsoluteUrl(request); assertEquals(expected, actual); diff --git a/jspwiki-war/src/main/webapp/rss.jsp b/jspwiki-war/src/main/webapp/rss.jsp index bf5eac4cab..01b7c2bb9c 100644 --- a/jspwiki-war/src/main/webapp/rss.jsp +++ b/jspwiki-war/src/main/webapp/rss.jsp @@ -134,7 +134,11 @@ // // TODO: Figure out if it would be a good idea to use a disk-based cache here. // - String hashKey = wikipage.getName()+";"+mode+";"+type+";"+latest.getTime(); + // The request origin is part of the key because the generated feed embeds + // absolute URLs derived from it; a cached copy must never be served for a + // different origin than the one it was generated for. + String origin = request.getScheme()+"://"+request.getServerName()+":"+request.getServerPort(); + String hashKey = wikipage.getName()+";"+mode+";"+type+";"+latest.getTime()+";"+origin; String rss = "";