diff --git a/core/src/main/java/apoc/export/graphml/XmlGraphMLReader.java b/core/src/main/java/apoc/export/graphml/XmlGraphMLReader.java index eb8e35cf96..ed16046f5c 100644 --- a/core/src/main/java/apoc/export/graphml/XmlGraphMLReader.java +++ b/core/src/main/java/apoc/export/graphml/XmlGraphMLReader.java @@ -206,6 +206,8 @@ public long parseXML(Reader input) throws XMLStreamException { XMLInputFactory inputFactory = XMLInputFactory.newInstance(); inputFactory.setProperty("javax.xml.stream.isCoalescing", true); inputFactory.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, true); + inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); XMLEventReader reader = inputFactory.createXMLEventReader(input); Entity last = null; Map nodeKeys = new HashMap<>(); @@ -218,10 +220,17 @@ public long parseXML(Reader input) throws XMLStreamException { XMLEvent event; try { event = (XMLEvent) reader.next(); + if ( event.getEventType() == XMLStreamConstants.DTD) + { + generateXmlDoctypeException(); + } } catch (Exception e) { // in case of unicode invalid chars we skip the event, or we exit in case of EOF if (e.getMessage().contains("Unexpected EOF")) { break; + } else if (e.getMessage().contains("DOCTYPE")) + { + throw e; } continue; } @@ -416,4 +425,8 @@ private String getAttribute(StartElement element, QName qname) { Attribute attribute = element.getAttributeByName(qname); return attribute != null ? attribute.getValue() : null; } + + private RuntimeException generateXmlDoctypeException() { + throw new RuntimeException("XML documents with a DOCTYPE are not allowed."); + } } diff --git a/core/src/test/java/apoc/export/graphml/ExportGraphMLTest.java b/core/src/test/java/apoc/export/graphml/ExportGraphMLTest.java index f5e1cde3c8..d3b22801b1 100644 --- a/core/src/test/java/apoc/export/graphml/ExportGraphMLTest.java +++ b/core/src/test/java/apoc/export/graphml/ExportGraphMLTest.java @@ -49,10 +49,12 @@ import static apoc.util.BinaryTestUtil.fileToBinary; import static apoc.util.MapUtil.map; import static apoc.util.TestUtil.isRunningInCI; +import static apoc.util.TestUtil.testResult; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeFalse; import static org.neo4j.configuration.GraphDatabaseSettings.TransactionStateMemoryAllocation.OFF_HEAP; @@ -714,6 +716,34 @@ public void testExportGraphmlQueryWithStringCaptionCamelCase() { assertXMLEquals(output, EXPECTED_TYPES_PATH_CAMEL_CASE); } + @Test + public void testImportGraphmlPreventXXEVulnerabilityThrowsQueryExecutionException() { + QueryExecutionException e = assertThrows(QueryExecutionException.class, + () -> testResult(db, "CALL apoc.import.graphml('" + TestUtil.getUrlFileName("xml/xxe.xml") + "', {})", (r) -> { + r.next(); + r.close(); + }) + ); + + Throwable except = ExceptionUtils.getRootCause(e); + assertTrue(except instanceof RuntimeException); + assertEquals(except.getMessage(), "XML documents with a DOCTYPE are not allowed."); + } + + @Test + public void testImportGraphmlPreventBillionLaughVulnerabilityThrowsQueryExecutionException() { + QueryExecutionException e = assertThrows(QueryExecutionException.class, + () -> testResult(db, "CALL apoc.import.graphml('" + TestUtil.getUrlFileName("xml/billion_laughs.xml") + "', {})", (r) -> { + r.next(); + r.close(); + }) + ); + + Throwable except = ExceptionUtils.getRootCause(e); + assertTrue(except instanceof RuntimeException); + assertEquals(except.getMessage(), "XML documents with a DOCTYPE are not allowed."); + } + private void assertResults(File output, Map r, final String source) { assertCommons(r); assertEquals(output.getAbsolutePath(), r.get("file"));