You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by re...@apache.org on 2026/01/10 12:01:42 UTC
(jackrabbit) branch 2.22 updated: JCR-5195: Utilities for 'safe' creation of XML document builders (#298)
This is an automated email from the ASF dual-hosted git repository.
reschke pushed a commit to branch 2.22
in repository https://gitbox.apache.org/repos/asf/jackrabbit.git
The following commit(s) were added to refs/heads/2.22 by this push:
new b402b4f7b JCR-5195: Utilities for 'safe' creation of XML document builders (#298)
b402b4f7b is described below
commit b402b4f7b5726532653b5cb90b6acd5184d42345
Author: Julian Reschke <re...@apache.org>
AuthorDate: Tue Oct 7 15:45:45 2025 +0200
JCR-5195: Utilities for 'safe' creation of XML document builders (#298)
---
.../jackrabbit/core/query/lucene/SearchIndex.java | 1 -
.../apache/jackrabbit/core/util/DOMBuilder.java | 3 +-
.../org/apache/jackrabbit/core/util/DOMWalker.java | 40 +----------
.../jackrabbit/commons/xml/XMLFactories.java | 77 ++++++++++++++++++++
.../jackrabbit/commons/xml/package-info.java | 2 +-
.../apache/jackrabbit/xml/XMLFactoriesTest.java | 84 ++++++++++++++++++++++
.../spi/commons/privilege/PrivilegeXmlHandler.java | 41 +----------
7 files changed, 170 insertions(+), 78 deletions(-)
diff --git a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
index 8e4db22cc..fe9dfbc9f 100644
--- a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
+++ b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/SearchIndex.java
@@ -92,7 +92,6 @@ import org.apache.lucene.document.Field;
import org.apache.lucene.document.Fieldable;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.MultiReader;
-import org.apache.lucene.index.Payload;
import org.apache.lucene.index.Term;
import org.apache.lucene.index.TermDocs;
import org.apache.lucene.search.IndexSearcher;
diff --git a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMBuilder.java b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMBuilder.java
index b0328fb4e..ccf95acad 100644
--- a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMBuilder.java
+++ b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMBuilder.java
@@ -16,6 +16,7 @@
*/
package org.apache.jackrabbit.core.util;
+import org.apache.jackrabbit.commons.xml.XMLFactories;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -39,7 +40,7 @@ public final class DOMBuilder {
/** Static factory for creating DOM DocumentBuilder instances. */
private static final DocumentBuilderFactory BUILDER_FACTORY =
- DocumentBuilderFactory.newInstance();
+ XMLFactories.safeDocumentBuilderFactory();
/** Static factory for creating document to output stream transformers. */
private static final TransformerFactory TRANSFORMER_FACTORY =
diff --git a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMWalker.java b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMWalker.java
index aa6b64467..538f442f9 100644
--- a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMWalker.java
+++ b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/DOMWalker.java
@@ -16,6 +16,7 @@
*/
package org.apache.jackrabbit.core.util;
+import org.apache.jackrabbit.commons.xml.XMLFactories;
import org.w3c.dom.Attr;
import org.w3c.dom.CharacterData;
import org.w3c.dom.Document;
@@ -23,15 +24,11 @@ import org.w3c.dom.Element;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
-import org.xml.sax.EntityResolver;
-import org.xml.sax.InputSource;
-import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import java.io.IOException;
import java.io.InputStream;
-import java.io.StringReader;
import java.util.Properties;
/**
@@ -41,36 +38,7 @@ import java.util.Properties;
public final class DOMWalker {
/** Static factory for creating stream to DOM transformers. */
- private static final DocumentBuilderFactory factory = createFactory();
-
- private static DocumentBuilderFactory createFactory() {
- DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
- factory.setIgnoringComments(false);
- factory.setIgnoringElementContentWhitespace(true);
- factory.setXIncludeAware(false);
-
- // Prevent XXE attacks by disabling external entity processing
- factory.setExpandEntityReferences(false);
-
- String feature = null;
-
- try {
- feature = XMLConstants.FEATURE_SECURE_PROCESSING;
- factory.setFeature(feature, true);
- feature = "http://apache.org/xml/features/disallow-doctype-decl";
- factory.setFeature(feature, true);
- feature = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
- factory.setFeature(feature, false);
- feature = "http://xml.org/sax/features/external-general-entities";
- factory.setFeature(feature, false);
- feature = "http://xml.org/sax/features/external-parameter-entities";
- factory.setFeature(feature, false);
- } catch (Exception ex) {
- // abort if secure processing is not supported
- throw new IllegalStateException("Secure processing feature '" + feature + "' not supported by the DocumentBuilderFactory: " + factory.getClass().getName(), ex);
- }
- return factory;
- }
+ private static final DocumentBuilderFactory factory = XMLFactories.safeDocumentBuilderFactory();
/** The DOM document being traversed by this walker. */
private final Document document;
@@ -90,9 +58,7 @@ public final class DOMWalker {
try {
DocumentBuilder builder = factory.newDocumentBuilder();
// defense in depth: entity resolver that will break any document on purpose
- EntityResolver stopMe = (publicId, systemId) -> new InputSource(
- new StringReader("<preventing read of: " + publicId + " " + systemId + ">"));
- builder.setEntityResolver(stopMe);
+ builder.setEntityResolver(XMLFactories.nonResolvingEntityResolver());
document = builder.parse(xml);
current = document.getDocumentElement();
} catch (IOException e) {
diff --git a/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/xml/XMLFactories.java b/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/xml/XMLFactories.java
new file mode 100755
index 000000000..925e37956
--- /dev/null
+++ b/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/xml/XMLFactories.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.jackrabbit.commons.xml;
+
+import org.xml.sax.EntityResolver;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilderFactory;
+import java.io.IOException;
+
+/**
+ * Factory for "safe" instances of XML parsers (wrt XXE etc.).
+ */
+public class XMLFactories {
+
+ private XMLFactories() {
+ }
+
+ /**
+ * @return a "safe" {@link DocumentBuilderFactory}
+ */
+ public static DocumentBuilderFactory safeDocumentBuilderFactory() {
+ // see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
+
+ javax.xml.parsers.DocumentBuilderFactory factory = javax.xml.parsers.DocumentBuilderFactory.newInstance();
+
+ factory.setIgnoringComments(false);
+ factory.setIgnoringElementContentWhitespace(true);
+ factory.setXIncludeAware(false);
+
+ // Prevent XXE attacks by disabling external entity processing
+ factory.setExpandEntityReferences(false);
+
+ String feature = null;
+
+ try {
+ feature = XMLConstants.FEATURE_SECURE_PROCESSING;
+ factory.setFeature(feature, true);
+ feature = "http://apache.org/xml/features/disallow-doctype-decl";
+ factory.setFeature(feature, true);
+ feature = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
+ factory.setFeature(feature, false);
+ feature = "http://xml.org/sax/features/external-general-entities";
+ factory.setFeature(feature, false);
+ feature = "http://xml.org/sax/features/external-parameter-entities";
+ factory.setFeature(feature, false);
+ } catch (Exception ex) {
+ // abort if secure processing is not supported
+ throw new IllegalStateException("Secure processing feature '" + feature + "' not supported by the DocumentBuilderFactory: " + factory.getClass().getName(), ex);
+ }
+ return factory;
+ }
+
+ /**
+ * @return an {@link EntityResolver} which will always cause a parse exception.
+ */
+ public static EntityResolver nonResolvingEntityResolver() {
+ return (publicId, systemId) -> {
+ throw new IOException("This parser does not support resolution of external entities (publicId: " + publicId
+ + ", systemId: " + systemId + ")");
+ };
+ }
+}
diff --git a/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/xml/package-info.java b/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/xml/package-info.java
index 28b80f00b..ff032b7bc 100644
--- a/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/xml/package-info.java
+++ b/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/xml/package-info.java
@@ -14,5 +14,5 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-@org.osgi.annotation.versioning.Version("2.2")
+@org.osgi.annotation.versioning.Version("2.3")
package org.apache.jackrabbit.commons.xml;
diff --git a/jackrabbit-jcr-commons/src/test/java/org/apache/jackrabbit/xml/XMLFactoriesTest.java b/jackrabbit-jcr-commons/src/test/java/org/apache/jackrabbit/xml/XMLFactoriesTest.java
new file mode 100755
index 000000000..519b93286
--- /dev/null
+++ b/jackrabbit-jcr-commons/src/test/java/org/apache/jackrabbit/xml/XMLFactoriesTest.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.jackrabbit.xml;
+
+import org.apache.jackrabbit.commons.xml.XMLFactories;
+import org.junit.Test;
+import org.w3c.dom.Document;
+import org.xml.sax.EntityResolver;
+import org.xml.sax.SAXParseException;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import java.io.ByteArrayInputStream;
+import java.io.FileNotFoundException;
+import java.util.UUID;
+
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+public class XMLFactoriesTest {
+
+ String randomName = UUID.randomUUID().toString();
+
+ String xxeXml =
+ "<!DOCTYPE root [\n" +
+ " <!ELEMENT root ANY >\n" +
+ " <!ENTITY xxe SYSTEM \"file:///tmp/" + randomName + "\" >]>\n" +
+ "<root>&xxe;</root>";
+
+ @Test
+ // verify that default builder is vulnerable
+ public void testDefaultParserDoesResolveExternalEntities() throws Exception {
+ DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+
+ DocumentBuilder db = dbf.newDocumentBuilder();
+ Document doc = null;
+
+ try (ByteArrayInputStream input = new ByteArrayInputStream(xxeXml.getBytes())) {
+ doc = db.parse(input);
+ } catch (FileNotFoundException expected) {
+ assertTrue("FileNotFoundException message should contain file name '"
+ + randomName + "', but was: '" + expected.getMessage() + "'", expected.getMessage().contains(randomName));
+ }
+
+ assertNull("There should be no doc: " + doc, doc);
+ }
+
+ @Test
+ // verify that utility builder is not vulnerable
+ public void testParserDoesNotResolveExternalEntities() throws Exception {
+ DocumentBuilderFactory dbf = XMLFactories.safeDocumentBuilderFactory();
+
+ DocumentBuilder db = dbf.newDocumentBuilder();
+
+ try (ByteArrayInputStream input = new ByteArrayInputStream(xxeXml.getBytes())) {
+ db.parse(input);
+ } catch (SAXParseException expected) {
+ // all good
+ }
+ }
+
+ @Test
+ public void testSafeEntityResolver() {
+ EntityResolver test = XMLFactories.nonResolvingEntityResolver();
+ assertThrows(Exception.class, () -> test.resolveEntity("-//The Apache Software Foundation//DTD Jackrabbit 1.6//EN" ,
+ "http://jackrabbit.apache.org/dtd/repository-1.6.dtd"));
+ }
+}
diff --git a/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/privilege/PrivilegeXmlHandler.java b/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/privilege/PrivilegeXmlHandler.java
index bc2414912..6da6ab7e5 100644
--- a/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/privilege/PrivilegeXmlHandler.java
+++ b/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/privilege/PrivilegeXmlHandler.java
@@ -16,6 +16,7 @@
*/
package org.apache.jackrabbit.spi.commons.privilege;
+import org.apache.jackrabbit.commons.xml.XMLFactories;
import org.apache.jackrabbit.spi.Name;
import org.apache.jackrabbit.spi.NameFactory;
import org.apache.jackrabbit.spi.PrivilegeDefinition;
@@ -27,12 +28,10 @@ import org.w3c.dom.Element;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
-import org.xml.sax.EntityResolver;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import org.xml.sax.helpers.DefaultHandler;
-import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
@@ -46,9 +45,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.Reader;
-import java.io.StringReader;
import java.io.Writer;
-import java.rmi.server.ExportException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
@@ -109,37 +106,7 @@ class PrivilegeXmlHandler implements PrivilegeHandler {
/**
* Constant for <code>DocumentBuilderFactory</code>.
*/
- private static DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY = createFactory();
-
- private static DocumentBuilderFactory createFactory() {
- DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
- factory.setNamespaceAware(true);
- factory.setIgnoringComments(false);
- factory.setIgnoringElementContentWhitespace(true);
- factory.setXIncludeAware(false);
-
- // Prevent XXE attacks by disabling external entity processing
- factory.setExpandEntityReferences(false);
-
- String feature = null;
-
- try {
- feature = XMLConstants.FEATURE_SECURE_PROCESSING;
- factory.setFeature(feature, true);
- feature = "http://apache.org/xml/features/disallow-doctype-decl";
- factory.setFeature(feature, true);
- feature = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
- factory.setFeature(feature, false);
- feature = "http://xml.org/sax/features/external-general-entities";
- factory.setFeature(feature, false);
- feature = "http://xml.org/sax/features/external-parameter-entities";
- factory.setFeature(feature, false);
- } catch (Exception ex) {
- // abort if secure processing is not supported
- throw new IllegalStateException("Secure processing feature '" + feature + "' not supported by the DocumentBuilderFactory: " + factory.getClass().getName(), ex);
- }
- return factory;
- }
+ private static DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY = XMLFactories.safeDocumentBuilderFactory();
/**
* Constant for <code>TransformerFactory</code>
@@ -306,9 +273,7 @@ class PrivilegeXmlHandler implements PrivilegeHandler {
private static DocumentBuilder createDocumentBuilder() throws ParserConfigurationException {
DocumentBuilder builder = DOCUMENT_BUILDER_FACTORY.newDocumentBuilder();
// defense in depth: entity resolver that will break any document on purpose
- EntityResolver stopMe = (publicId, systemId) -> new InputSource(
- new StringReader("<preventing read of: " + publicId + " " + systemId + ">"));
- builder.setEntityResolver(stopMe);
+ builder.setEntityResolver(XMLFactories.nonResolvingEntityResolver());
builder.setErrorHandler(new DefaultHandler());
return builder;
}