From 4f4809d145eff99445bb1f86f6e9447507bb7a97 Mon Sep 17 00:00:00 2001 From: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:35:51 -0600 Subject: [PATCH 1/2] Prevent XXE in the HL7 v2.x strict parser The strict parser hands XML-encoded inbound messages to HAPI 2.3, whose XMLUtils.parse resolves external XML entities. On a channel with the strict parser and strict validation enabled, an unauthenticated message to the MLLP/TCP listener could trigger SSRF and local file disclosure. Override CustomDefaultXMLParser.parseStringIntoDocument -- the sole path to the vulnerable parse -- to reject DOCTYPE declarations, matching the disallow-doctype-decl hardening already used on the fromXML path. Legitimate HL7 v2.xml is schema-based and never carries a DOCTYPE, so no valid message is affected and the strict parser keeps accepting XML as before. Verified with a live MLLP reproduction (xxe-poc): the unpatched build fetched the attacker URL; the patched build rejects the message and never calls out. Signed-off-by: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> --- .../datatypes/hl7v2/ER7Serializer.java | 27 ++++++++ .../datatypes/hl7v2/ER7SerializerTest.java | 64 ++++++++++++++++--- .../tests/test-xxe-hl7-strict-mllp-valid.xml | 2 + server/tests/test-xxe-hl7-strict-mllp.xml | 3 + 4 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 server/tests/test-xxe-hl7-strict-mllp-valid.xml create mode 100644 server/tests/test-xxe-hl7-strict-mllp.xml diff --git a/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java b/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java index 71e9a49b54..05838b5df0 100644 --- a/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java +++ b/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java @@ -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; @@ -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); + } + } } } diff --git a/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java b/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java index a7319dca5a..67e77787f8 100644 --- a/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java +++ b/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java @@ -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; @@ -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); } } diff --git a/server/tests/test-xxe-hl7-strict-mllp-valid.xml b/server/tests/test-xxe-hl7-strict-mllp-valid.xml new file mode 100644 index 0000000000..0622b17fb9 --- /dev/null +++ b/server/tests/test-xxe-hl7-strict-mllp-valid.xml @@ -0,0 +1,2 @@ + +|^~\&APPACK12.4AA1 diff --git a/server/tests/test-xxe-hl7-strict-mllp.xml b/server/tests/test-xxe-hl7-strict-mllp.xml new file mode 100644 index 0000000000..3433b023de --- /dev/null +++ b/server/tests/test-xxe-hl7-strict-mllp.xml @@ -0,0 +1,3 @@ + + ]> +|^~\&&xxe;ACK12.4AA1 From 03eefcfe06c10be530cce03f60a2505bcc0aa925 Mon Sep 17 00:00:00 2001 From: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> Date: Thu, 24 Sep 2026 12:42:30 -0600 Subject: [PATCH 2/2] Keep CDATA content in the HL7 v2.x strict XML parser The DOCTYPE-rejecting parser override did not coalesce CDATA into text nodes. HAPI reads only text nodes into a field, so a field sent as CDATA was silently emptied, where HAPI's own parser kept it. Enable coalescing. Adds a unit test and a smoke case (111-hl7-strict-xml-cdata) that sends a v2.xml message with a CDATA field through a strict-parser channel. Signed-off-by: Finnegan's Owner <44065187+pacmano1@users.noreply.github.com> --- .../01-hl7-strict-xml-cdata/channel.xml | 382 ++++++++++++++++++ .../messages/01-cdata-field/source | 2 + .../01-cdata-field/source_transformed | 1 + .../datatypes/hl7v2/ER7Serializer.java | 4 + .../datatypes/hl7v2/ER7SerializerTest.java | 9 + 5 files changed, 398 insertions(+) create mode 100644 ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/channel.xml create mode 100644 ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source create mode 100644 ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source_transformed diff --git a/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/channel.xml b/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/channel.xml new file mode 100644 index 0000000000..11e8498c6a --- /dev/null +++ b/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/channel.xml @@ -0,0 +1,382 @@ + + 5aa08bd1-92f8-405d-9105-d85a79bfa3a0 + 2 + hl7-strict-xml-cdata + + 1 + + 0 + sourceConnector + + + + Auto-generate (Destinations completed) + true + false + false + 1 + + + Default Resource + [Default Resource] + + + 1000 + + + + + HL7V2 + HL7V2 + + + true + true + true + true + false + \r + true + + + false + false + \r + + + MSH_Segment + + + + \r + AA + + AE + An Error Occurred Processing Message. + AR + Message Rejected. + false + yyyyMMddHHmmss.SSS + + + AA,CA + AE,CE + AR,CR + true + Destination_Encoded + + + + + + true + true + false + false + false + \r + true + + + true + true + \r + + + MSH_Segment + + + + \r + AA + + AE + An Error Occurred Processing Message. + AR + Message Rejected. + false + yyyyMMddHHmmss.SSS + + + AA,CA + AE,CE + AR,CR + true + Destination_Encoded + + + + + + + + Channel Reader + SOURCE + true + true + + + + 1 + Destination 1 + + + + false + false + 10000 + false + 0 + false + false + 1 + + false + + + Default Resource + [Default Resource] + + + 1000 + true + + none + ${message.encodedData} + + + + + HL7V2 + HL7V2 + + + true + true + false + false + false + \r + true + + + false + false + \r + + + MSH_Segment + + + + \r + AA + + AE + An Error Occurred Processing Message. + AR + Message Rejected. + false + yyyyMMddHHmmss.SSS + + + AA,CA + AE,CE + AR,CR + true + Destination_Encoded + + + + + + true + true + false + false + false + \r + true + + + false + false + \r + + + MSH_Segment + + + + \r + AA + + AE + An Error Occurred Processing Message. + AR + Message Rejected. + false + yyyyMMddHHmmss.SSS + + + AA,CA + AE,CE + AR,CR + true + Destination_Encoded + + + + + + + HL7V2 + HL7V2 + + + true + true + false + false + false + \r + true + + + false + false + \r + + + MSH_Segment + + + + \r + AA + + AE + An Error Occurred Processing Message. + AR + Message Rejected. + false + yyyyMMddHHmmss.SSS + + + AA,CA + AE,CE + AR,CR + true + Destination_Encoded + + + + + + true + true + false + false + false + \r + true + + + false + false + \r + + + MSH_Segment + + + + \r + AA + + AE + An Error Occurred Processing Message. + AR + Message Rejected. + false + yyyyMMddHHmmss.SSS + + + AA,CA + AE,CE + AR,CR + true + Destination_Encoded + + + + + + + + Channel Writer + DESTINATION + true + true + + + // Modify the message variable below to pre process data +return message; + // This script executes once after a message has been processed +// Responses returned from here will be stored as "Postprocessor" in the response map +return; + // This script executes once when the channel is deployed +// You only have access to the globalMap and globalChannelMap here to persist data +return; + // This script executes once when the channel is undeployed +// You only have access to the globalMap and globalChannelMap here to persist data +return; + + true + DEVELOPMENT + false + false + false + false + false + false + STARTED + true + + + SOURCE + STRING + mirth_source + + + TYPE + STRING + mirth_type + + + + None + + + + + Default Resource + [Default Resource] + + + + + + true + + + America/Chicago + + + true + false + + 1 + + + \ No newline at end of file diff --git a/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source b/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source new file mode 100644 index 0000000000..93efbb50de --- /dev/null +++ b/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source @@ -0,0 +1,2 @@ + +|^~\&ACKPLAIN-CONTROL2.4AA1 \ No newline at end of file diff --git a/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source_transformed b/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source_transformed new file mode 100644 index 0000000000..19c94a9d40 --- /dev/null +++ b/ci/tests/111-hl7-strict-xml-cdata/channels/01-hl7-strict-xml-cdata/messages/01-cdata-field/source_transformed @@ -0,0 +1 @@ +|^~\&CDATA-APPACKPLAIN-CONTROL2.4AA1 \ No newline at end of file diff --git a/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java b/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java index 05838b5df0..d491ad7703 100644 --- a/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java +++ b/server/src/main/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7Serializer.java @@ -537,6 +537,9 @@ protected Message instantiateMessage(String theName, String theVersion, boolean * 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. + * + * Coalescing is required: HAPI reads only text nodes into a field, so without it CDATA + * content is silently dropped, which HAPI's own parser does not do. */ @Override protected synchronized Document parseStringIntoDocument(String xml) throws HL7Exception { @@ -544,6 +547,7 @@ protected synchronized Document parseStringIntoDocument(String xml) throws HL7Ex DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); factory.setNamespaceAware(true); + factory.setCoalescing(true); return factory.newDocumentBuilder().parse(new InputSource(new StringReader(xml))); } catch (Exception e) { throw new HL7Exception("Exception parsing XML", e); diff --git a/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java b/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java index 67e77787f8..1119c60d7f 100644 --- a/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java +++ b/server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java @@ -101,4 +101,13 @@ public void testToXmlStrictValidatingAllowsValidXml() throws Exception { assertFalse(exceptionThrown); } + + @Test + public void testToXmlStrictValidatingKeepsCdataContent() throws Exception { + // HAPI reads only text nodes into a field, so CDATA content is silently dropped unless the + // parser coalesces it into the surrounding text. + String xml = "|^~\\&ACK12.4AA1"; + + assertTrue(strictValidatingSerializer.toXML(xml).contains("CDATA-APP")); + } }