Fix for SECURITY-2979 / CVE-2023-50771

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

@@ -730,6 +730,29 @@ protected AuthorizationCodeFlow buildAuthorizationCodeFlow() {
         return builder.build();
     }
 
+    protected String getValidRedirectUrl(String url) {
+        if (url != null && !url.isEmpty()) {
+            // Check if the URL is relative and starts with a slash
+            if (url.startsWith("/")) {
+                return getRootUrl() + url; // Convert to absolute URL
+            }
+            // If not relative, then check if it's a valid absolute URL
+            try {
+                URL parsedUrl = new URL(url);
+                String host = parsedUrl.getHost();
+                String expectedHost = new URL(getRootUrl()).getHost();
+                // Check if the host matches the Jenkins domain
+                if (host.equals(expectedHost)) {
+                    return url; // The URL is absolute and valid
+                }
+            } catch (MalformedURLException e) {
+                // Invalid absolute URL, will return root URL
+            }
+        }
+        // If the URL is null, empty, or invalid, return the root URL
+        return getRootUrl();
+    }
+
     /**
      * Handles the the securityRealm/commenceLogin resource and sends the user off to the IdP
      * @param from the relative URL to the page that the user has just come from
@@ -741,7 +764,7 @@ public HttpResponse doCommenceLogin(@QueryParameter String from, @Header("Refere
         // reload config if needed
         loadWellKnownOpenIDConfigurationUrl();
 
-        final String redirectOnFinish = determineRedirectTarget(from, referer);
+        final String redirectOnFinish = getValidRedirectUrl(from != null ? from : referer);
 
         final AuthorizationCodeFlow flow = this.buildAuthorizationCodeFlow();
 
@@ -1034,18 +1057,6 @@ private String getRootUrl() {
         }
     }
 
-    private String determineRedirectTarget(@QueryParameter String from, @Header("Referer") String referer) {
-        String target;
-        if (from != null) {
-            target = from;
-        } else if (referer != null) {
-            target = referer;
-        } else {
-            target = getRootUrl();
-        }
-        return target;
-    }
-
     private String buildOAuthRedirectUrl() throws NullPointerException {
         String rootUrl = getRootUrl();
         if (rootUrl == null) {

src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java

@@ -4,6 +4,8 @@
 import com.github.tomakehurst.wiremock.junit.WireMockRule;
 import hudson.util.Secret;
 import java.io.IOException;
+import java.net.MalformedURLException;
+
 import org.acegisecurity.AuthenticationManager;
 import org.acegisecurity.BadCredentialsException;
 import org.acegisecurity.GrantedAuthority;
@@ -16,6 +18,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 
 public class OicSecurityRealmTest {
 
@@ -83,4 +86,17 @@ public void testShouldSetNullClientSecretWhenSecretIsNone() throws IOException {
             .build();
         assertEquals("none", Secret.toString(realm.getClientSecret()));
     }
+
+    @Test
+    public void testGetValidRedirectUrl() throws IOException {
+        String rootUrl = "http://localhost:" + wireMockRule.port() + "/jenkins/";
+
+        TestRealm realm = new TestRealm.Builder(wireMockRule)
+                .WithMinimalDefaults().build();
+        assertEquals(rootUrl + "foo", realm.getValidRedirectUrl("/foo"));
+        assertEquals(rootUrl + "bar", realm.getValidRedirectUrl(rootUrl + "bar"));
+        assertEquals(rootUrl, realm.getValidRedirectUrl(null));
+        assertEquals(rootUrl, realm.getValidRedirectUrl(""));
+        assertThrows(MalformedURLException.class, () -> realm.getValidRedirectUrl("foobar"));
+    }
 }