[SECURITY-798] Prevent open redirect

Wadeck · Wadeck · commit a77729de4217 · 2018-05-28T12:45:31.000+02:00
- use the from in priority as it is managed directly inside the main layout
- otherwise fallback to the referer header value
- in all case, check the URL is either relative or inside Jenkins
diff --git a/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java
@@ -79,6 +79,7 @@ of this software and associated documentation files (the "Software"), to deal
 import org.kohsuke.stapler.HttpRedirect;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.DataRetrievalFailureException;
@@ -333,9 +334,18 @@ public String getOauthScopes() {
         return oauthScopes;
     }
 
-    public HttpResponse doCommenceLogin(StaplerRequest request, @Header("Referer") final String referer)
+    public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter String from, @Header("Referer") final String referer)
             throws IOException {
-        request.getSession().setAttribute(REFERER_ATTRIBUTE,referer);
+        String redirectOnFinish;
+        if (from != null && Util.isSafeToRedirectTo(from)) {
+            redirectOnFinish = from;
+        } else if (referer != null && (referer.startsWith(Jenkins.getInstance().getRootUrl()) || Util.isSafeToRedirectTo(referer))) {
+            redirectOnFinish = referer;
+        } else {
+            redirectOnFinish = Jenkins.getInstance().getRootUrl();
+        }
+
+        request.getSession().setAttribute(REFERER_ATTRIBUTE, redirectOnFinish);
 
         Set<String> scopes = new HashSet<>();
         for (GitHubOAuthScope s : getJenkins().getExtensionList(GitHubOAuthScope.class)) {
