From 68e84726ab9bc1c79fb9118f2c866b8a37421a73 Mon Sep 17 00:00:00 2001
From: Kohsuke Kawaguchi <kk@kohsuke.org>
Date: Sat, 17 Nov 2012 12:02:20 -0800
Subject: [PATCH] [FIXED SECURITY-45]

---
 core/src/main/java/hudson/Util.java           | 25 +++++++++++++++++++
 .../hudson/model/DirectoryBrowserSupport.java |  2 +-
 .../AuthenticationProcessingFilter2.java      |  4 +++
 core/src/test/java/hudson/UtilTest.java       | 10 ++++++++
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java
index a93ef736d0..782d94e8fe 100644
--- a/core/src/main/java/hudson/Util.java
+++ b/core/src/main/java/hudson/Util.java
@@ -1233,6 +1233,31 @@ public class Util {
         return s==null ? s : s.intern();
     }
 
+    /**
+     * Return true if the systemId denotes an absolute URI .
+     *
+     * The same algorithm can be seen in {@link URI}, but
+     * implementing this by ourselves allow it to be more lenient about
+     * escaping of URI.
+     */
+    public static boolean isAbsoluteUri(String uri) {
+        int idx = uri.indexOf(':');
+        if (idx<0)  return false;   // no ':'. can't be absolute
+
+        // #, ?, and / must not be before ':'
+        return idx<_indexOf(uri, '#') && idx<_indexOf(uri,'?') && idx<_indexOf(uri,'/');
+    }
+
+    /**
+     * Works like {@link String#indexOf(int)} but 'not found' is returned as s.length(), not -1.
+     * This enables more straight-forward comparison.
+     */
+    private static int _indexOf(String s, char ch) {
+        int idx = s.indexOf(ch);
+        if (idx<0)  return s.length();
+        return idx;
+    }
+
     /**
      * Loads a key/value pair string as {@link Properties}
      * @since 1.392
diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java
index 3d9753a6f9..d924710c76 100644
--- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java
+++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java
@@ -137,7 +137,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) {
+        if(pattern!=null && !Util.isAbsoluteUri(pattern)) {// avoid open redirect
             rsp.sendRedirect2(pattern);
             return;
         }
diff --git a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java
index faef1690a6..7f282199e8 100644
--- a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java
+++ b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java
@@ -31,6 +31,7 @@ import java.io.IOException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import hudson.Util;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.AuthenticationException;
 import org.acegisecurity.ui.webapp.AuthenticationProcessingFilter;
@@ -51,6 +52,9 @@ public class AuthenticationProcessingFilter2 extends AuthenticationProcessingFil
         if (targetUrl == null)
             return getDefaultTargetUrl();
 
+        if (Util.isAbsoluteUri(targetUrl))
+            return "."; // avoid open redirect
+
         // URL returned from determineTargetUrl() is resolved against the context path,
         // whereas the "from" URL is resolved against the top of the website, so adjust this.
         if(targetUrl.startsWith(request.getContextPath()))
diff --git a/core/src/test/java/hudson/UtilTest.java b/core/src/test/java/hudson/UtilTest.java
index 8461ee2ee7..c4738cbb6d 100644
--- a/core/src/test/java/hudson/UtilTest.java
+++ b/core/src/test/java/hudson/UtilTest.java
@@ -290,4 +290,14 @@ public class UtilTest {
 			}
 		}
     }
+
+    public void testIsAbsoluteUri() {
+        assertTrue(Util.isAbsoluteUri("http://foobar/"));
+        assertTrue(Util.isAbsoluteUri("mailto:kk@kohsuke.org"));
+        assertTrue(Util.isAbsoluteUri("d123://test/"));
+        assertFalse(Util.isAbsoluteUri("foo/bar/abc:def"));
+        assertFalse(Util.isAbsoluteUri("foo?abc:def"));
+        assertFalse(Util.isAbsoluteUri("foo#abc:def"));
+        assertFalse(Util.isAbsoluteUri("foo/bar"));
+    }
 }
-- 
GitLab