Merge pull request keycloak#93 from mposolda/RHSSO-1580

RHSSO-1580 XSS-Vulnerability with response_mode=form_post
vmuzikar · Sep 7, 2018 · dce15c8 · dce15c8
2 parents 31979cd + ccc8f4f
commit dce15c8
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 3 deletions.
diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java
@@ -18,6 +18,7 @@
 package org.keycloak.protocol.oidc.utils;
 
 import org.keycloak.common.util.Encode;
+import org.keycloak.common.util.HtmlUtils;
 import org.keycloak.common.util.KeycloakUriBuilder;
 
 import javax.ws.rs.core.MediaType;
@@ -143,8 +144,11 @@ public Response build() {
             builder.append("    <FORM METHOD=\"POST\" ACTION=\"" + redirectUri.toString() + "\">");
 
             for (Map.Entry<String, String> param : params.entrySet()) {
-                builder.append("  <INPUT TYPE=\"HIDDEN\" NAME=\"").append(param.getKey())
-                        .append("\" VALUE=\"").append(param.getValue()).append("\" />");
+                builder.append("  <INPUT TYPE=\"HIDDEN\" NAME=\"")
+                        .append(param.getKey())
+                        .append("\" VALUE=\"")
+                        .append(HtmlUtils.escapeAttribute(param.getValue()))
+                        .append("\" />");
             }
 
             builder.append("      <NOSCRIPT>");

diff --git a/...-providers/src/main/java/org/keycloak/testsuite/rest/TestApplicationResourceProvider.java b/...-providers/src/main/java/org/keycloak/testsuite/rest/TestApplicationResourceProvider.java
@@ -20,6 +20,7 @@
 import org.jboss.resteasy.annotations.cache.NoCache;
 import org.jboss.resteasy.spi.HttpRequest;
 import org.jboss.resteasy.spi.ResteasyProviderFactory;
+import org.keycloak.common.util.HtmlUtils;
 import org.keycloak.jose.jws.JWSInput;
 import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.models.KeycloakSession;
@@ -135,7 +136,10 @@ public String post(@PathParam("action") String action) {
         HttpRequest request = ResteasyProviderFactory.getContextData(HttpRequest.class);
         MultivaluedMap<String, String> formParams = request.getDecodedFormParameters();
         for (String paramName : formParams.keySet()) {
-            sb.append(paramName).append(": ").append("<span id=\"").append(paramName).append("\">").append(formParams.getFirst(paramName)).append("</span><br>");
+            sb.append(paramName).append(": ").append("<span id=\"")
+                    .append(paramName).append("\">")
+                    .append(HtmlUtils.escapeAttribute(formParams.getFirst(paramName)))
+                    .append("</span><br>");
         }
         sb.append("<br>");
 

diff --git a/...quillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java b/...quillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java
@@ -150,4 +150,22 @@ public void authorizationRequestFormPostResponseMode() throws IOException {
         String codeId = events.expectLogin().assertEvent().getDetails().get(Details.CODE_ID);
     }
 
+
+    @Test
+    public void authorizationRequestFormPostResponseModeWithCustomState() throws IOException {
+        oauth.responseMode(OIDCResponseMode.FORM_POST.toString().toLowerCase());
+        oauth.stateParamHardcoded("\"><foo>bar_baz(2)far</foo>");
+        oauth.doLoginGrant("test-user@localhost", "password");
+
+        String sources = driver.getPageSource();
+        System.out.println(sources);
+
+        String code = driver.findElement(By.id("code")).getText();
+        String state = driver.findElement(By.id("state")).getText();
+
+        assertEquals("\"><foo>bar_baz(2)far</foo>", state);
+
+        String codeId = events.expectLogin().assertEvent().getDetails().get(Details.CODE_ID);
+    }
+
 }