From e4078933f854c72a65d3d5619163da0ece874f2d Mon Sep 17 00:00:00 2001 From: mhajas Date: Tue, 21 Jul 2020 13:14:27 +0200 Subject: [PATCH] KEYCLOAK-14828 Disable DTD for SAML XML parser (cherry picked from commit 37de7de78b2ae0eebee97fe917642bb849325f86) --- .../saml/common/util/StaxParserUtil.java | 1 + .../saml/common/util/StaxParserUtilTest.java | 30 +++++++++ .../testsuite/saml/SamlXMLAttacksTest.java | 67 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlXMLAttacksTest.java diff --git a/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java b/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java index e4bd59d632..10622bb012 100755 --- a/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java @@ -895,6 +895,7 @@ public class StaxParserUtil { xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); xmlInputFactory.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, Boolean.TRUE); xmlInputFactory.setProperty(XMLInputFactory.IS_COALESCING, Boolean.TRUE); + xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE); return xmlInputFactory; } finally { diff --git a/saml-core/src/test/java/org/keycloak/saml/common/util/StaxParserUtilTest.java b/saml-core/src/test/java/org/keycloak/saml/common/util/StaxParserUtilTest.java index 14438c4eeb..7a4c277704 100644 --- a/saml-core/src/test/java/org/keycloak/saml/common/util/StaxParserUtilTest.java +++ b/saml-core/src/test/java/org/keycloak/saml/common/util/StaxParserUtilTest.java @@ -214,4 +214,34 @@ public class StaxParserUtilTest { Assert.fail(String.valueOf(reader.nextEvent())); } + @Test + public void testXMLBombAttack() throws XMLStreamException { + String xml = "" + + "" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + "]>" + + "&lol9;"; + + XMLEventReader reader = StaxParserUtil.getXMLEventReader(IOUtils.toInputStream(xml, Charset.defaultCharset())); + + reader.nextEvent(); // + reader.nextEvent(); // + try { + StaxParserUtil.getDOMElement(reader); + } catch (ParsingException exception) { + // DTD should be disabled for SAML Parser, therefore this should fail with following error message + assertThat("", exception.getMessage(), containsString("The entity \"lol9\" was referenced, but not declared")); + } + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlXMLAttacksTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlXMLAttacksTest.java new file mode 100644 index 0000000000..a7b50a1200 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlXMLAttacksTest.java @@ -0,0 +1,67 @@ +package org.keycloak.testsuite.saml; + +import org.apache.http.NameValuePair; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.message.BasicNameValuePair; +import org.junit.Test; +import org.keycloak.saml.common.constants.GeneralConstants; +import org.keycloak.saml.processing.web.util.PostBindingUtil; + +import java.io.UnsupportedEncodingException; +import java.util.LinkedList; +import java.util.List; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertThat; +import static org.keycloak.testsuite.util.Matchers.bodyHC; + +public class SamlXMLAttacksTest extends AbstractSamlTest { + + @Test(timeout = 4000) + public void testXMLBombAttackResistance() throws Exception { + + String bombDoctype = "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "]>"; + + String samlAuthnRequest = "" + + "" + SAML_CLIENT_ID_SALES_POST + "&lol9;" + + ""; + + try (CloseableHttpClient client = HttpClientBuilder.create().build()) { + HttpPost post = new HttpPost(getAuthServerSamlEndpoint(REALM_NAME)); + + List parameters = new LinkedList<>(); + String encoded = PostBindingUtil.base64Encode(bombDoctype + samlAuthnRequest); + parameters.add(new BasicNameValuePair(GeneralConstants.SAML_REQUEST_KEY, encoded)); + + UrlEncodedFormEntity formEntity; + try { + formEntity = new UrlEncodedFormEntity(parameters, "UTF-8"); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + + post.setEntity(formEntity); + + try (CloseableHttpResponse response = client.execute(post)) { + assertThat(response, bodyHC(containsString("Invalid Request"))); + } + } + } + +}