diff --git a/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/channel.xml b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/channel.xml new file mode 100644 index 0000000000..5175d66711 --- /dev/null +++ b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/channel.xml @@ -0,0 +1,195 @@ + + 5ec00001-0000-4000-8000-00000000c1a5 + 2 + XSLT Step XXE + + 1 + + 0 + sourceConnector + + + + None + true + false + false + 1 + + + Default Resource + [Default Resource] + + + 1000 + + + + + + XSLT XXE + 0 + true + connectorMessage.getRawData() + xsltResult + + false + + + + RAW + RAW + + + JavaScript + + + + + + JavaScript + + + + + + + + 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} + + + + + RAW + RAW + + + JavaScript + + + + + + JavaScript + + + + + + + RAW + RAW + + + JavaScript + + + + + + JavaScript + + + + + + + + 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/200-xslt-step-xxe/channels/01-xslt-xxe/messages/01-external-entity/source b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/01-external-entity/source new file mode 100644 index 0000000000..f804122910 --- /dev/null +++ b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/01-external-entity/source @@ -0,0 +1 @@ +]>&x; diff --git a/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/01-external-entity/source_status b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/01-external-entity/source_status new file mode 100644 index 0000000000..5df7507e2d --- /dev/null +++ b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/01-external-entity/source_status @@ -0,0 +1 @@ +ERROR diff --git a/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/02-benign-control/source b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/02-benign-control/source new file mode 100644 index 0000000000..17c9334477 --- /dev/null +++ b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/02-benign-control/source @@ -0,0 +1 @@ +hello diff --git a/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/02-benign-control/source_status b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/02-benign-control/source_status new file mode 100644 index 0000000000..ddc7ed8516 --- /dev/null +++ b/ci/tests/200-xslt-step-xxe/channels/01-xslt-xxe/messages/02-benign-control/source_status @@ -0,0 +1 @@ +TRANSFORMED diff --git a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java index 2493be8d7b..b020e93dd6 100644 --- a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java +++ b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java @@ -33,6 +33,7 @@ import org.apache.logging.log4j.Logger; import com.mirth.connect.client.core.api.MirthApiException; +import com.mirth.connect.model.DriverInfo; import com.mirth.connect.server.api.MirthServlet; import com.mirth.connect.server.controllers.ContextFactoryController; import com.mirth.connect.server.controllers.ControllerFactory; @@ -52,6 +53,13 @@ public DatabaseConnectorServlet(@Context HttpServletRequest request, @Context Se @Override public SortedSet getTables(String channelId, String channelName, String driver, String url, String username, String password, Set tableNamePatterns, String selectLimit, Set resourceIds) { + // selectLimit is a driver-specific, server-owned metadata-probe template, not a caller input. + // The caller-supplied value is ignored and the template is resolved from the built-in driver + // list keyed by the JDBC driver class; anything unrecognised uses the safe + // DatabaseMetaData.getColumns() path. This removes the SQL-injection surface (CVE-2026-82583) + // without depending on any API-writable configuration. + String resolvedSelectLimit = resolveSelectLimit(driver); + CustomDriver customDriver = null; Connection connection = null; try { @@ -150,9 +158,11 @@ public SortedSet
getTables(String channelId, String channelName, String d // then we'll define to the generic method of getting column information, but // this could be extremely slow List columnList = new ArrayList(); - if (StringUtils.isEmpty(selectLimit)) { + if (StringUtils.isEmpty(resolvedSelectLimit)) { logger.debug("No select limit is defined, using generic method"); - rs = dbMetaData.getColumns(null, null, tableName, null); + // Scope to the discovered schema (may be null) so a same-named table in + // another schema does not leak its columns into the result. + rs = dbMetaData.getColumns(null, schema, tableName, null); // retrieve all relevant column information for (int i = 0; rs.next(); i++) { @@ -160,12 +170,12 @@ public SortedSet
getTables(String channelId, String channelName, String d columnList.add(column); } } else { - logger.debug("Select limit is defined, using specific select query : '" + selectLimit + "'"); + logger.debug("Select limit is defined, using specific select query : '" + resolvedSelectLimit + "'"); - // replace the '?' with the appropriate schema.table name, and use ResultSetMetaData to - // retrieve column information - final String schemaTableName = StringUtils.isNotEmpty(schema) ? "\"" + schema + "\".\"" + tableName + "\"" : "\"" + tableName + "\""; - final String queryString = selectLimit.trim().replaceAll("\\?", Matcher.quoteReplacement(schemaTableName)); + // replace the '?' with the appropriate schema.table name, and use ResultSetMetaData to + // retrieve column information + final String schemaTableName = quoteSchemaTable(schema, tableName); + final String queryString = resolvedSelectLimit.trim().replaceAll("\\?", Matcher.quoteReplacement(schemaTableName)); Statement statement = connection.createStatement(); try { rs = statement.executeQuery(queryString); @@ -192,7 +202,7 @@ public SortedSet
getTables(String channelId, String channelName, String d columnList = new ArrayList(); logger.debug("Using fallback method for retrieving columns"); - backupRs = dbMetaData.getColumns(null, null, tableName.replace("/", "//"), null); + backupRs = dbMetaData.getColumns(null, schema, tableName.replace("/", "//"), null); // retrieve all relevant column information while (backupRs.next()) { @@ -229,6 +239,50 @@ public SortedSet
getTables(String channelId, String channelName, String d } } + /** + * Resolves the driver-specific metadata-probe query from the built-in driver definitions, keyed + * by the JDBC driver class (including known alternative class names). Returns {@code ""} - the + * safe {@link DatabaseMetaData#getColumns} path - for anything not built in. + *

+ * The caller-supplied {@code selectLimit} query parameter is deliberately ignored: it is executed + * as SQL (CVE-2026-82583), and the metadata dialog only ever sends the driver's own template + * anyway. This method consults only {@link DriverInfo#getDefaultDrivers()}, never the + * API-writable configured driver list, so a caller cannot introduce an arbitrary query even by + * first writing it to the driver configuration. Package-private and static so it is unit-testable + * without a servlet instance or a live server. + */ + static String resolveSelectLimit(String driver) { + if (StringUtils.isBlank(driver)) { + return ""; + } + + for (DriverInfo driverInfo : DriverInfo.getDefaultDrivers()) { + if (driver.equals(driverInfo.getClassName()) + || (driverInfo.getAlternativeClassNames() != null && driverInfo.getAlternativeClassNames().contains(driver))) { + return StringUtils.defaultString(driverInfo.getSelectLimit()); + } + } + + return ""; + } + + /** + * Builds the {@code "schema"."table"} (or {@code "table"}) identifier that replaces the {@code ?} + * placeholder in the metadata-probe query. The schema and table names come from the database's own + * metadata ({@link DatabaseMetaData#getSchemas}/{@link DatabaseMetaData#getTables}), but a name may + * still contain a double quote; embedding it verbatim would break out of the quoting and alter the + * query (a second-order SQL injection). Following the SQL standard, any embedded {@code "} is + * doubled so the value is always a single quoted identifier. Package-private and static so it is + * unit-testable without a live database. + */ + static String quoteSchemaTable(String schema, String tableName) { + String quotedTable = "\"" + StringUtils.defaultString(tableName).replace("\"", "\"\"") + "\""; + if (StringUtils.isNotEmpty(schema)) { + return "\"" + schema.replace("\"", "\"\"") + "\"." + quotedTable; + } + return quotedTable; + } + /** * Translate the given pattern expression so that it can be used properly for searching tables * in the database. Multiple table name patterns are delimited by comma (,) diff --git a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServletInterface.java b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServletInterface.java index 40ae6174fb..13ad53652a 100644 --- a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServletInterface.java +++ b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServletInterface.java @@ -59,7 +59,7 @@ public SortedSet

getTables(// @formatter:off @Param("username") @Parameter(description = "The username to authenticate with.") @DefaultValue("") @QueryParam("username") String username, @Param(value = "password", excludeFromAudit = true) @Parameter(description = "The password to authenticate with.", schema = @Schema(format = "password")) @DefaultValue("") @QueryParam("password") String password, @Param("tableNamePatterns") @Parameter(description = "If specified, filters by table name. Wildcards (* or %) are allowed.") @QueryParam("tableNamePattern") Set tableNamePatterns, - @Param("selectLimit") @Parameter(description = "A simple query to use to retrieve database metadata information.", schema = @Schema(defaultValue = "SELECT * FROM ? LIMIT 1")) @DefaultValue("SELECT * FROM ? LIMIT 1") @QueryParam("selectLimit") String selectLimit, + @Param("selectLimit") @Parameter(description = "Deprecated and ignored: the metadata-probe query is resolved server-side from the driver class, never from this value, to prevent SQL injection (CVE-2026-82583). Retained only for wire/source compatibility.", schema = @Schema(defaultValue = "SELECT * FROM ? LIMIT 1")) @DefaultValue("SELECT * FROM ? LIMIT 1") @QueryParam("selectLimit") String selectLimit, @Param("resourceIds") @Parameter(description = "Library resource IDs to use, if a custom driver is necessary.") @QueryParam("resourceId") Set resourceIds) throws ClientException; // @formatter:on) } \ No newline at end of file diff --git a/server/src/main/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptor.java b/server/src/main/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptor.java index bb6994e339..867fa25087 100644 --- a/server/src/main/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptor.java +++ b/server/src/main/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptor.java @@ -18,6 +18,7 @@ import java.util.Map; import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerFactory; @@ -33,6 +34,7 @@ import org.mozilla.javascript.Context; import org.mozilla.javascript.Script; import org.mozilla.javascript.Scriptable; +import org.w3c.dom.Document; import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.InputSource; @@ -42,6 +44,7 @@ import com.mirth.connect.donkey.server.message.batch.BatchMessageException; import com.mirth.connect.donkey.server.message.batch.BatchMessageReader; import com.mirth.connect.donkey.server.message.batch.BatchMessageReceiver; +import com.mirth.connect.model.converters.DocumentSerializer; import com.mirth.connect.plugins.datatypes.xml.XMLBatchProperties.SplitType; import com.mirth.connect.server.controllers.ContextFactoryController; import com.mirth.connect.server.controllers.ControllerFactory; @@ -127,7 +130,9 @@ private String getMessageFromReader() throws Exception { XPath xpath = xPathFactory.newXPath(); - nodeList = (NodeList) xpath.evaluate(query.toString(), new InputSource(bufferedReader), XPathConstants.NODESET); + Document document = parseBatchSecurely(new InputSource(bufferedReader)); + + nodeList = (NodeList) xpath.evaluate(query.toString(), document, XPathConstants.NODESET); } if (currentNode < nodeList.getLength()) { @@ -187,6 +192,27 @@ public String doCall() throws Exception { return null; } + /** + * Parses an untrusted batch document with a hardened parser before any XPath evaluation, rather + * than letting {@code XPath.evaluate(InputSource)} build its own DOCTYPE-resolving parser (XXE, + * CVE-2026-82578). {@link DocumentSerializer#getSecureDocumentBuilderFactory()} already sets + * {@code disallow-doctype-decl} (so any DOCTYPE is rejected outright); the extra features below + * block external entities/DTDs and entity expansion as defense in depth. Namespace-awareness is + * enabled to match the previous {@code XPath.evaluate(InputSource)} path, so namespace-sensitive + * split queries ({@code namespace-uri()}/prefixes) behave as before. Package-private and static so + * the parser hardening is unit-testable without constructing a full adaptor. + */ + static Document parseBatchSecurely(InputSource source) throws Exception { + DocumentBuilderFactory dbf = DocumentSerializer.getSecureDocumentBuilderFactory(); + dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + dbf.setXIncludeAware(false); + dbf.setExpandEntityReferences(false); + dbf.setNamespaceAware(true); + return dbf.newDocumentBuilder().parse(source); + } + private String toXML(Node node) throws Exception { Writer writer = new StringWriter(); diff --git a/server/src/main/java/com/mirth/connect/plugins/xsltstep/XsltStep.java b/server/src/main/java/com/mirth/connect/plugins/xsltstep/XsltStep.java index 1facca8b46..6261465970 100644 --- a/server/src/main/java/com/mirth/connect/plugins/xsltstep/XsltStep.java +++ b/server/src/main/java/com/mirth/connect/plugins/xsltstep/XsltStep.java @@ -66,6 +66,19 @@ private String getTransformationScript() { script.append("tFactory = Packages.javax.xml.transform.TransformerFactory.newInstance();\n"); } + // Harden the factory against XXE (CVE-2026-78224). The source XML is attacker-controlled, so + // ACCESS_EXTERNAL_DTD is set to '' to deny external DTDs/entities in it outright -- this is + // what closes the CVE. The stylesheet is channel-author content; per the OWASP XXE cheat + // sheet ("restrict rather than close" external references in your own stylesheets), + // ACCESS_EXTERNAL_STYLESHEET is restricted to the 'file' protocol rather than blocked, so + // local xsl:import/xsl:include/document() keep working while http(s) SSRF is denied. Neither + // is swallowed: if a configured factory rejects an attribute the transform fails rather than + // running with external access silently left open. FEATURE_SECURE_PROCESSING is deliberately + // NOT enabled: on the JDK's built-in Xalan it also disables Java extension functions, which + // would break existing stylesheets that call Java. + script.append("tFactory.setAttribute(Packages.javax.xml.XMLConstants.ACCESS_EXTERNAL_DTD, '');\n"); + script.append("tFactory.setAttribute(Packages.javax.xml.XMLConstants.ACCESS_EXTERNAL_STYLESHEET, 'file');\n"); + script.append("xsltTemplate = new Packages.java.io.StringReader(" + template + ");\n"); script.append("transformer = tFactory.newTransformer(new Packages.javax.xml.transform.stream.StreamSource(xsltTemplate));\n"); script.append("sourceVar = new Packages.java.io.StringReader(" + sourceXml + ");\n"); diff --git a/server/src/test/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServletTest.java b/server/src/test/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServletTest.java new file mode 100644 index 0000000000..ffe38f3a11 --- /dev/null +++ b/server/src/test/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServletTest.java @@ -0,0 +1,64 @@ +/* + * Copyright (c) Open Integration Engine. All rights reserved. + * + * The software in this package is published under the terms of the MPL license a copy of which has + * been included with this distribution in the LICENSE.txt file. + */ + +package com.mirth.connect.connectors.jdbc; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +/** + * Unit coverage for the SQL-injection fix in the JDBC connector metadata endpoint (CVE-2026-82583). + * + *

+ * {@code getTables} used to execute the caller-supplied {@code selectLimit} as SQL. It now ignores + * that parameter and resolves the metadata-probe template server-side from the built-in driver list, + * keyed by the JDBC driver class. These tests pin that behavior with no database or live server: the + * unknown-driver case is the proof that a caller-chosen value can never reach {@code executeQuery}. + */ +public class DatabaseConnectorServletTest { + + @Test + public void resolvesBuiltInDriverTemplates() { + assertEquals("SELECT * FROM ? LIMIT 1", DatabaseConnectorServlet.resolveSelectLimit("org.postgresql.Driver")); + assertEquals("SELECT * FROM ? LIMIT 1", DatabaseConnectorServlet.resolveSelectLimit("com.mysql.cj.jdbc.Driver")); + assertEquals("SELECT * FROM ? WHERE ROWNUM < 2", DatabaseConnectorServlet.resolveSelectLimit("oracle.jdbc.driver.OracleDriver")); + assertEquals("SELECT TOP 1 * FROM ?", DatabaseConnectorServlet.resolveSelectLimit("com.microsoft.sqlserver.jdbc.SQLServerDriver")); + } + + @Test + public void resolvesAlternativeDriverClassName() { + // The legacy MySQL driver class is registered as an alternative class name. + assertEquals("SELECT * FROM ? LIMIT 1", DatabaseConnectorServlet.resolveSelectLimit("com.mysql.jdbc.Driver")); + } + + @Test + public void unknownOrInjectedDriverUsesGenericMetadataPath() { + // Anything not built in resolves to "" -> DatabaseMetaData.getColumns(), never executeQuery. + assertEquals("", DatabaseConnectorServlet.resolveSelectLimit("com.attacker.EvilDriver")); + assertEquals("", DatabaseConnectorServlet.resolveSelectLimit("")); + assertEquals("", DatabaseConnectorServlet.resolveSelectLimit(" ")); + assertEquals("", DatabaseConnectorServlet.resolveSelectLimit(null)); + } + + @Test + public void quotesPlainSchemaAndTableIdentifiers() { + assertEquals("\"myschema\".\"mytable\"", DatabaseConnectorServlet.quoteSchemaTable("myschema", "mytable")); + assertEquals("\"mytable\"", DatabaseConnectorServlet.quoteSchemaTable(null, "mytable")); + assertEquals("\"mytable\"", DatabaseConnectorServlet.quoteSchemaTable("", "mytable")); + } + + @Test + public void escapesEmbeddedQuotesToBlockSecondOrderInjection() { + // A database identifier containing a double quote must not break out of the quoting: the + // embedded quote is doubled so the value stays a single quoted identifier. + assertEquals("\"normal\"\" AS n JOIN secret ON 1=1 --\"", + DatabaseConnectorServlet.quoteSchemaTable(null, "normal\" AS n JOIN secret ON 1=1 --")); + assertEquals("\"ev\"\"il\".\"ta\"\"ble\"", + DatabaseConnectorServlet.quoteSchemaTable("ev\"il", "ta\"ble")); + } +} diff --git a/server/src/test/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptorSecurityTest.java b/server/src/test/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptorSecurityTest.java new file mode 100644 index 0000000000..131a384c4c --- /dev/null +++ b/server/src/test/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptorSecurityTest.java @@ -0,0 +1,104 @@ +/* + * Copyright (c) Open Integration Engine. All rights reserved. + * + * The software in this package is published under the terms of the MPL license a copy of which has + * been included with this distribution in the LICENSE.txt file. + */ + +package com.mirth.connect.plugins.datatypes.xml; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.StringReader; + +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathFactory; + +import org.junit.Test; +import org.w3c.dom.Document; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; + +/** + * Guards the XXE hardening of the XML batch adaptor (CVE-2026-82578). The adaptor used to let + * {@code XPath.evaluate(InputSource)} build its own DOCTYPE-resolving parser; it now parses untrusted + * batches through {@link XMLBatchAdaptor#parseBatchSecurely} with {@code disallow-doctype-decl} and + * external access disabled. + * + *

+ * These are pure unit tests against the exact hardened parse path -- no live server. The benign + * control proves the parser still works (and stays namespace-aware), so the rejection tests are not + * false-passing on an unrelated failure. + */ +public class XMLBatchAdaptorSecurityTest { + + private static InputSource source(String xml) { + return new InputSource(new StringReader(xml)); + } + + @Test + public void rejectsExternalEntityDoctype() { + // An external SYSTEM entity is the file-exfiltration vector. It must be rejected at parse time. + String xml = "" + + "]>" + + "&x;"; + try { + XMLBatchAdaptor.parseBatchSecurely(source(xml)); + fail("expected the hardened parser to reject a DOCTYPE (external entity)"); + } catch (Exception e) { + // disallow-doctype-decl surfaces as a SAXParseException mentioning DOCTYPE. Assert a + // specific rejection rather than accepting any exception, so transport/other failures + // could not masquerade as success. + assertTrue("expected a DOCTYPE rejection but was: " + e, + String.valueOf(e.getMessage()).toUpperCase().contains("DOCTYPE")); + } + } + + @Test + public void rejectsInternalEntityDoctype() { + String xml = "" + + "]>" + + "&x;"; + try { + XMLBatchAdaptor.parseBatchSecurely(source(xml)); + fail("expected the hardened parser to reject a DOCTYPE (internal entity)"); + } catch (Exception e) { + assertTrue("expected a DOCTYPE rejection but was: " + e, + String.valueOf(e.getMessage()).toUpperCase().contains("DOCTYPE")); + } + } + + @Test + public void parsesBenignBatchAndSplitsByElementName() throws Exception { + String xml = "helloworld"; + + Document document = XMLBatchAdaptor.parseBatchSecurely(source(xml)); + assertNotNull(document); + + XPath xpath = XPathFactory.newInstance().newXPath(); + NodeList nodes = (NodeList) xpath.evaluate("//*[local-name()='message']", document, XPathConstants.NODESET); + assertEquals(2, nodes.getLength()); + assertEquals("hello", nodes.item(0).getTextContent()); + } + + @Test + public void parsesNamespacedBatchNamespaceAware() throws Exception { + // Locks in setNamespaceAware(true): a namespace-uri() predicate must still match, matching the + // behavior of the previous XPath.evaluate(InputSource) path. + String xml = "" + + "ns-hello" + + ""; + + Document document = XMLBatchAdaptor.parseBatchSecurely(source(xml)); + + XPath xpath = XPathFactory.newInstance().newXPath(); + NodeList nodes = (NodeList) xpath.evaluate( + "//*[local-name()='message' and namespace-uri()='urn:oie:batch']", document, XPathConstants.NODESET); + assertEquals("namespace-aware parsing should let a namespace-uri() predicate match", 1, nodes.getLength()); + assertEquals("ns-hello", nodes.item(0).getTextContent()); + } +} diff --git a/server/src/test/java/com/mirth/connect/plugins/xsltstep/XsltStepSecurityTest.java b/server/src/test/java/com/mirth/connect/plugins/xsltstep/XsltStepSecurityTest.java new file mode 100644 index 0000000000..ca30bc5c10 --- /dev/null +++ b/server/src/test/java/com/mirth/connect/plugins/xsltstep/XsltStepSecurityTest.java @@ -0,0 +1,65 @@ +/* + * Copyright (c) Open Integration Engine. All rights reserved. + * + * The software in this package is published under the terms of the MPL license a copy of which has + * been included with this distribution in the LICENSE.txt file. + */ + +package com.mirth.connect.plugins.xsltstep; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +/** + * Guards the XXE hardening of the XSLT transformer step (CVE-2026-78224). The step emits JavaScript + * that builds a TransformerFactory at runtime, so the fix lives in the generated script text. + */ +public class XsltStepSecurityTest { + + @Test + public void generatedScriptHardensTransformerFactoryAgainstXxe() { + XsltStep step = new XsltStep(); + step.setSourceXml("connectorMessage.getRawData()"); + step.setResultVariable("xsltResult"); + step.setTemplate("''"); + + String script = step.getScript(false); + + // The attacker-controlled source XML: external DTDs/entities denied outright (closes the CVE). + assertTrue("external DTD access should be denied ('')", + script.contains("ACCESS_EXTERNAL_DTD, ''")); + // The author-controlled stylesheet: restricted to the 'file' protocol (OWASP "restrict rather + // than close"), so local xsl:import/include/document() work but http(s) SSRF is denied. It + // must NOT be blocked outright ('') -- that would break legitimate includes. + assertTrue("external stylesheet access should be restricted to the file protocol", + script.contains("ACCESS_EXTERNAL_STYLESHEET, 'file'")); + assertFalse("external stylesheet access must not be blocked outright (breaks includes)", + script.contains("ACCESS_EXTERNAL_STYLESHEET, ''")); + // FEATURE_SECURE_PROCESSING is intentionally not emitted: on the JDK's built-in Xalan it + // disables Java XSLT extension functions and would break existing stylesheets. Blocking + // external access is sufficient for the CVE. + assertFalse("secure processing must not be enabled (it breaks Java extension functions)", + script.contains("FEATURE_SECURE_PROCESSING")); + // Fail closed: the hardening must not be wrapped in a try/catch that would let a factory + // silently ignore the restrictions and run with external access still open. + assertFalse("XXE hardening must not be swallowed", script.contains("catch")); + } + + @Test + public void hardeningAlsoAppliedOnTheIteratorPath() throws Exception { + XsltStep step = new XsltStep(); + step.setSourceXml("connectorMessage.getRawData()"); + step.setResultVariable("xsltResult"); + step.setTemplate("''"); + + String script = step.getIterationScript(false, new java.util.LinkedList<>()); + + assertTrue("external DTD access should be denied on the iterator path", + script.contains("ACCESS_EXTERNAL_DTD, ''")); + assertTrue("external stylesheet access should be restricted to 'file' on the iterator path", + script.contains("ACCESS_EXTERNAL_STYLESHEET, 'file'")); + assertFalse("XXE hardening must not be swallowed on the iterator path", script.contains("catch")); + } +} diff --git a/smoketest/build.gradle b/smoketest/build.gradle index 4d121012c8..b6a2809c75 100644 --- a/smoketest/build.gradle +++ b/smoketest/build.gradle @@ -15,6 +15,11 @@ dependencies { testCompileOnly files(clientCoreJar) // RawMessage, Message, ConnectorMessage, MessageContent, Status, DeployedState testCompileOnly files(donkeyModelJar) + // Connector/plugin classes the security tests build channels with and call directly + // (DatabaseConnectorServletInterface, Table, XsltStep, XMLDataTypeProperties, + // VmReceiverProperties, ...). These live in the server module's main output and are present at + // runtime via /opt/engine/extensions; needed only to compile the tests, hence compileOnly. + testCompileOnly project(':server').sourceSets.main.output // Provided at runtime by /opt/engine/server-lib (server-main uses it too). testCompileOnly libs.snakeyaml diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java index 51ded95d85..93fd692371 100644 --- a/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java @@ -89,6 +89,15 @@ private static synchronized void initSerializer(String serverVersion) throws Exc */ String deployChannel(String xml, String label) throws Exception { Channel channel = ObjectXMLSerializer.getInstance().deserialize(xml, Channel.class); + return deployChannel(channel, label); + } + + /** + * Deploys a {@link Channel} built in code (used by the security tests that construct XSLT-step + * and XML-batch channels from a base fixture) and waits for it to reach + * {@link DeployedState#STARTED}. + */ + String deployChannel(Channel channel, String label) throws Exception { String channelId = channel.getId(); if (channelId == null || channelId.isBlank()) { throw new IllegalArgumentException("Channel fixture has no id: " + label); @@ -132,6 +141,11 @@ long submitMessage(String channelId, String rawData, Map sourceM return messageId; } + /** Returns up to {@code limit} of the most recent messages on a channel, with content. */ + List getMessages(String channelId, int limit) throws ClientException { + return client.getMessages(channelId, new MessageFilter(), true, 0, limit); + } + /** Reads one message back, with content, so assertions can inspect every connector. */ Message fetchMessage(String channelId, long messageId) throws ClientException { MessageFilter filter = new MessageFilter(); diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/SecurityChannels.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/SecurityChannels.java new file mode 100644 index 0000000000..317088998d --- /dev/null +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/SecurityChannels.java @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2026 Open Integration Engine + +package org.openintegrationengine.smoketest; + +import com.mirth.connect.donkey.model.channel.SourceConnectorPropertiesInterface; +import com.mirth.connect.model.Channel; +import com.mirth.connect.model.Connector; +import com.mirth.connect.model.Transformer; +import com.mirth.connect.model.converters.ObjectXMLSerializer; +import com.mirth.connect.plugins.datatypes.xml.XMLBatchProperties; +import com.mirth.connect.plugins.datatypes.xml.XMLBatchProperties.SplitType; +import com.mirth.connect.plugins.datatypes.xml.XMLDataTypeProperties; + +/** + * Builds the XML-batch channel the batch-XXE smoke test deploys. It starts from a known-good VM + * reader -> VM writer no-op channel ({@code fixtures/security/base-vm-noop.xml}, a copy of the + * 101-raw-no-op fixture) and customises only the source transformer, so the test does not have to + * hand-author connector XML. The serializer is already initialised by {@link OieServer}. + * + *

+ * The XSLT-step XXE repro is a fixture test under {@code ci/tests} instead; only the batch case needs + * code, because a rejected batch surfaces as a submit-time exception the fixture runner cannot model. + */ +final class SecurityChannels { + + private static final String BASE_RESOURCE = "fixtures/security/base-vm-noop.xml"; + + private SecurityChannels() { + } + + private static Channel base(String id, String name) { + Channel channel = ObjectXMLSerializer.getInstance().deserialize(Harness.resource(BASE_RESOURCE), Channel.class); + channel.setId(id); + channel.setName(name); + return channel; + } + + /** + * A channel with an XML data type source and batch processing enabled, splitting on an element + * name. Used to prove CVE-2026-82578: the batch adaptor parses the untrusted XML and resolves + * DOCTYPE entities (pre-fix) or rejects the DOCTYPE (post-fix). + */ + static Channel xmlBatchXxe(String id, String name, String splitElement) { + Channel channel = base(id, name); + Connector source = channel.getSourceConnector(); + + ((SourceConnectorPropertiesInterface) source.getProperties()).getSourceConnectorProperties() + .setProcessBatch(true); + + Transformer transformer = source.getTransformer(); + transformer.setInboundDataType("XML"); + + XMLDataTypeProperties xmlProperties = new XMLDataTypeProperties(); + XMLBatchProperties batchProperties = (XMLBatchProperties) xmlProperties.getBatchProperties(); + batchProperties.setSplitType(SplitType.Element_Name); + batchProperties.setElementName(splitElement); + transformer.setInboundProperties(xmlProperties); + + return channel; + } +} diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/XmlBatchXxeTest.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/XmlBatchXxeTest.java new file mode 100644 index 0000000000..6ddfef8738 --- /dev/null +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/XmlBatchXxeTest.java @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2026 Open Integration Engine + +package org.openintegrationengine.smoketest; + +import static org.junit.jupiter.api.Assertions.fail; + +import java.time.Duration; +import java.util.LinkedHashMap; + +import org.junit.jupiter.api.Test; + +import com.mirth.connect.donkey.model.message.ConnectorMessage; +import com.mirth.connect.donkey.model.message.Message; +import com.mirth.connect.donkey.model.message.MessageContent; +import com.mirth.connect.model.Channel; + +/** + * Validates CVE-2026-82578 end-to-end: the XML batch adaptor parsed untrusted batch XML with a + * DOCTYPE-resolving parser, expanding entities (the same parse that resolves external entities for + * file exfiltration). + * + *

+ * The channel splits a batch by element name. Two batches go through it: + *

+ * + *

+ * This asserts a specific marker rather than treating any exception as success. The external-file + * exfiltration vector (a {@code SYSTEM} entity) is covered precisely and hermetically by + * {@code XMLBatchAdaptorSecurityTest.rejectsExternalEntityDoctype} in the server module; here the + * fixed parser rejects internal and external DOCTYPEs identically ({@code disallow-doctype-decl}), so + * the internal marker is the assertable end-to-end oracle. + */ +class XmlBatchXxeTest { + + private static final String CHANNEL_ID = "5ec00002-0000-4000-8000-00000000ba7c"; + private static final String XXE_MARKER = "OIE_XXE_MARKER"; + private static final String BENIGN_MARKER = "OIE_BENIGN_CONTROL"; + + @Test + void doctypeEntityInBatchIsNotExpanded() throws Exception { + OieServer server = SharedServer.get(); + Channel channel = SecurityChannels.xmlBatchXxe(CHANNEL_ID, "SEC XML BATCH XXE", "message"); + String channelId = server.deployChannel(channel, "xml-batch-xxe"); + try { + // Positive control: a well-formed batch must split and produce a child with the marker. + String benign = "" + BENIGN_MARKER + ""; + server.submitMessage(channelId, benign, new LinkedHashMap<>()); + + Duration window = min(HarnessConfig.TIMEOUT, Duration.ofSeconds(20)); + if (!waitForMarker(server, channelId, BENIGN_MARKER, window)) { + fail("benign control batch never produced a split child containing '" + BENIGN_MARKER + + "' -- the batch-splitting path is broken, so the XXE assertion would be meaningless"); + } + + // Malicious batch: if the DOCTYPE entity is expanded, a split child contains the marker. + String malicious = "" + + "]>" + + "&x;"; + try { + server.submitMessage(channelId, malicious, new LinkedHashMap<>()); + } catch (Exception batchRejected) { + // Post-fix the batch parse throws; no expanded child is produced. Acceptable. + } + + // Give any (pre-fix) expanded child time to be stored, then assert the marker never appears. + long deadline = System.nanoTime() + window.toNanos(); + while (System.nanoTime() < deadline) { + for (Message message : server.getMessages(channelId, 50)) { + if (containsMarker(message, XXE_MARKER)) { + fail("XML batch adaptor expanded a DOCTYPE entity (message contained '" + XXE_MARKER + + "') -- CVE-2026-82578 is present"); + } + } + Thread.sleep(500); + } + } finally { + server.removeChannel(channelId); + } + } + + private static boolean waitForMarker(OieServer server, String channelId, String marker, Duration window) + throws Exception { + long deadline = System.nanoTime() + window.toNanos(); + while (System.nanoTime() < deadline) { + for (Message message : server.getMessages(channelId, 50)) { + if (containsMarker(message, marker)) { + return true; + } + } + Thread.sleep(500); + } + return false; + } + + private static boolean containsMarker(Message message, String marker) { + if (message.getConnectorMessages() == null) { + return false; + } + for (ConnectorMessage connectorMessage : message.getConnectorMessages().values()) { + if (contentContainsMarker(connectorMessage.getRaw(), marker) + || contentContainsMarker(connectorMessage.getTransformed(), marker) + || contentContainsMarker(connectorMessage.getEncoded(), marker)) { + return true; + } + } + return false; + } + + private static boolean contentContainsMarker(MessageContent content, String marker) { + return content != null && content.getContent() != null && content.getContent().contains(marker); + } + + private static Duration min(Duration a, Duration b) { + return a.compareTo(b) <= 0 ? a : b; + } +} diff --git a/smoketest/src/test/resources/fixtures/security/base-vm-noop.xml b/smoketest/src/test/resources/fixtures/security/base-vm-noop.xml new file mode 100644 index 0000000000..923b5d43e6 --- /dev/null +++ b/smoketest/src/test/resources/fixtures/security/base-vm-noop.xml @@ -0,0 +1,184 @@ + + 62af393b-ff61-47ec-b5fa-ccd2cd08ce55 + 2 + Noop + + 1 + + 0 + sourceConnector + + + + None + true + false + false + 1 + + + Default Resource + [Default Resource] + + + 1000 + + + + + RAW + RAW + + + JavaScript + + + + + + JavaScript + + + + + + + + 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} + + + + + RAW + RAW + + + JavaScript + + + + + + JavaScript + + + + + + + RAW + RAW + + + JavaScript + + + + + + JavaScript + + + + + + + + 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