Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
import java.util.Map;
import java.util.regex.Pattern;

import javax.xml.parsers.DocumentBuilderFactory;

import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.w3c.dom.Document;
import org.xml.sax.InputSource;
import org.xml.sax.XMLReader;
import org.xml.sax.helpers.XMLReaderFactory;
Expand Down Expand Up @@ -522,6 +525,30 @@ protected Message instantiateMessage(String theName, String theVersion, boolean

return message;
}

/*
* HAPI 2.3's XMLUtils.parse builds its DOM parser with no protection against external XML
* entities, so a strict-parsed inbound message carrying a DOCTYPE can trigger XXE (SSRF and
* local file disclosure), reachable unauthenticated over an MLLP/TCP listener. This is the
* only method that reaches that parser, so override it to reject any DOCTYPE up front, using
* the same disallow-doctype-decl hardening already applied to the fromXML path above.
* Legitimate HL7 v2.x XML never contains a DOCTYPE.
*
* This stays stronger than HAPI's own >= 2.4 fix, which permits a DOCTYPE and only disables
* entity resolution. Remove only once HAPI is upgraded to >= 2.4 AND that weaker posture is
* deliberately accepted.
*/
@Override
protected synchronized Document parseStringIntoDocument(String xml) throws HL7Exception {
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setNamespaceAware(true);
return factory.newDocumentBuilder().parse(new InputSource(new StringReader(xml)));
} catch (Exception e) {
throw new HL7Exception("Exception parsing XML", e);
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.io.File;

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.junit.BeforeClass;
import org.junit.Test;
import org.xml.sax.SAXParseException;
Expand All @@ -15,42 +16,89 @@

public class ER7SerializerTest {
private static ER7Serializer serializer;

// Strict parser with strict validation: XML input is parsed by HAPI (the XXE sink).
private static ER7Serializer strictValidatingSerializer;

@BeforeClass
public static void setupClass() throws Exception {
SerializerProperties serializerProperties = new SerializerProperties(new HL7v2SerializationProperties(), new HL7v2DeserializationProperties(), null);
serializer = new ER7Serializer(serializerProperties);

HL7v2SerializationProperties strictValidatingProperties = new HL7v2SerializationProperties();
strictValidatingProperties.setUseStrictParser(true);
strictValidatingProperties.setUseStrictValidation(true);
strictValidatingSerializer = new ER7Serializer(new SerializerProperties(strictValidatingProperties, new HL7v2DeserializationProperties(), null));
}

@Test
public void testFromXMLWithExternalDTD() throws Exception {
String xml = FileUtils.readFileToString(new File("tests/test-xxe-hl7-example.xml"), "UTF-8");

boolean exceptionThrown = false;
try {
serializer.fromXML(xml);
} catch (MessageSerializerException e) {
exceptionThrown = true;

// See https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#jaxp-documentbuilderfactory-saxparserfactory-and-dom4j
assertTrue(e.getCause() instanceof SAXParseException);
}

assertTrue(exceptionThrown);
}

@Test
public void testValidFromXMLWithExternalDTD() throws Exception {
String xml = FileUtils.readFileToString(new File("tests/test-xxe-hl7-example-valid.xml"), "UTF-8");

boolean exceptionThrown = false;
try {
serializer.fromXML(xml);
} catch (MessageSerializerException e) {
exceptionThrown = true;


}

assertFalse(exceptionThrown);
}

@Test
public void testToXmlStrictValidatingRejectsExternalDTD() throws Exception {
// A DOCTYPE-bearing message on the strict-parser toXML path is the unauthenticated MLLP XXE
// vector (HAPI 2.3 resolved external entities). It must be rejected rather than have its
// external entity resolved. Note: this asserts the intended behavior; the discriminating
// before/after proof that the override (not incidental parser behavior) closes the XXE is the
// live MLLP reproduction documented in the PR.
String xml = FileUtils.readFileToString(new File("tests/test-xxe-hl7-strict-mllp.xml"), "UTF-8");

boolean exceptionThrown = false;
try {
strictValidatingSerializer.toXML(xml);
} catch (MessageSerializerException e) {
exceptionThrown = true;

// The rejection must be the DOCTYPE being disallowed, not some incidental parse failure.
Throwable rootCause = ExceptionUtils.getRootCause(e);
assertTrue(rootCause instanceof SAXParseException);
assertTrue(rootCause.getMessage().contains("DOCTYPE"));
}

assertTrue(exceptionThrown);
}

@Test
public void testToXmlStrictValidatingAllowsValidXml() throws Exception {
// The same message without a DOCTYPE is legitimate HL7 v2.x XML and must still round-trip, so
// the hardening does not break the strict parser's XML support.
String xml = FileUtils.readFileToString(new File("tests/test-xxe-hl7-strict-mllp-valid.xml"), "UTF-8");

boolean exceptionThrown = false;
try {
strictValidatingSerializer.toXML(xml);
} catch (MessageSerializerException e) {
exceptionThrown = true;
}

assertFalse(exceptionThrown);
}
}
2 changes: 2 additions & 0 deletions server/tests/test-xxe-hl7-strict-mllp-valid.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?xml version="1.0"?>
<ACK xmlns="urn:hl7-org:v2xml"><MSH><MSH.1>|</MSH.1><MSH.2>^~\&amp;</MSH.2><MSH.3><HD.1>APP</HD.1></MSH.3><MSH.9><MSG.1>ACK</MSG.1></MSH.9><MSH.10>1</MSH.10><MSH.12><VID.1>2.4</VID.1></MSH.12></MSH><MSA><MSA.1>AA</MSA.1><MSA.2>1</MSA.2></MSA></ACK>
3 changes: 3 additions & 0 deletions server/tests/test-xxe-hl7-strict-mllp.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?xml version="1.0"?>
<!DOCTYPE ACK [ <!ENTITY xxe SYSTEM "tests/test-dtd"> ]>
<ACK xmlns="urn:hl7-org:v2xml"><MSH><MSH.1>|</MSH.1><MSH.2>^~\&amp;</MSH.2><MSH.3><HD.1>&xxe;</HD.1></MSH.3><MSH.9><MSG.1>ACK</MSG.1></MSH.9><MSH.10>1</MSH.10><MSH.12><VID.1>2.4</VID.1></MSH.12></MSH><MSA><MSA.1>AA</MSA.1><MSA.2>1</MSA.2></MSA></ACK>
Loading