diff --git a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java index af7eeabedbf..61c3f6f7b10 100644 --- a/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java +++ b/modules/flowable-engine/src/main/java/org/flowable/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java @@ -22,8 +22,10 @@ import java.util.TreeMap; import javax.imageio.ImageIO; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathExpression; import javax.xml.xpath.XPathExpressionException; @@ -103,6 +105,20 @@ protected Document parseXml(InputStream bpmnXmlStream) { // Get one that understands namespaces factory.setNamespaceAware(true); + try { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + } catch (ParserConfigurationException e) { + throw new FlowableException("Error configuring XML parser for safe BPMN parsing", e); + } + DocumentBuilder builder; Document bpmnModel; try { diff --git a/modules/flowable-engine/src/test/java/org/flowable/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactoryTest.java b/modules/flowable-engine/src/test/java/org/flowable/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactoryTest.java new file mode 100644 index 00000000000..15095f9319e --- /dev/null +++ b/modules/flowable-engine/src/test/java/org/flowable/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactoryTest.java @@ -0,0 +1,79 @@ +/* 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.flowable.engine.impl.bpmn.diagram; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.flowable.common.engine.api.FlowableException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.w3c.dom.Document; + +public class ProcessDiagramLayoutFactoryTest { + + private final ProcessDiagramLayoutFactory factory = new ProcessDiagramLayoutFactory(); + + @Test + public void testExternalEntityInElementTextIsRejected(@TempDir Path tempDir) throws Exception { + // The entity must be referenced in element text content. In an attribute value the JDK + // rejects an external entity reference even when the parser is not hardened, so such a + // test would also pass without the fix. + Path secret = tempDir.resolve("secret.txt"); + Files.writeString(secret, "TOP-SECRET-CONTENT"); + + String maliciousBpmn = """ + + ]> + + &xxe; + + """.formatted(secret.toUri()); + + assertThatThrownBy(() -> factory.parseXml(new ByteArrayInputStream(maliciousBpmn.getBytes(StandardCharsets.UTF_8)))) + .isInstanceOf(FlowableException.class) + .hasMessageContaining("Error while parsing BPMN model"); + } + + @Test + public void testDoctypeIsRejected() { + String bpmnWithDoctype = """ + + ]> + + """; + + assertThatThrownBy(() -> factory.parseXml(new ByteArrayInputStream(bpmnWithDoctype.getBytes(StandardCharsets.UTF_8)))) + .isInstanceOf(FlowableException.class); + } + + @Test + public void testValidBpmnStillParses() { + String validBpmn = """ + + + + + """; + + Document document = factory.parseXml(new ByteArrayInputStream(validBpmn.getBytes(StandardCharsets.UTF_8))); + + assertThat(document.getDocumentElement().getLocalName()).isEqualTo("definitions"); + } +} diff --git a/modules/flowable5-engine/src/main/java/org/activiti/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java b/modules/flowable5-engine/src/main/java/org/activiti/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java index ee07a057ddf..f613ea24aef 100644 --- a/modules/flowable5-engine/src/main/java/org/activiti/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java +++ b/modules/flowable5-engine/src/main/java/org/activiti/engine/impl/bpmn/diagram/ProcessDiagramLayoutFactory.java @@ -22,8 +22,10 @@ import java.util.TreeMap; import javax.imageio.ImageIO; +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathExpression; import javax.xml.xpath.XPathExpressionException; @@ -98,6 +100,20 @@ protected Document parseXml(InputStream bpmnXmlStream) { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); // Get one that understands namespaces factory.setNamespaceAware(true); + + try { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + } catch (ParserConfigurationException e) { + throw new FlowableException("Error configuring XML parser for safe BPMN parsing", e); + } DocumentBuilder builder; Document bpmnModel;