KEYCLOAK-14828 Disable DTD for SAML XML parser

(cherry picked from commit 37de7de78b2ae0eebee97fe917642bb849325f86)
This commit is contained in:
mhajas 2020-07-21 13:14:27 +02:00 committed by Hynek Mlnařík
parent 267be2d416
commit e4078933f8
3 changed files with 98 additions and 0 deletions

View file

@ -895,6 +895,7 @@ public class StaxParserUtil {
xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE); xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE);
xmlInputFactory.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, Boolean.TRUE); xmlInputFactory.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, Boolean.TRUE);
xmlInputFactory.setProperty(XMLInputFactory.IS_COALESCING, Boolean.TRUE); xmlInputFactory.setProperty(XMLInputFactory.IS_COALESCING, Boolean.TRUE);
xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
return xmlInputFactory; return xmlInputFactory;
} finally { } finally {

View file

@ -214,4 +214,34 @@ public class StaxParserUtilTest {
Assert.fail(String.valueOf(reader.nextEvent())); Assert.fail(String.valueOf(reader.nextEvent()));
} }
@Test
public void testXMLBombAttack() throws XMLStreamException {
String xml = "<?xml version=\"1.0\"?>" +
"<!DOCTYPE lolz [" +
" <!ENTITY lol \"lol\">" +
" <!ELEMENT lolz (#PCDATA)>" +
" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">" +
" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">" +
" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">" +
" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">" +
" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">" +
" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">" +
" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">" +
" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">" +
" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">" +
"]>" +
"<lolz>&lol9;</lolz>";
XMLEventReader reader = StaxParserUtil.getXMLEventReader(IOUtils.toInputStream(xml, Charset.defaultCharset()));
reader.nextEvent(); // <?xml version="1.0"?>
reader.nextEvent(); // <!DOCTYPE lolz [ ........ ]>
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"));
}
}
} }

View file

@ -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 = "<!DOCTYPE AuthnRequest [" +
" <!ENTITY lol \"lol\">" +
"<!ELEMENT AuthnRequest (#PCDATA)>" +
"<!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">" +
"<!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">" +
"<!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">" +
"<!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">" +
"<!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">" +
"<!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">" +
"<!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">" +
"<!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">" +
"<!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">" +
"]>";
String samlAuthnRequest = "<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"a123\" Version=\"2.0\" IssueInstant=\"2014-07-16T23:52:45Z\" >" +
"<saml:Issuer>" + SAML_CLIENT_ID_SALES_POST + "&lol9;</saml:Issuer>" +
"</samlp:AuthnRequest>";
try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
HttpPost post = new HttpPost(getAuthServerSamlEndpoint(REALM_NAME));
List<NameValuePair> 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")));
}
}
}
}