From 2ed0c046dfbb2003a17df27c53777e72c6eaff25 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Mon, 25 Apr 2016 19:50:53 +0200 Subject: [PATCH] [FIX SECURITY-276] Don't allow open redirect using scheme-rel. URL --- core/src/main/java/hudson/Util.java | 14 ++++++++++++++ .../java/hudson/model/DirectoryBrowserSupport.java | 2 +- .../hudson/model/ParametersDefinitionProperty.java | 2 +- .../security/AuthenticationProcessingFilter2.java | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 3a1ba13864..e9f09e8187 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -77,6 +77,8 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.apache.commons.codec.digest.DigestUtils; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Various utility methods that don't have more proper home. @@ -1454,7 +1456,12 @@ public class Util { * The same algorithm can be seen in {@link URI}, but * implementing this by ourselves allow it to be more lenient about * escaping of URI. + * + * @deprecated Use {@code isAbsoluteOrSchemeRelativeUri} instead if your goal is to prevent open redirects */ + @Deprecated + @RestrictedSince("1.651.2 / 2.TODO") + @Restricted(NoExternalUse.class) public static boolean isAbsoluteUri(@Nonnull String uri) { int idx = uri.indexOf(':'); if (idx<0) return false; // no ':'. can't be absolute @@ -1463,6 +1470,13 @@ public class Util { return idx<_indexOf(uri, '#') && idx<_indexOf(uri,'?') && idx<_indexOf(uri,'/'); } + /** + * Return true iff the parameter denotes an absolute URI, or a scheme-relative URI. + */ + public static boolean isAbsoluteOrSchemeRelativeUri(@Nonnull String uri) { + return isAbsoluteUri(uri) || uri.startsWith("//"); + } + /** * Works like {@link String#indexOf(int)} but 'not found' is returned as s.length(), not -1. * This enables more straight-forward comparison. diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index 7c562dcb12..550ee5ad80 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -158,7 +158,7 @@ public final class DirectoryBrowserSupport implements HttpResponse { String pattern = req.getParameter("pattern"); if(pattern==null) pattern = req.getParameter("path"); // compatibility with Hudson<1.129 - if(pattern!=null && !Util.isAbsoluteUri(pattern)) {// avoid open redirect + if(pattern!=null && !Util.isAbsoluteOrSchemeRelativeUri(pattern)) {// avoid open redirect rsp.sendRedirect2(pattern); return; } diff --git a/core/src/main/java/hudson/model/ParametersDefinitionProperty.java b/core/src/main/java/hudson/model/ParametersDefinitionProperty.java index 13a6156b5c..e271bb5bd9 100644 --- a/core/src/main/java/hudson/model/ParametersDefinitionProperty.java +++ b/core/src/main/java/hudson/model/ParametersDefinitionProperty.java @@ -158,7 +158,7 @@ public class ParametersDefinitionProperty extends JobProperty> getJob(), delay.getTime(), new ParametersAction(values), new CauseAction(new Cause.UserIdCause())); if (item!=null) { String url = formData.optString("redirectTo"); - if (url==null || Util.isAbsoluteUri(url)) // avoid open redirect + if (url==null || Util.isAbsoluteOrSchemeRelativeUri(url)) // avoid open redirect url = req.getContextPath()+'/'+item.getUrl(); rsp.sendRedirect(formData.optInt("statusCode",SC_CREATED), url); } else diff --git a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java index de1a0cb3ce..8d6fec50dc 100644 --- a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java +++ b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java @@ -53,7 +53,7 @@ public class AuthenticationProcessingFilter2 extends AuthenticationProcessingFil if (targetUrl == null) return getDefaultTargetUrl(); - if (Util.isAbsoluteUri(targetUrl)) + if (Util.isAbsoluteOrSchemeRelativeUri(targetUrl)) return "."; // avoid open redirect // URL returned from determineTargetUrl() is resolved against the context path,