Skip to content

Commit

Permalink
Disallow DOCTYPE element in SAML responses to prevent XXE vulnerabili…
Browse files Browse the repository at this point in the history
…ties (#38)
  • Loading branch information
malaporte authored Sep 18, 2019
1 parent 210558f commit 25b0320
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
language: java

jdk:
- oraclejdk8
- openjdk8

cache:
directories:
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/com/coveo/saml/SamlClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.io.Reader;
import java.io.StringReader;
import java.io.StringWriter;
import java.io.UnsupportedEncodingException;
import java.nio.charset.StandardCharsets;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
Expand All @@ -60,6 +59,9 @@

import net.shibboleth.utilities.java.support.component.ComponentInitializationException;

import static com.sun.org.apache.xerces.internal.impl.Constants.DISALLOW_DOCTYPE_DECL_FEATURE;
import static com.sun.org.apache.xerces.internal.impl.Constants.XERCES_FEATURE_PREFIX;

public class SamlClient {
private static final Logger logger = LoggerFactory.getLogger(SamlClient.class);
private static boolean initializedOpenSaml = false;
Expand Down Expand Up @@ -548,6 +550,13 @@ private static DOMParser createDOMParser() throws SamlException {
"Cannot disable comments parsing to mitigate https://www.kb.cert.org/vuls/id/475445",
ex);
}

try {
setFeature(XERCES_FEATURE_PREFIX + DISALLOW_DOCTYPE_DECL_FEATURE, true);
} catch (Throwable ex) {
throw new SamlException(
"Cannot disable external entities to prevent XXE injection", ex);
}
}
};

Expand Down
18 changes: 18 additions & 0 deletions src/test/java/com/coveo/saml/SamlClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,22 @@ public void decodeAndValidateSamlResponseRejectsNowBeforeNotBefore() throws Thro
SamlResponse response = client.decodeAndValidateSamlResponse(AN_ENCODED_RESPONSE);
assertEquals("[email protected]", response.getNameID());
}

// Test for https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
@Test
public void itIsNotVulnerableToXXEAttacks() throws Throwable {
String decoded = new String(Base64.decodeBase64(AN_ENCODED_RESPONSE), StandardCharsets.UTF_8);
String withDtd = "<!DOCTYPE note SYSTEM \"Note.dtd\">" + decoded;
SamlClient client =
SamlClient.fromMetadata(
"myidentifier", "http://some/url", getXml("adfs.xml"), SamlClient.SamlIdpBinding.POST);
client.setDateTimeNow(ASSERTION_DATE);
try {
client.decodeAndValidateSamlResponse(
Base64.encodeBase64String(withDtd.getBytes(StandardCharsets.UTF_8)));
assertTrue(false);
} catch (SamlException ex) {
assertTrue(ex.getCause().toString().contains("DOCTYPE is disallowed"));
}
}
}

0 comments on commit 25b0320

Please sign in to comment.