Do not take information from request parameters which opens page up to XSS attacks...

git-svn-id: https://subversion.switch.ch/svn/shibboleth/java-idp/branches/REL_2@2763 ab3bd59b-922f-494d-bb5f-6f0a3c29deca

diff --git a/doc/RELEASE-NOTES.txt b/doc/RELEASE-NOTES.txt
index f308965..103573c 100644 (file)
--- a/doc/RELEASE-NOTES.txt
+++ b/doc/RELEASE-NOTES.txt
@@ -24,4 +24,5 @@ Changes in Release 2.1.0
 [SIDP-209] - Enforce SAML 2 metadata SPSSODescriptor/@AuthnRequestsSigned
 [SIDP-214] - Installer needs to put (at least) bcprov onto the calsspath before it runs ant
 [SIDP-222] - Template engine used by LDAP and database connectors throw an NPE on startup
-[SIDP-224] - Add version information in library JAR manifest and provide command line tool to view it
\ No newline at end of file
+[SIDP-224] - Add version information in library JAR manifest and provide command line tool to view it
+[SIDP-226] - Cross site scripting vulnerability
\ No newline at end of file

diff --git a/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java b/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java
index 7419c1a..50d3b82 100644 (file)
--- a/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java
+++ b/src/main/java/edu/internet2/middleware/shibboleth/idp/authn/provider/UsernamePasswordLoginServlet.java
@@ -29,23 +29,23 @@ import javax.security.auth.callback.NameCallback;
 import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.callback.UnsupportedCallbackException;
 import javax.security.auth.login.LoginException;
+import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.opensaml.util.URLBuilder;
 import org.opensaml.xml.util.DatatypeHelper;
 import org.opensaml.xml.util.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import edu.internet2.middleware.shibboleth.idp.authn.AuthenticationEngine;
-import edu.internet2.middleware.shibboleth.idp.authn.UsernamePrincipal;
 import edu.internet2.middleware.shibboleth.idp.authn.LoginHandler;
+import edu.internet2.middleware.shibboleth.idp.authn.UsernamePrincipal;
 
 /**
- * This servlet should be protected by a filter which populates REMOTE_USER. The serlvet will then set the remote user
+ * This Servlet should be protected by a filter which populates REMOTE_USER. The Servlet will then set the remote user
  * field in a LoginContext.
  */
 public class UsernamePasswordLoginServlet extends HttpServlet {
@@ -81,13 +81,19 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
     private final String passwordAttribute = "j_password";
 
     /** {@inheritDoc} */
-    public void init() {
+    public void init(ServletConfig config) throws ServletException {
+        super.init(config);
+        
         if (getInitParameter(jaasInitParam) != null) {
             jaasConfigName = getInitParameter(jaasInitParam);
         }
+        
         if (getInitParameter(loginPageInitParam) != null) {
             loginPage = getInitParameter(loginPageInitParam);
         }
+        if(!loginPage.startsWith("/")){
+            loginPage = "/" + loginPage;
+        }
     }
 
     /** {@inheritDoc} */
@@ -119,30 +125,26 @@ public class UsernamePasswordLoginServlet extends HttpServlet {
      */
     protected void redirectToLoginPage(HttpServletRequest request, HttpServletResponse response,
             List<Pair<String, String>> queryParams) {
-        try {
-            StringBuilder pathBuilder = new StringBuilder();
-            pathBuilder.append(request.getContextPath());
-            pathBuilder.append("/");
-            pathBuilder.append(loginPage);
-
-            URLBuilder urlBuilder = new URLBuilder();
-            urlBuilder.setScheme(request.getScheme());
-            urlBuilder.setHost(request.getServerName());
-            urlBuilder.setPort(request.getServerPort());
-            urlBuilder.setPath(pathBuilder.toString());
-
-            if (queryParams == null) {
-                queryParams = new ArrayList<Pair<String, String>>();
-            }
-
-            queryParams.add(new Pair<String, String>("actionUrl", request.getContextPath() + request.getServletPath()));
-            urlBuilder.getQueryParams().addAll(queryParams);
+       
+        String requestContext = DatatypeHelper.safeTrimOrNullString(request.getContextPath());
+        if(request == null){
+            requestContext = "/";
+        }
+        request.setAttribute("actionUrl", requestContext + request.getServletPath());
 
-            log.debug("Redirecting to login page {}", urlBuilder.buildURL());
-            response.sendRedirect(urlBuilder.buildURL());
-            return;
+        if(queryParams != null){
+            for(Pair<String, String> param : queryParams){
+                request.setAttribute(param.getFirst(), param.getSecond());
+            }
+        }
+        
+        try {
+            request.getRequestDispatcher(loginPage).forward(request, response);
+            log.debug("Redirecting to login page {}", loginPage);
         } catch (IOException ex) {
             log.error("Unable to redirect to login page.", ex);
+        }catch (ServletException ex){
+            log.error("Unable to redirect to login page.", ex);            
         }
     }
 

diff --git a/src/main/webapp/login.jsp b/src/main/webapp/login.jsp
index edd22ec..b2f5c4a 100644 (file)
--- a/src/main/webapp/login.jsp
+++ b/src/main/webapp/login.jsp
@@ -8,12 +8,12 @@
                <img src="<%= request.getContextPath() %>/images/logo.jpg" />
                <h2>Shibboleth Identity Provider Login</h2>
                
-               <% if ("true".equals(request.getParameter("loginFailed"))) { %>
-               <p>Authentication Failed</p>
+               <% if ("true".equals(request.getAttribute("loginFailed"))) { %>
+               <p><font color="red"Authentication Failed</font></p>
                <% } %>
                
-               <% if(request.getParameter("actionUrl") != null){ %>
-                   <form action="<%=request.getParameter("actionUrl")%>" method="post">
+               <% if(request.getAttribute("actionUrl") != null){ %>
+                   <form action="<%=request.getAttribute("actionUrl")%>" method="post">
                <% }else{ %>
                    <form action="j_security_check" method="post">
                <% } %>