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 0f79761fafa..ecb2ec0ad0d 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.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 b0328fb4e25..ccf95acad61 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 aa6b64467e1..538f442f9b6 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.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 @@ 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 DOMWalker(InputStream xml) throws IOException { 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("")); - 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 00000000000..925e379562f --- /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 28b80f00b30..ff032b7bc45 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 00000000000..519b9328601 --- /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 = + "\n" + + " ]>\n" + + "&xxe;"; + + @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 bc241491296..6da6ab7e55f 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.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.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 @@ private static String createLicenseHeader() { /** * Constant for DocumentBuilderFactory. */ - 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 TransformerFactory @@ -306,9 +273,7 @@ private PrivilegeDefinition parseDefinition(Node n, Map namespac 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("")); - builder.setEntityResolver(stopMe); + builder.setEntityResolver(XMLFactories.nonResolvingEntityResolver()); builder.setErrorHandler(new DefaultHandler()); return builder; }