From 00d80d17bafb797c42302f3dda2883404e24e593 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Wed, 22 Jul 2026 15:53:34 -0700 Subject: [PATCH 1/6] Use centrally configured SchemaFactory --- api/src/org/labkey/api/util/XmlBeansUtil.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index 69cbf03e430..bb225bd5631 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -33,6 +33,8 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParserFactory; import javax.xml.stream.XMLInputFactory; +import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; import java.util.Collection; import java.util.Date; import java.util.LinkedList; @@ -197,4 +199,40 @@ private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocTyp result.setExpandEntityReferences(false); return result; } + + /** + * A {@link SchemaFactory} hardened against XXE (CWE-611): external DTD and schema access blocked, + * secure processing on. Not thread-safe, so a fresh instance is returned per call. + */ + public static SchemaFactory schemaFactory() + { + //noinspection SchemaFactory + SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + try + { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + } + catch (SAXException e) + { + throw UnexpectedException.wrap(e); + } + return factory; + } + + // The Validator resolves entities in the instance document independently of the SchemaFactory, so it must be locked down separately. + public static Validator hardenValidator(Validator validator) + { + try + { + validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + } + catch (SAXException e) + { + throw UnexpectedException.wrap(e); + } + return validator; + } } From db2714ac90fb13975e0b3b295bd7b3b83978a04a Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Wed, 22 Jul 2026 17:22:42 -0700 Subject: [PATCH 2/6] Update per Claude code review guidance --- api/src/org/labkey/api/util/XmlBeansUtil.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index bb225bd5631..7779a8ee2ef 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -201,8 +201,9 @@ private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocTyp } /** - * A {@link SchemaFactory} hardened against XXE (CWE-611): external DTD and schema access blocked, - * secure processing on. Not thread-safe, so a fresh instance is returned per call. + * A {@link SchemaFactory} hardened against XXE (CWE-611): external DTD access blocked and external + * schema access limited to local protocols, secure processing on. Not thread-safe, so a fresh + * instance is returned per call. */ public static SchemaFactory schemaFactory() { @@ -212,7 +213,8 @@ public static SchemaFactory schemaFactory() { factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + // Bundled schemas compose sibling XSDs via import/include; permit local file/jar resolution while blocking network (http/https/ftp) access + factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar"); } catch (SAXException e) { From 770d1caab83e9e5f4b04e066f561d4a417124741 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Wed, 22 Jul 2026 17:35:13 -0700 Subject: [PATCH 3/6] Update per Claude code review guidance --- api/src/org/labkey/api/util/XmlBeansUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index 7779a8ee2ef..6f3b4cb3302 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -228,6 +228,7 @@ public static Validator hardenValidator(Validator validator) { try { + validator.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); } From ec6d54f83a6cb63b2b8da576533592614004ecb8 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 24 Jul 2026 14:31:09 -0700 Subject: [PATCH 4/6] Another attempt --- api/src/org/labkey/api/util/XmlBeansUtil.java | 54 +++++++++++++++---- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index 6f3b4cb3302..432a9a94202 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -15,6 +15,7 @@ */ package org.labkey.api.util; +import org.apache.logging.log4j.Logger; import org.apache.xmlbeans.XmlCursor; import org.apache.xmlbeans.XmlError; import org.apache.xmlbeans.XmlException; @@ -26,7 +27,10 @@ import org.labkey.api.portal.ProjectUrls; import org.labkey.api.security.User; import org.labkey.api.settings.LookAndFeelProperties; +import org.labkey.api.util.logging.LogHelper; import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilderFactory; @@ -41,6 +45,8 @@ public class XmlBeansUtil { + private static final Logger LOG = LogHelper.getLogger(XmlBeansUtil.class, "XML schema and validator XXE hardening"); + private XmlBeansUtil() { } @@ -209,33 +215,59 @@ public static SchemaFactory schemaFactory() { //noinspection SchemaFactory SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + require(() -> factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)); + attempt(() -> factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""), XMLConstants.ACCESS_EXTERNAL_DTD); + // Bundled schemas compose sibling XSDs via import/include; permit local file/jar resolution while blocking network (http/https/ftp) access + attempt(() -> factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar"), XMLConstants.ACCESS_EXTERNAL_SCHEMA); + return factory; + } + + // The Validator resolves entities in the instance document independently of the SchemaFactory, so it must be locked down separately. + public static Validator hardenValidator(Validator validator) + { + require(() -> validator.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)); + // Standalone Apache Xerces ignores the accessExternal* properties below, so these SAX features (which it does honor) are what actually block instance-document XXE on the server; the JDK factory relies on the properties instead. + attempt(() -> validator.setFeature("http://xml.org/sax/features/external-general-entities", false), "external-general-entities"); + attempt(() -> validator.setFeature("http://xml.org/sax/features/external-parameter-entities", false), "external-parameter-entities"); + attempt(() -> validator.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false), "load-external-dtd"); + attempt(() -> validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""), XMLConstants.ACCESS_EXTERNAL_DTD); + attempt(() -> validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""), XMLConstants.ACCESS_EXTERNAL_SCHEMA); + return validator; + } + + @FunctionalInterface + private interface XmlSetting + { + void apply() throws SAXException; + } + + // FEATURE_SECURE_PROCESSING is honored by every JAXP implementation, so failure to set it is fatal. + private static void require(XmlSetting setting) + { try { - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - // Bundled schemas compose sibling XSDs via import/include; permit local file/jar resolution while blocking network (http/https/ftp) access - factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar"); + setting.apply(); } catch (SAXException e) { throw UnexpectedException.wrap(e); } - return factory; } - // The Validator resolves entities in the instance document independently of the SchemaFactory, so it must be locked down separately. - public static Validator hardenValidator(Validator validator) + // Implementations recognize different subsets of the XXE controls (e.g. standalone Apache Xerces rejects the JAXP 1.5 accessExternal* properties, XERCESJ-1654), so each is attempted independently and a not-recognized setting is skipped rather than aborting the rest. + private static void attempt(XmlSetting setting, String name) { try { - validator.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + setting.apply(); + } + catch (SAXNotRecognizedException | SAXNotSupportedException e) + { + LOG.debug("XML implementation does not recognize {}; relying on the other hardening settings", name); } catch (SAXException e) { throw UnexpectedException.wrap(e); } - return validator; } } From 763f38083d9c63dbeda54810ecbf4bf377a29e36 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Thu, 6 Aug 2026 18:40:38 -0700 Subject: [PATCH 5/6] Improve test coverage --- api/src/org/labkey/api/ApiModule.java | 4 +- .../api/util/ExternalReferenceProbe.java | 109 +++ api/src/org/labkey/api/util/XmlBeansUtil.java | 824 ++++++++++++------ 3 files changed, 663 insertions(+), 274 deletions(-) create mode 100644 api/src/org/labkey/api/util/ExternalReferenceProbe.java diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 407bae5d230..057dcfa5b70 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -180,6 +180,7 @@ import org.labkey.api.util.SystemMaintenanceStartupListener; import org.labkey.api.util.URIUtil; import org.labkey.api.util.URLHelper; +import org.labkey.api.util.XmlBeansUtil; import org.labkey.api.util.emailTemplate.EmailTemplate; import org.labkey.api.view.ActionURL; import org.labkey.api.view.FileServlet; @@ -482,7 +483,8 @@ public void registerServlets(ServletContext servletCtx) TabLoader.HeaderMatchTest.class, Table.IsSelectTestCase.class, URIUtil.TestCase.class, - ValidEmail.TestCase.class + ValidEmail.TestCase.class, + XmlBeansUtil.TestCase.class ); } diff --git a/api/src/org/labkey/api/util/ExternalReferenceProbe.java b/api/src/org/labkey/api/util/ExternalReferenceProbe.java new file mode 100644 index 00000000000..2e145b09142 --- /dev/null +++ b/api/src/org/labkey/api/util/ExternalReferenceProbe.java @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.util; + +import com.sun.net.httpserver.HttpServer; +import org.jetbrains.annotations.NotNull; +import org.junit.Assert; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; + +/** + * Test helper that plays the attacker-controlled host at the far end of an XML external reference: a recorded request + * proves the XML machinery resolved that reference over the network, the XXE (CWE-611) behavior {@link XmlBeansUtil} + * exists to prevent. + * + *

A real loopback server rather than a {@code ProxySelector}, which is global JVM state visible to every other + * thread in a running server. + * + *

The probe only sees references it hosts, so a test proving "nothing external at all" must point every reference + * in its fixture at {@link #url}. Not thread-safe across tests: start and close one per test. + */ +public class ExternalReferenceProbe implements AutoCloseable +{ + /** A DTD body that is well-formed enough for a parser to accept once it has been fetched. */ + public static final String DTD_BODY = ""; + /** An entity replacement body that is visible in the parsed document if expansion happened. */ + public static final String ENTITY_BODY = "external-content-was-fetched"; + + private final HttpServer _server; + private final List _contacted = new CopyOnWriteArrayList<>(); + private final Map _bodies = new ConcurrentHashMap<>(); + + private ExternalReferenceProbe(HttpServer server) + { + _server = server; + } + + public static ExternalReferenceProbe start() throws IOException + { + HttpServer server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); + ExternalReferenceProbe probe = new ExternalReferenceProbe(server); + server.createContext("/", exchange -> { + String path = exchange.getRequestURI().getPath(); + probe._contacted.add(path); + byte[] body = probe._bodies.getOrDefault(path, "").getBytes(StandardCharsets.UTF_8); + exchange.sendResponseHeaders(200, body.length); + try (OutputStream out = exchange.getResponseBody()) + { + out.write(body); + } + }); + server.start(); + return probe; + } + + /** + * Register {@code body} at {@code path} and return the absolute URL to embed in the XML under test. + * @param path must start with "/", e.g. "/external.dtd" + */ + public String url(@NotNull String path, @NotNull String body) + { + _bodies.put(path, body); + return "http://" + _server.getAddress().getHostString() + ":" + _server.getAddress().getPort() + path; + } + + /** Paths the XML machinery actually requested, in order. */ + public @NotNull List contactedPaths() + { + return List.copyOf(_contacted); + } + + public boolean wasContacted() + { + return !_contacted.isEmpty(); + } + + public void assertNotContacted(String message) + { + Assert.assertTrue(message + " -- external reference(s) were resolved over the network: " + _contacted, + _contacted.isEmpty()); + } + + @Override + public void close() + { + _server.stop(0); + } +} diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index 432a9a94202..66f01aa3612 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -1,273 +1,551 @@ -/* - * Copyright (c) 2009-2026 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.labkey.api.util; - -import org.apache.logging.log4j.Logger; -import org.apache.xmlbeans.XmlCursor; -import org.apache.xmlbeans.XmlError; -import org.apache.xmlbeans.XmlException; -import org.apache.xmlbeans.XmlObject; -import org.apache.xmlbeans.XmlOptions; -import org.apache.xmlbeans.XmlTokenSource; -import org.jetbrains.annotations.Nullable; -import org.labkey.api.data.Container; -import org.labkey.api.portal.ProjectUrls; -import org.labkey.api.security.User; -import org.labkey.api.settings.LookAndFeelProperties; -import org.labkey.api.util.logging.LogHelper; -import org.xml.sax.SAXException; -import org.xml.sax.SAXNotRecognizedException; -import org.xml.sax.SAXNotSupportedException; - -import javax.xml.XMLConstants; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParserFactory; -import javax.xml.stream.XMLInputFactory; -import javax.xml.validation.SchemaFactory; -import javax.xml.validation.Validator; -import java.util.Collection; -import java.util.Date; -import java.util.LinkedList; - -public class XmlBeansUtil -{ - private static final Logger LOG = LogHelper.getLogger(XmlBeansUtil.class, "XML schema and validator XXE hardening"); - - private XmlBeansUtil() - { - } - - // Standard options used by folder export - public static XmlOptions getDefaultSaveOptions() - { - XmlOptions options = new XmlOptions(); - options.setSavePrettyPrint(); - options.setUseDefaultNamespace(); - options.setCharacterEncoding("UTF-8"); - options.setSaveCDataEntityCountThreshold(0); - options.setSaveCDataLengthThreshold(0); - options.setSaveAggressiveNamespaces(); // causes the saver to reduce the number of namespace declarations - - return options; - } - - // Standard options used for parsing to enable validation. - public static XmlOptions getDefaultParseOptions() - { - XmlOptions options = new XmlOptions(); - options.setLoadLineNumbers(); - - return options; - } - - @Deprecated // Use the version below, and pass in details (filename, etc.) - public static void validateXmlDocument(XmlObject doc) throws XmlValidationException - { - validateXmlDocument(doc, null); - } - - // Details can be filename, etc. to help admin narrow down the source of the problem - public static void validateXmlDocument(XmlObject doc, @Nullable String details) throws XmlValidationException - { - XmlOptions options = getDefaultParseOptions(); - Collection errorList = new LinkedList<>(); - options.setErrorListener(errorList); - - if (!doc.validate(options)) - throw new XmlValidationException(errorList, doc.schemaType().toString(), details); - } - - public static String getErrorMessage(XmlException ex) - { - if (ex.getError() != null) - return getErrorMessage(ex.getError()); - return ex.getMessage(); - } - - public static String getErrorMessage(XmlError error) - { - StringBuilder sb = new StringBuilder(); - sb.append(error.toString()); - if (error.getLine() > 0) - { - sb.append(" (line ").append(error.getLine()); - if (error.getColumn() > 0) - sb.append(", column ").append(error.getColumn()); - sb.append(")"); - } - return sb.toString(); - } - - // Insert standard export comment explaining where the data lives, who exported it, and when - public static void addStandardExportComment(XmlTokenSource doc, Container c, User user) - { - String urlString = PageFlowUtil.urlProvider(ProjectUrls.class).getBeginURL(c).getURIString(); - if (urlString.endsWith("?")) - urlString = urlString.substring(0, urlString.length() - 1); - String shortName = LookAndFeelProperties.getInstance(c).getShortName(); - String comment = "Exported from " + shortName + " at " + urlString + " by " + user.getFriendlyName() + " on " + new Date(); - addComment(doc, comment); - } - - public static void addComment(XmlTokenSource doc, String comment) - { - try (XmlCursor cursor = doc.newCursor()) - { - cursor.insertComment(comment); - } - } - - /** - * XML parsing factories preconfigured to prevent XML external entity references (XXE). - * These are static and are unfortunately mutable. We could switch to a factory pattern to create - * freshly configured factories. - */ - public static final SAXParserFactory SAX_PARSER_FACTORY; - public static final SAXParserFactory SAX_PARSER_FACTORY_ALLOWING_DOCTYPE; - public static final XMLInputFactory XML_INPUT_FACTORY; - public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY; - public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE; - - static - { - //noinspection XMLInputFactory - XML_INPUT_FACTORY = XMLInputFactory.newInstance(); - XML_INPUT_FACTORY.setProperty(XMLInputFactory.SUPPORT_DTD, false); - XML_INPUT_FACTORY.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); - - try - { - SAX_PARSER_FACTORY = saxParserFactory(false); - SAX_PARSER_FACTORY_ALLOWING_DOCTYPE = saxParserFactory(true); - - DOCUMENT_BUILDER_FACTORY = documentBuilderFactory(false); - // Use the ALLOWING_DOCTYPE variant when parsing XML that contains a declaration (e.g. NCBI's eSummary responses) - DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE = documentBuilderFactory(true); - } - catch (ParserConfigurationException | SAXException e) - { - throw UnexpectedException.wrap(e); - } - } - - private static SAXParserFactory saxParserFactory(boolean allowDocType) throws SAXException, ParserConfigurationException - { - //noinspection XMLInputFactory - SAXParserFactory result = SAXParserFactory.newInstance(); - result.setNamespaceAware(true); - result.setFeature("http://xml.org/sax/features/validation", false); - result.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - - // Disable features that could lead to XXE or other vulnerabilities - // Keep in sync with ModuleArchive.nameFromModuleXML() - if (!allowDocType) - { - result.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - } - result.setFeature("http://xml.org/sax/features/external-general-entities", false); - result.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - return result; - } - - private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocType) throws ParserConfigurationException - { - //noinspection XMLInputFactory - DocumentBuilderFactory result = DocumentBuilderFactory.newInstance(); - result.setNamespaceAware(true); - - // Disable features that could lead to XXE or other vulnerabilities. - // When allowDocType is true the DOCTYPE declaration is permitted. External entity - // resolution remains disabled, so XXE protection is still in effect. - if (!allowDocType) - { - result.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - } - result.setFeature("http://xml.org/sax/features/external-general-entities", false); - result.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - result.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - result.setXIncludeAware(false); - result.setExpandEntityReferences(false); - return result; - } - - /** - * A {@link SchemaFactory} hardened against XXE (CWE-611): external DTD access blocked and external - * schema access limited to local protocols, secure processing on. Not thread-safe, so a fresh - * instance is returned per call. - */ - public static SchemaFactory schemaFactory() - { - //noinspection SchemaFactory - SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); - require(() -> factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)); - attempt(() -> factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""), XMLConstants.ACCESS_EXTERNAL_DTD); - // Bundled schemas compose sibling XSDs via import/include; permit local file/jar resolution while blocking network (http/https/ftp) access - attempt(() -> factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar"), XMLConstants.ACCESS_EXTERNAL_SCHEMA); - return factory; - } - - // The Validator resolves entities in the instance document independently of the SchemaFactory, so it must be locked down separately. - public static Validator hardenValidator(Validator validator) - { - require(() -> validator.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)); - // Standalone Apache Xerces ignores the accessExternal* properties below, so these SAX features (which it does honor) are what actually block instance-document XXE on the server; the JDK factory relies on the properties instead. - attempt(() -> validator.setFeature("http://xml.org/sax/features/external-general-entities", false), "external-general-entities"); - attempt(() -> validator.setFeature("http://xml.org/sax/features/external-parameter-entities", false), "external-parameter-entities"); - attempt(() -> validator.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false), "load-external-dtd"); - attempt(() -> validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""), XMLConstants.ACCESS_EXTERNAL_DTD); - attempt(() -> validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""), XMLConstants.ACCESS_EXTERNAL_SCHEMA); - return validator; - } - - @FunctionalInterface - private interface XmlSetting - { - void apply() throws SAXException; - } - - // FEATURE_SECURE_PROCESSING is honored by every JAXP implementation, so failure to set it is fatal. - private static void require(XmlSetting setting) - { - try - { - setting.apply(); - } - catch (SAXException e) - { - throw UnexpectedException.wrap(e); - } - } - - // Implementations recognize different subsets of the XXE controls (e.g. standalone Apache Xerces rejects the JAXP 1.5 accessExternal* properties, XERCESJ-1654), so each is attempted independently and a not-recognized setting is skipped rather than aborting the rest. - private static void attempt(XmlSetting setting, String name) - { - try - { - setting.apply(); - } - catch (SAXNotRecognizedException | SAXNotSupportedException e) - { - LOG.debug("XML implementation does not recognize {}; relying on the other hardening settings", name); - } - catch (SAXException e) - { - throw UnexpectedException.wrap(e); - } - } -} +/* + * Copyright (c) 2009-2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.util; + +import org.apache.logging.log4j.Logger; +import org.apache.xmlbeans.XmlCursor; +import org.apache.xmlbeans.XmlError; +import org.apache.xmlbeans.XmlException; +import org.apache.xmlbeans.XmlObject; +import org.apache.xmlbeans.XmlOptions; +import org.apache.xmlbeans.XmlTokenSource; +import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.data.Container; +import org.labkey.api.portal.ProjectUrls; +import org.labkey.api.security.User; +import org.labkey.api.settings.LookAndFeelProperties; +import org.labkey.api.util.logging.LogHelper; +import org.w3c.dom.ls.LSInput; +import org.w3c.dom.ls.LSResourceResolver; +import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParserFactory; +import javax.xml.stream.XMLInputFactory; +import javax.xml.transform.stream.StreamSource; +import javax.xml.validation.Schema; +import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; +import java.io.StringReader; +import java.net.URI; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collection; +import java.util.Date; +import java.util.LinkedList; +import java.util.function.Function; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; + +public class XmlBeansUtil +{ + private static final Logger LOG = LogHelper.getLogger(XmlBeansUtil.class, "XML schema and validator XXE hardening"); + + private XmlBeansUtil() + { + } + + // Standard options used by folder export + public static XmlOptions getDefaultSaveOptions() + { + XmlOptions options = new XmlOptions(); + options.setSavePrettyPrint(); + options.setUseDefaultNamespace(); + options.setCharacterEncoding("UTF-8"); + options.setSaveCDataEntityCountThreshold(0); + options.setSaveCDataLengthThreshold(0); + options.setSaveAggressiveNamespaces(); // causes the saver to reduce the number of namespace declarations + + return options; + } + + // Standard options used for parsing to enable validation. + public static XmlOptions getDefaultParseOptions() + { + XmlOptions options = new XmlOptions(); + options.setLoadLineNumbers(); + + return options; + } + + @Deprecated // Use the version below, and pass in details (filename, etc.) + public static void validateXmlDocument(XmlObject doc) throws XmlValidationException + { + validateXmlDocument(doc, null); + } + + // Details can be filename, etc. to help admin narrow down the source of the problem + public static void validateXmlDocument(XmlObject doc, @Nullable String details) throws XmlValidationException + { + XmlOptions options = getDefaultParseOptions(); + Collection errorList = new LinkedList<>(); + options.setErrorListener(errorList); + + if (!doc.validate(options)) + throw new XmlValidationException(errorList, doc.schemaType().toString(), details); + } + + public static String getErrorMessage(XmlException ex) + { + if (ex.getError() != null) + return getErrorMessage(ex.getError()); + return ex.getMessage(); + } + + public static String getErrorMessage(XmlError error) + { + StringBuilder sb = new StringBuilder(); + sb.append(error.toString()); + if (error.getLine() > 0) + { + sb.append(" (line ").append(error.getLine()); + if (error.getColumn() > 0) + sb.append(", column ").append(error.getColumn()); + sb.append(")"); + } + return sb.toString(); + } + + // Insert standard export comment explaining where the data lives, who exported it, and when + public static void addStandardExportComment(XmlTokenSource doc, Container c, User user) + { + String urlString = PageFlowUtil.urlProvider(ProjectUrls.class).getBeginURL(c).getURIString(); + if (urlString.endsWith("?")) + urlString = urlString.substring(0, urlString.length() - 1); + String shortName = LookAndFeelProperties.getInstance(c).getShortName(); + String comment = "Exported from " + shortName + " at " + urlString + " by " + user.getFriendlyName() + " on " + new Date(); + addComment(doc, comment); + } + + public static void addComment(XmlTokenSource doc, String comment) + { + try (XmlCursor cursor = doc.newCursor()) + { + cursor.insertComment(comment); + } + } + + /** + * XML parsing factories preconfigured to prevent XML external entity references (XXE). + * These are static and are unfortunately mutable. We could switch to a factory pattern to create + * freshly configured factories. + */ + public static final SAXParserFactory SAX_PARSER_FACTORY; + public static final SAXParserFactory SAX_PARSER_FACTORY_ALLOWING_DOCTYPE; + public static final XMLInputFactory XML_INPUT_FACTORY; + public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY; + public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE; + + static + { + //noinspection XMLInputFactory + XML_INPUT_FACTORY = XMLInputFactory.newInstance(); + XML_INPUT_FACTORY.setProperty(XMLInputFactory.SUPPORT_DTD, false); + XML_INPUT_FACTORY.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + + try + { + SAX_PARSER_FACTORY = saxParserFactory(false); + SAX_PARSER_FACTORY_ALLOWING_DOCTYPE = saxParserFactory(true); + + DOCUMENT_BUILDER_FACTORY = documentBuilderFactory(false); + // Use the ALLOWING_DOCTYPE variant when parsing XML that contains a declaration (e.g. NCBI's eSummary responses) + DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE = documentBuilderFactory(true); + } + catch (ParserConfigurationException | SAXException e) + { + throw UnexpectedException.wrap(e); + } + } + + private static SAXParserFactory saxParserFactory(boolean allowDocType) throws SAXException, ParserConfigurationException + { + //noinspection XMLInputFactory + SAXParserFactory result = SAXParserFactory.newInstance(); + result.setNamespaceAware(true); + result.setFeature("http://xml.org/sax/features/validation", false); + result.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + + // Disable features that could lead to XXE or other vulnerabilities + // Keep in sync with ModuleArchive.nameFromModuleXML() + if (!allowDocType) + { + result.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + } + result.setFeature("http://xml.org/sax/features/external-general-entities", false); + result.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + return result; + } + + private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocType) throws ParserConfigurationException + { + //noinspection XMLInputFactory + DocumentBuilderFactory result = DocumentBuilderFactory.newInstance(); + result.setNamespaceAware(true); + + // Disable features that could lead to XXE or other vulnerabilities. + // When allowDocType is true the DOCTYPE declaration is permitted. External entity + // resolution remains disabled, so XXE protection is still in effect. + if (!allowDocType) + { + result.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + } + result.setFeature("http://xml.org/sax/features/external-general-entities", false); + result.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + result.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + result.setXIncludeAware(false); + result.setExpandEntityReferences(false); + return result; + } + + /** A {@link SchemaFactory} hardened against XXE (CWE-611). Not thread-safe, so a fresh instance per call. */ + public static SchemaFactory schemaFactory() + { + //noinspection SchemaFactory + SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + require(() -> factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)); + // Xerces rejects both, but set for the JDK implementation + attempt(() -> factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""), XMLConstants.ACCESS_EXTERNAL_DTD); + attempt(() -> factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar"), XMLConstants.ACCESS_EXTERNAL_SCHEMA); + // Bundled schemas import sibling XSDs, so local references must still resolve + factory.setResourceResolver(LOCAL_ONLY_RESOLVER); + return factory; + } + + // The Validator resolves entities in the instance document independently of the SchemaFactory, so it must be locked down separately. + public static Validator hardenValidator(Validator validator) + { + require(() -> validator.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)); + // Xerces accepts these but never applies them to StreamSource input: StreamValidatorHelper builds its own XML11Configuration and copies properties, not features + attempt(() -> validator.setFeature("http://xml.org/sax/features/external-general-entities", false), "external-general-entities"); + attempt(() -> validator.setFeature("http://xml.org/sax/features/external-parameter-entities", false), "external-parameter-entities"); + attempt(() -> validator.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false), "load-external-dtd"); + attempt(() -> validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""), XMLConstants.ACCESS_EXTERNAL_DTD); + attempt(() -> validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""), XMLConstants.ACCESS_EXTERNAL_SCHEMA); + // Stored as the ENTITY_RESOLVER property, which that copy does carry across, so this is what actually blocks XXE under Xerces + validator.setResourceResolver(REFUSE_ALL_RESOLVER); + return validator; + } + + /** Resolves to nothing, so a refused reference expands to the empty string instead of being fetched. */ + private static final LSInput EMPTY_INPUT = new LSInput() + { + @Override public Reader getCharacterStream() { return new StringReader(""); } + @Override public void setCharacterStream(Reader characterStream) { } + @Override public InputStream getByteStream() { return null; } + @Override public void setByteStream(InputStream byteStream) { } + @Override public String getStringData() { return ""; } + @Override public void setStringData(String stringData) { } + @Override public String getSystemId() { return null; } + @Override public void setSystemId(String systemId) { } + @Override public String getPublicId() { return null; } + @Override public void setPublicId(String publicId) { } + @Override public String getBaseURI() { return null; } + @Override public void setBaseURI(String baseURI) { } + @Override public String getEncoding() { return StandardCharsets.UTF_8.name(); } + @Override public void setEncoding(String encoding) { } + @Override public boolean getCertifiedText() { return false; } + @Override public void setCertifiedText(boolean certifiedText) { } + }; + + /** The instance document is the attacker-supplied half of every call site, and never legitimately references anything external. */ + private static final LSResourceResolver REFUSE_ALL_RESOLVER = (_, _, _, systemId, _) -> { + LOG.warn("Refused external reference to {} while validating XML", systemId); + return EMPTY_INPUT; + }; + + /** Debug rather than warn: bundled xenc-schema-11.xsd still declares a w3.org DOCTYPE, so this fires on every SAML schema compile. */ + private static final LSResourceResolver LOCAL_ONLY_RESOLVER = (_, _, _, systemId, baseURI) -> { + if (isLocal(systemId, baseURI)) + return null; // fall through to the default resolver + LOG.debug("Refused non-local schema reference to {} while compiling XML schema", systemId); + return EMPTY_INPUT; + }; + + /** A jar: base URI is opaque, so {@link URI#resolve} leaves a relative reference untouched -- it stays scheme-less, which counts as local. */ + private static boolean isLocal(@Nullable String systemId, @Nullable String baseURI) + { + if (systemId == null) + return true; + + try + { + URI uri = URI.create(systemId); + if (!uri.isAbsolute() && baseURI != null) + uri = URI.create(baseURI).resolve(uri); + String scheme = uri.getScheme(); + return scheme == null || "file".equalsIgnoreCase(scheme) || "jar".equalsIgnoreCase(scheme); + } + catch (IllegalArgumentException e) + { + return false; // unparseable, so not something we're willing to vouch for + } + } + + @FunctionalInterface + private interface XmlSetting + { + void apply() throws SAXException; + } + + // FEATURE_SECURE_PROCESSING is honored by every JAXP implementation, so failure to set it is fatal. + private static void require(XmlSetting setting) + { + try + { + setting.apply(); + } + catch (SAXException e) + { + throw UnexpectedException.wrap(e); + } + } + + // Implementations recognize different subsets of these controls (Xerces rejects the accessExternal* properties, XERCESJ-1654), so skip a rejected setting rather than aborting the rest. + private static void attempt(XmlSetting setting, String name) + { + try + { + setting.apply(); + } + catch (SAXNotRecognizedException | SAXNotSupportedException e) + { + LOG.debug("XML implementation does not recognize {}; relying on the other hardening settings", name); + } + catch (SAXException e) + { + throw UnexpectedException.wrap(e); + } + } + + /** + * Covers the XXE (CWE-611) contract of {@link #schemaFactory()} and {@link #hardenValidator(Validator)}, asserting + * on observable behavior rather than on which properties were set: a real server resolves Xerces instead of the + * JDK's JAXP, the two honor different subsets of the controls, and {@link #attempt} swallows the rejections -- so + * the hardening can compile, look correct, and do nothing. + */ + public static class TestCase extends Assert + { + private static final String TRIVIAL_XSD = + "" + + "" + + ""; + + // ---------- hardenValidator(): instance-document XXE ---------- + + @Test + public void hardenedValidatorRefusesExternalDtdSubset() throws Exception + { + assertHardenedValidatorRefuses("external DTD subset", probe -> + "" + + "hello"); + } + + @Test + public void hardenedValidatorRefusesExternalGeneralEntity() throws Exception + { + assertHardenedValidatorRefuses("external general entity", probe -> + "]>" + + "&x;"); + } + + @Test + public void hardenedValidatorRefusesExternalParameterEntity() throws Exception + { + assertHardenedValidatorRefuses("external parameter entity", probe -> + "") + "\">%p;]>" + + "hello"); + } + + /** + * Harness self-check: without it a URL typo would make the three tests above pass with no protection in place. + * If the platform default ever becomes safe on its own, delete this rather than weakening those assertions. + */ + @Test + public void probeDetectsFetchWhenValidatorIsNotHardened() throws Exception + { + try (ExternalReferenceProbe probe = ExternalReferenceProbe.start()) + { + String xml = "" + + "hello"; + //noinspection SchemaFactory + SchemaFactory raw = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + Validator validator = raw.newSchema(new StreamSource(new StringReader(TRIVIAL_XSD))).newValidator(); + validateIgnoringErrors(validator, xml); + + assertTrue("Probe must observe a fetch from an unhardened validator, otherwise the hardening " + + "tests in this class prove nothing", probe.wasContacted()); + } + } + + // ---------- schemaFactory(): schema-document XXE ---------- + + @Test + public void schemaFactoryRefusesExternalDoctypeInSchemaDocument() throws Exception + { + try (ExternalReferenceProbe probe = ExternalReferenceProbe.start()) + { + String xsd = "") + "\">" + TRIVIAL_XSD; + compileIgnoringErrors(schemaFactory(), new StreamSource(new StringReader(xsd))); + + probe.assertNotContacted("schemaFactory() must not fetch a DOCTYPE declared by a schema document"); + } + } + + @Test + public void schemaFactoryRefusesRemoteImport() throws Exception + { + try (ExternalReferenceProbe probe = ExternalReferenceProbe.start()) + { + String remote = probe.url("/remote.xsd", + ""); + String xsd = "" + + "" + + "" + + ""; + compileIgnoringErrors(schemaFactory(), new StreamSource(new StringReader(xsd))); + + probe.assertNotContacted("schemaFactory() must not fetch a remote xs:import"); + } + } + + // ---------- over-blocking guards ---------- + // Bundled schemas compose sibling XSDs by relative path, from a jar: URL when deployed and a file: URL in a dev build. + + @Test + public void schemaFactoryCompilesLocalImportChainFromFileUrl() throws Exception + { + Path dir = Files.createTempDirectory("xmlBeansUtilFile"); + + try + { + writeSchemaPair(dir); + Schema schema = schemaFactory().newSchema(dir.resolve("main.xsd").toUri().toURL()); + + assertNotNull("A local, relative xs:import must still resolve from a file: URL", schema); + } + finally + { + FileUtil.deleteDir(dir.toFile()); + } + } + + @Test + public void schemaFactoryCompilesLocalImportChainFromJarUrl() throws Exception + { + Path dir = Files.createTempDirectory("xmlBeansUtilJar"); + + try + { + writeSchemaPair(dir); + File jar = FileUtil.appendName(dir.toFile(), "schemas.jar"); + + try (JarOutputStream out = new JarOutputStream(new FileOutputStream(jar))) + { + for (String name : new String[]{"main.xsd", "imported.xsd"}) + { + out.putNextEntry(new JarEntry("schemas/" + name)); + out.write(Files.readAllBytes(dir.resolve(name))); + out.closeEntry(); + } + } + + URL url = URI.create("jar:" + jar.toURI() + "!/schemas/main.xsd").toURL(); + Schema schema = schemaFactory().newSchema(url); + + assertNotNull("A local, relative xs:import must still resolve from a jar: URL, which is how " + + "schemas are loaded in a deployed server", schema); + } + finally + { + FileUtil.deleteDir(dir.toFile()); + } + } + + // ---------- helpers ---------- + + private void assertHardenedValidatorRefuses(String vector, Function instanceDoc) throws Exception + { + try (ExternalReferenceProbe probe = ExternalReferenceProbe.start()) + { + String xml = instanceDoc.apply(probe); + Validator validator = hardenValidator(schemaFactory() + .newSchema(new StreamSource(new StringReader(TRIVIAL_XSD))) + .newValidator()); + validateIgnoringErrors(validator, xml); + + probe.assertNotContacted("hardenValidator() must not resolve the " + vector + " of an instance document"); + } + } + + /** Whether the document is schema-valid is beside the point; the fetch is the signal. */ + private void validateIgnoringErrors(Validator validator, String xml) throws IOException + { + try + { + validator.validate(new StreamSource(new StringReader(xml))); + } + catch (SAXException ignored) + { + } + } + + /** Likewise: a refused reference may or may not surface as a compile error. */ + private void compileIgnoringErrors(SchemaFactory factory, StreamSource source) + { + try + { + factory.newSchema(source); + } + catch (SAXException ignored) + { + } + } + + /** The relative-import shape every bundled LabKey schema chain uses. */ + private void writeSchemaPair(Path dir) throws IOException + { + Files.writeString(dir.resolve("imported.xsd"), + "" + + "" + + "", StandardCharsets.UTF_8); + + Files.writeString(dir.resolve("main.xsd"), + "" + + "" + + "" + + "" + + "" + + "", StandardCharsets.UTF_8); + } + } +} From 4b14a486150b191fefe6cee4b26737cdf5a034a4 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Thu, 6 Aug 2026 19:29:21 -0700 Subject: [PATCH 6/6] Improve comments --- .../api/util/ExternalReferenceProbe.java | 2 + api/src/org/labkey/api/util/XmlBeansUtil.java | 74 ++++++++++++++++--- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/util/ExternalReferenceProbe.java b/api/src/org/labkey/api/util/ExternalReferenceProbe.java index 2e145b09142..bf9881890bd 100644 --- a/api/src/org/labkey/api/util/ExternalReferenceProbe.java +++ b/api/src/org/labkey/api/util/ExternalReferenceProbe.java @@ -39,6 +39,8 @@ * *

The probe only sees references it hosts, so a test proving "nothing external at all" must point every reference * in its fixture at {@link #url}. Not thread-safe across tests: start and close one per test. + * + *

Test-only, but here so that other modules can use it for their own testing. */ public class ExternalReferenceProbe implements AutoCloseable { diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index 66f01aa3612..19a0fe2f2ce 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -226,7 +226,11 @@ private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocTyp return result; } - /** A {@link SchemaFactory} hardened against XXE (CWE-611). Not thread-safe, so a fresh instance per call. */ + /** + * A {@link SchemaFactory} hardened against XXE (CWE-611). Not thread-safe, so get a fresh instance per call. + * The schema document itself must still be trusted: local file: and jar: references stay enabled so bundled schemas + * can import their sibling + */ public static SchemaFactory schemaFactory() { //noinspection SchemaFactory @@ -301,8 +305,7 @@ private static boolean isLocal(@Nullable String systemId, @Nullable String baseU URI uri = URI.create(systemId); if (!uri.isAbsolute() && baseURI != null) uri = URI.create(baseURI).resolve(uri); - String scheme = uri.getScheme(); - return scheme == null || "file".equalsIgnoreCase(scheme) || "jar".equalsIgnoreCase(scheme); + return isLocalScheme(uri); } catch (IllegalArgumentException e) { @@ -310,6 +313,33 @@ private static boolean isLocal(@Nullable String systemId, @Nullable String baseU } } + private static boolean isLocalScheme(URI uri) + { + String scheme = uri.getScheme(); + + if (scheme == null) + return true; + + // A protocol-relative "//host/x.xsd" resolves against a file: base into file://host/x.xsd, which Windows maps + // to a UNC path and fetches over SMB. Java emits file:/path and file:///path, both of which parse to a null + // authority, so requiring that costs nothing. + if ("file".equalsIgnoreCase(scheme)) + { + String authority = uri.getAuthority(); + return authority == null || "localhost".equalsIgnoreCase(authority); + } + + // jar:!/ is opaque, so the scheme above says nothing about what gets fetched -- the inner URL does + if ("jar".equalsIgnoreCase(scheme)) + { + String ssp = uri.getSchemeSpecificPart(); + int separator = ssp.indexOf("!/"); + return isLocalScheme(URI.create(separator < 0 ? ssp : ssp.substring(0, separator))); + } + + return false; + } + @FunctionalInterface private interface XmlSetting { @@ -426,18 +456,30 @@ public void schemaFactoryRefusesRemoteImport() throws Exception { try (ExternalReferenceProbe probe = ExternalReferenceProbe.start()) { - String remote = probe.url("/remote.xsd", - ""); - String xsd = "" + - "" + - "" + - ""; - compileIgnoringErrors(schemaFactory(), new StreamSource(new StringReader(xsd))); + compileIgnoringErrors(schemaFactory(), new StreamSource(new StringReader(remoteImportSchema(probe)))); probe.assertNotContacted("schemaFactory() must not fetch a remote xs:import"); } } + /** + * Harness self-check for the two tests above, mirroring {@link #probeDetectsFetchWhenValidatorIsNotHardened}: + * a compile that aborts before it ever walks the import graph satisfies assertNotContacted while proving nothing. + */ + @Test + public void probeDetectsFetchWhenSchemaFactoryIsNotHardened() throws Exception + { + try (ExternalReferenceProbe probe = ExternalReferenceProbe.start()) + { + //noinspection SchemaFactory + SchemaFactory raw = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + compileIgnoringErrors(raw, new StreamSource(new StringReader(remoteImportSchema(probe)))); + + assertTrue("Probe must observe a fetch from an unhardened factory, otherwise the schemaFactory() " + + "tests in this class prove nothing", probe.wasContacted()); + } + } + // ---------- over-blocking guards ---------- // Bundled schemas compose sibling XSDs by relative path, from a jar: URL when deployed and a file: URL in a dev build. @@ -519,6 +561,18 @@ private void validateIgnoringErrors(Validator validator, String xml) throws IOEx } } + /** A schema whose only external reference is a remote xs:import pointed at the probe. */ + private String remoteImportSchema(ExternalReferenceProbe probe) + { + String remote = probe.url("/remote.xsd", + ""); + + return "" + + "" + + "" + + ""; + } + /** Likewise: a refused reference may or may not surface as a compile error. */ private void compileIgnoringErrors(SchemaFactory factory, StreamSource source) {