Skip to content

Commit f263cd7

Browse files
jonbartelsclaude
andcommitted
Fix SQLi + XXE CVEs (CVE-2026-82583, -78224, -82578) with regression tests
Three vulnerabilities reported against NextGen/Mirth Connect are present in this 4.6.0 tree (below every upstream fix version). Each is fixed using the project's own established idioms, and each is covered by an integration test in the ci/smoketest harness that is RED on the vulnerable code and GREEN once the fix is applied. CVE-2026-82583 (SQL injection): DatabaseConnectorServlet.getTables executed the caller-supplied selectLimit query parameter verbatim via Statement.executeQuery. The Database connector metadata dialog only ever sends a selectLimit taken from the configured driver list, so selectLimit is now validated against that list (getDatabaseDrivers plus the built-in DriverInfo defaults, which are always included so a cleared list cannot disable the check) before any SQL runs. A non-allowlisted value is rejected with a generic exception that does not reflect the input. A blank value still routes to the safe DatabaseMetaData.getColumns path. Not fixed here (noted as follow-ups): the endpoint's missing @MirthOperation permission and unconstrained driver/url, a gap shared by every connector test servlet. CVE-2026-78224 (XSLT step XXE): XsltStep builds a TransformerFactory inside generated JavaScript, so it was never reached by the Java-side XML hardening. The generated script now enables FEATURE_SECURE_PROCESSING and sets ACCESS_EXTERNAL_DTD/ACCESS_EXTERNAL_STYLESHEET to "" (setAttribute guarded for implementations that reject it), on both the normal and iterator paths, blocking external entity resolution in the stylesheet and the source XML. CVE-2026-82578 (XML batch XXE): XMLBatchAdaptor evaluated XPath directly over an InputSource, letting XPath build its own DOCTYPE-resolving parser. It now parses with DocumentSerializer.getSecureDocumentBuilderFactory() (disallow-doctype-decl, external entities/DTD off, no entity expansion) and evaluates against the parsed Document. The output path in this file was already hardened; only the reader side was missed. Tests: - XsltStepSecurityTest (unit): asserts the hardening is emitted into the script. - DatabaseConnectorSqlInjectionTest, XsltStepXxeTest, XmlBatchXxeTest (smoketest): drive the live server; SQLi runs on DB-backed configurations (DB coordinates passed as oie.db.* via run-configuration.sh + harness.compose.yml) and skips embedded-Derby, the XXE tests build channels from a base fixture via new OieServer/SecurityChannels helpers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9359d9a commit f263cd7

13 files changed

Lines changed: 745 additions & 4 deletions

File tree

‎ci/harness.compose.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,9 @@ services:
1919
OIE_BASE_URL: https://oie:8443
2020
OIE_CONFIGURATION: ${OIE_CONFIGURATION}
2121
OIE_PASSWORD: ${OIE_ADMIN_PASSWORD}
22+
# Extra -D flags forwarded to the JUnit JVM. run-configuration.sh sets these to the DB
23+
# coordinates for DB-backed configurations so the SQL-injection test can reach the database;
24+
# empty for embedded-Derby configurations, where that test skips.
25+
OIE_HARNESS_OPTS: ${OIE_HARNESS_OPTS:-}
2226
volumes:
2327
- ${WORKSPACE}/ci/test-results:/results

‎ci/run-configuration.sh‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,36 @@ if [[ ! -f "$1" ]]; then
1414
exit 2
1515
fi
1616
export OIE_IMAGE="$2"
17-
export OIE_CONFIGURATION="$(basename "$1" .compose.yml)"
17+
OIE_CONFIGURATION="$(basename "$1" .compose.yml)"
18+
export OIE_CONFIGURATION
1819
# The engine generates a random admin password on first boot, so the stack and the
1920
# harness have to agree on one up front. See ci/harness.compose.yml.
2021
export OIE_ADMIN_PASSWORD="${OIE_ADMIN_PASSWORD:-ci-smoke-admin}"
2122
export WORKSPACE="$PWD"
22-
export HOST_UID="$(id -u)"
23-
export HOST_GID="$(id -g)"
23+
HOST_UID="$(id -u)"
24+
export HOST_UID
25+
HOST_GID="$(id -g)"
26+
export HOST_GID
2427
mkdir -p "$WORKSPACE/ci/test-results"
2528

29+
# Database coordinates for the DB-backed configurations, forwarded to the harness (see
30+
# ci/harness.compose.yml) so the SQL-injection smoke test can drive the JDBC metadata endpoint
31+
# against the same database the stack uses. The endpoint runs inside the oie container, so the URL
32+
# host is the compose "db" service. Embedded-Derby configurations expose no separate database
33+
# service and are left unset, so that test skips. Values are space-separated -D flags and must not
34+
# themselves contain spaces (run-harness.sh word-splits them onto the java command line).
35+
case "$OIE_CONFIGURATION" in
36+
*-postgres)
37+
export OIE_HARNESS_OPTS="-Doie.db.driver=org.postgresql.Driver -Doie.db.url=jdbc:postgresql://db:5432/mirthdb -Doie.db.user=mirthdb -Doie.db.password=mirthdb"
38+
;;
39+
*-mysql)
40+
export OIE_HARNESS_OPTS="-Doie.db.driver=com.mysql.cj.jdbc.Driver -Doie.db.url=jdbc:mysql://db:3306/mirthdb -Doie.db.user=mirthdb -Doie.db.password=mirthdb"
41+
;;
42+
*)
43+
export OIE_HARNESS_OPTS=""
44+
;;
45+
esac
46+
2647
compose=(docker compose -f "$1" -f ci/harness.compose.yml -p "oie-ci-${OIE_CONFIGURATION//[^a-z0-9-]/-}-$$")
2748

2849
cleanup() {

‎server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
import org.apache.logging.log4j.Logger;
3434

3535
import com.mirth.connect.client.core.api.MirthApiException;
36+
import com.mirth.connect.model.DriverInfo;
3637
import com.mirth.connect.server.api.MirthServlet;
38+
import com.mirth.connect.server.controllers.ConfigurationController;
3739
import com.mirth.connect.server.controllers.ContextFactoryController;
3840
import com.mirth.connect.server.controllers.ControllerFactory;
3941
import com.mirth.connect.server.util.TemplateValueReplacer;
@@ -45,13 +47,18 @@ public class DatabaseConnectorServlet extends MirthServlet implements DatabaseCo
4547
private static final Logger logger = LogManager.getLogger(DatabaseConnectorServlet.class);
4648
private static final TemplateValueReplacer replacer = new TemplateValueReplacer();
4749
private static final ContextFactoryController contextFactoryController = ControllerFactory.getFactory().createContextFactoryController();
50+
private static final ConfigurationController configurationController = ControllerFactory.getFactory().createConfigurationController();
4851

4952
public DatabaseConnectorServlet(@Context HttpServletRequest request, @Context SecurityContext sc) {
5053
super(request, sc, PLUGIN_POINT);
5154
}
5255

5356
@Override
5457
public SortedSet<Table> getTables(String channelId, String channelName, String driver, String url, String username, String password, Set<String> tableNamePatterns, String selectLimit, Set<String> resourceIds) {
58+
// Reject any selectLimit that is not one the server itself configured, before it can be
59+
// executed as SQL (CVE-2026-82583). Done outside the try below so it is not re-wrapped.
60+
validateSelectLimit(selectLimit);
61+
5562
CustomDriver customDriver = null;
5663
Connection connection = null;
5764
try {
@@ -229,6 +236,49 @@ public SortedSet<Table> getTables(String channelId, String channelName, String d
229236
}
230237
}
231238

239+
/**
240+
* Validates the caller-supplied {@code selectLimit} against the server's configured driver list
241+
* before it is ever executed as SQL. The Database connector metadata dialog only ever sends a
242+
* {@code selectLimit} taken from the configured drivers (dbdrivers.xml / {@link DriverInfo}), so
243+
* any other value is a SQL-injection attempt (CVE-2026-82583) and is rejected. A blank value is
244+
* allowed: it routes to the safe {@link DatabaseMetaData#getColumns} path. The exception message
245+
* is deliberately generic so the rejected value is not reflected back to the caller, and the
246+
* built-in default drivers are always included so an empty/cleared configured list cannot
247+
* disable the check (fail closed).
248+
*/
249+
private void validateSelectLimit(String selectLimit) {
250+
if (StringUtils.isBlank(selectLimit)) {
251+
return;
252+
}
253+
254+
Set<String> allowedSelectLimits = new HashSet<String>();
255+
addSelectLimits(allowedSelectLimits, DriverInfo.getDefaultDrivers());
256+
257+
try {
258+
addSelectLimits(allowedSelectLimits, configurationController.getDatabaseDrivers());
259+
} catch (Exception e) {
260+
// Fall back to the built-in driver list rather than failing open if the configured
261+
// list cannot be read.
262+
logger.warn("Could not load configured database drivers for selectLimit validation; using built-in defaults.", e);
263+
}
264+
265+
if (!allowedSelectLimits.contains(selectLimit.trim())) {
266+
logger.warn("Rejected database metadata request with a selectLimit that is not in the configured driver list.");
267+
throw new MirthApiException("The provided selectLimit is not permitted.");
268+
}
269+
}
270+
271+
private void addSelectLimits(Set<String> allowedSelectLimits, List<DriverInfo> drivers) {
272+
if (drivers == null) {
273+
return;
274+
}
275+
for (DriverInfo driver : drivers) {
276+
if (driver != null && driver.getSelectLimit() != null) {
277+
allowedSelectLimits.add(driver.getSelectLimit().trim());
278+
}
279+
}
280+
}
281+
232282
/**
233283
* Translate the given pattern expression so that it can be used properly for searching tables
234284
* in the database. Multiple table name patterns are delimited by comma (,)

‎server/src/main/java/com/mirth/connect/plugins/datatypes/xml/XMLBatchAdaptor.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.Map;
1919

2020
import javax.xml.XMLConstants;
21+
import javax.xml.parsers.DocumentBuilderFactory;
2122
import javax.xml.transform.OutputKeys;
2223
import javax.xml.transform.Transformer;
2324
import javax.xml.transform.TransformerFactory;
@@ -33,6 +34,7 @@
3334
import org.mozilla.javascript.Context;
3435
import org.mozilla.javascript.Script;
3536
import org.mozilla.javascript.Scriptable;
37+
import org.w3c.dom.Document;
3638
import org.w3c.dom.Node;
3739
import org.w3c.dom.NodeList;
3840
import org.xml.sax.InputSource;
@@ -42,6 +44,7 @@
4244
import com.mirth.connect.donkey.server.message.batch.BatchMessageException;
4345
import com.mirth.connect.donkey.server.message.batch.BatchMessageReader;
4446
import com.mirth.connect.donkey.server.message.batch.BatchMessageReceiver;
47+
import com.mirth.connect.model.converters.DocumentSerializer;
4548
import com.mirth.connect.plugins.datatypes.xml.XMLBatchProperties.SplitType;
4649
import com.mirth.connect.server.controllers.ContextFactoryController;
4750
import com.mirth.connect.server.controllers.ControllerFactory;
@@ -127,7 +130,19 @@ private String getMessageFromReader() throws Exception {
127130

128131
XPath xpath = xPathFactory.newXPath();
129132

130-
nodeList = (NodeList) xpath.evaluate(query.toString(), new InputSource(bufferedReader), XPathConstants.NODESET);
133+
// Parse the untrusted batch with a hardened parser before evaluating XPath, rather than
134+
// letting XPath.evaluate(InputSource) build its own DOCTYPE-resolving parser (XXE,
135+
// CVE-2026-82578). getSecureDocumentBuilderFactory() already sets disallow-doctype-decl;
136+
// the extra features below block external entities/DTDs and entity expansion outright.
137+
DocumentBuilderFactory dbf = DocumentSerializer.getSecureDocumentBuilderFactory();
138+
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
139+
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
140+
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
141+
dbf.setXIncludeAware(false);
142+
dbf.setExpandEntityReferences(false);
143+
Document document = dbf.newDocumentBuilder().parse(new InputSource(bufferedReader));
144+
145+
nodeList = (NodeList) xpath.evaluate(query.toString(), document, XPathConstants.NODESET);
131146
}
132147

133148
if (currentNode < nodeList.getLength()) {

‎server/src/main/java/com/mirth/connect/plugins/xsltstep/XsltStep.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ private String getTransformationScript() {
6666
script.append("tFactory = Packages.javax.xml.transform.TransformerFactory.newInstance();\n");
6767
}
6868

69+
// Harden the factory against XXE (CVE-2026-78224): enable secure processing and forbid
70+
// external DTD/stylesheet access so external entities in the stylesheet or the source XML
71+
// are not resolved. setAttribute is guarded because some implementations (e.g. Saxon) reject
72+
// these attributes; secure processing alone still applies. Mirrors XmlProcessor.configureSecureTF.
73+
script.append("tFactory.setFeature(Packages.javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING, true);\n");
74+
script.append("try { tFactory.setAttribute(Packages.javax.xml.XMLConstants.ACCESS_EXTERNAL_DTD, ''); } catch (e) {}\n");
75+
script.append("try { tFactory.setAttribute(Packages.javax.xml.XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ''); } catch (e) {}\n");
76+
6977
script.append("xsltTemplate = new Packages.java.io.StringReader(" + template + ");\n");
7078
script.append("transformer = tFactory.newTransformer(new Packages.javax.xml.transform.stream.StreamSource(xsltTemplate));\n");
7179
script.append("sourceVar = new Packages.java.io.StringReader(" + sourceXml + ");\n");
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright (c) Open Integration Engine. All rights reserved.
3+
*
4+
* The software in this package is published under the terms of the MPL license a copy of which has
5+
* been included with this distribution in the LICENSE.txt file.
6+
*/
7+
8+
package com.mirth.connect.plugins.xsltstep;
9+
10+
import static org.junit.Assert.assertTrue;
11+
12+
import org.junit.Test;
13+
14+
/**
15+
* Guards the XXE hardening of the XSLT transformer step (CVE-2026-78224). The step emits JavaScript
16+
* that builds a TransformerFactory at runtime, so the fix lives in the generated script text.
17+
*/
18+
public class XsltStepSecurityTest {
19+
20+
@Test
21+
public void generatedScriptHardensTransformerFactoryAgainstXxe() {
22+
XsltStep step = new XsltStep();
23+
step.setSourceXml("connectorMessage.getRawData()");
24+
step.setResultVariable("xsltResult");
25+
step.setTemplate("'<xsl:stylesheet version=\"1.0\"/>'");
26+
27+
String script = step.getScript(false);
28+
29+
assertTrue("secure processing should be enabled on the transformer factory",
30+
script.contains("FEATURE_SECURE_PROCESSING"));
31+
assertTrue("external DTD access should be disabled", script.contains("ACCESS_EXTERNAL_DTD"));
32+
assertTrue("external stylesheet access should be disabled", script.contains("ACCESS_EXTERNAL_STYLESHEET"));
33+
}
34+
35+
@Test
36+
public void hardeningAlsoAppliedOnTheIteratorPath() throws Exception {
37+
XsltStep step = new XsltStep();
38+
step.setSourceXml("connectorMessage.getRawData()");
39+
step.setResultVariable("xsltResult");
40+
step.setTemplate("'<xsl:stylesheet version=\"1.0\"/>'");
41+
42+
String script = step.getIterationScript(false, new java.util.LinkedList<>());
43+
44+
assertTrue("secure processing should be enabled on the iterator path",
45+
script.contains("FEATURE_SECURE_PROCESSING"));
46+
assertTrue("external DTD access should be disabled on the iterator path",
47+
script.contains("ACCESS_EXTERNAL_DTD"));
48+
}
49+
}

‎smoketest/build.gradle‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ dependencies {
1515
testCompileOnly files(clientCoreJar)
1616
// RawMessage, Message, ConnectorMessage, MessageContent, Status, DeployedState
1717
testCompileOnly files(donkeyModelJar)
18+
// Connector/plugin classes the security tests build channels with and call directly
19+
// (DatabaseConnectorServletInterface, Table, XsltStep, XMLDataTypeProperties,
20+
// VmReceiverProperties, ...). These live in the server module's main output and are present at
21+
// runtime via /opt/engine/extensions; needed only to compile the tests, hence compileOnly.
22+
testCompileOnly project(':server').sourceSets.main.output
1823
// Provided at runtime by /opt/engine/server-lib (server-main uses it too).
1924
testCompileOnly libs.snakeyaml
2025

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// SPDX-FileCopyrightText: 2026 Open Integration Engine
3+
4+
package org.openintegrationengine.smoketest;
5+
6+
import static org.junit.jupiter.api.Assertions.assertFalse;
7+
import static org.junit.jupiter.api.Assertions.assertTrue;
8+
import static org.junit.jupiter.api.Assumptions.assumeTrue;
9+
10+
import java.util.Set;
11+
import java.util.SortedSet;
12+
13+
import org.junit.jupiter.api.Test;
14+
15+
import com.mirth.connect.connectors.jdbc.Column;
16+
import com.mirth.connect.connectors.jdbc.Table;
17+
18+
/**
19+
* Validates CVE-2026-82583: the Database connector {@code _getTables} endpoint executed the
20+
* caller-supplied {@code selectLimit} as raw SQL.
21+
*
22+
* <p>
23+
* The test drives the same {@code getTables} API the Database connector metadata dialog uses,
24+
* against the database the CI stack already runs (coordinates passed as {@code oie.db.*} system
25+
* properties). It skips on embedded-Derby configurations that expose no separate database service.
26+
*
27+
* <p>
28+
* Pre-fix this test is RED (the injected {@code SELECT 1 AS OIE_SQLI_MARKER} runs and its alias
29+
* surfaces as a column); post-fix it is GREEN (the non-allowlisted {@code selectLimit} is rejected
30+
* before any SQL executes).
31+
*/
32+
class DatabaseConnectorSqlInjectionTest {
33+
34+
private static final String MARKER = "OIE_SQLI_MARKER";
35+
36+
@Test
37+
void selectLimitDoesNotExecuteArbitrarySql() throws Exception {
38+
Db db = Db.fromSystemProperties();
39+
assumeTrue(db != null, "oie.db.* coordinates not provided; skipping (embedded-database configuration)");
40+
41+
OieServer server = SharedServer.get();
42+
43+
// Discover a real table via the safe metadata path (empty selectLimit is always allowed).
44+
Table target = firstTableWithColumns(server.getConnectorTables(db.driver, db.url, db.user, db.password,
45+
Set.of("%"), ""));
46+
assumeTrue(target != null, "No table with columns found in the target database");
47+
48+
// If executed, this injected SELECT's alias becomes the sole returned column name.
49+
String maliciousSelectLimit = "SELECT 1 AS " + MARKER + " FROM ?";
50+
SortedSet<Table> result = null;
51+
try {
52+
result = server.getConnectorTables(db.driver, db.url, db.user, db.password, Set.of(target.getName()),
53+
maliciousSelectLimit);
54+
} catch (Exception rejectedByAllowlist) {
55+
// Post-fix: the non-allowlisted selectLimit is rejected before any SQL runs -> blocked.
56+
return;
57+
}
58+
59+
assertFalse(hasColumnNamed(result, MARKER), "selectLimit executed arbitrary SQL (returned an injected '"
60+
+ MARKER + "' column) -- CVE-2026-82583 is present");
61+
}
62+
63+
@Test
64+
void allowlistedSelectLimitStillReturnsColumns() throws Exception {
65+
Db db = Db.fromSystemProperties();
66+
assumeTrue(db != null, "oie.db.* coordinates not provided; skipping (embedded-database configuration)");
67+
String allowlisted = System.getProperty("oie.db.selectLimit", "SELECT * FROM ? LIMIT 1");
68+
69+
OieServer server = SharedServer.get();
70+
Table target = firstTableWithColumns(server.getConnectorTables(db.driver, db.url, db.user, db.password,
71+
Set.of("%"), ""));
72+
assumeTrue(target != null, "No table with columns found in the target database");
73+
74+
// The legitimate flow (a selectLimit taken from the configured driver list) must keep working.
75+
SortedSet<Table> result = server.getConnectorTables(db.driver, db.url, db.user, db.password,
76+
Set.of(target.getName()), allowlisted);
77+
assertTrue(result.stream().anyMatch(t -> !t.getColumns().isEmpty()),
78+
"An allowlisted selectLimit should still return table columns");
79+
assertFalse(hasColumnNamed(result, MARKER), "Unexpected injected column from an allowlisted selectLimit");
80+
}
81+
82+
private static Table firstTableWithColumns(SortedSet<Table> tables) {
83+
for (Table table : tables) {
84+
if (table.getColumns() != null && !table.getColumns().isEmpty()) {
85+
return table;
86+
}
87+
}
88+
return null;
89+
}
90+
91+
private static boolean hasColumnNamed(SortedSet<Table> tables, String columnName) {
92+
for (Table table : tables) {
93+
if (table.getColumns() == null) {
94+
continue;
95+
}
96+
for (Column column : table.getColumns()) {
97+
if (columnName.equalsIgnoreCase(column.getName())) {
98+
return true;
99+
}
100+
}
101+
}
102+
return false;
103+
}
104+
105+
/** DB coordinates supplied to the harness for the configuration under test. */
106+
private static final class Db {
107+
final String driver;
108+
final String url;
109+
final String user;
110+
final String password;
111+
112+
private Db(String driver, String url, String user, String password) {
113+
this.driver = driver;
114+
this.url = url;
115+
this.user = user;
116+
this.password = password;
117+
}
118+
119+
static Db fromSystemProperties() {
120+
String driver = System.getProperty("oie.db.driver");
121+
String url = System.getProperty("oie.db.url");
122+
String user = System.getProperty("oie.db.user");
123+
String password = System.getProperty("oie.db.password");
124+
if (driver == null || url == null || user == null || password == null) {
125+
return null;
126+
}
127+
return new Db(driver, url, user, password);
128+
}
129+
}
130+
}

0 commit comments

Comments
 (0)