This is an automated email from the ASF dual-hosted git repository.
cgivre pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push:
new 0e88b7a510 DRILL-8461: Prevent XXE Attacks in XML Format Plugin (#2845)
0e88b7a510 is described below
commit 0e88b7a5101d24c561a2a3efb12d7a3b3f7933f3
Author: Charles S. Givre <[email protected]>
AuthorDate: Sun Nov 5 22:35:14 2023 -0500
DRILL-8461: Prevent XXE Attacks in XML Format Plugin (#2845)
---
.../org/apache/drill/exec/store/xml/XMLReader.java | 3 +++
.../apache/drill/exec/store/xml/TestXMLReader.java | 14 +++++++++++
contrib/format-xml/src/test/resources/xml/bad.xml | 29 ++++++++++++++++++++++
3 files changed, 46 insertions(+)
diff --git
a/contrib/format-xml/src/main/java/org/apache/drill/exec/store/xml/XMLReader.java
b/contrib/format-xml/src/main/java/org/apache/drill/exec/store/xml/XMLReader.java
index 8d1fb59ff7..dbb8f459ec 100644
---
a/contrib/format-xml/src/main/java/org/apache/drill/exec/store/xml/XMLReader.java
+++
b/contrib/format-xml/src/main/java/org/apache/drill/exec/store/xml/XMLReader.java
@@ -100,6 +100,9 @@ public class XMLReader implements Closeable {
public XMLReader(InputStream fsStream, int dataLevel, boolean allTextMode)
throws XMLStreamException {
this.fsStream = fsStream;
XMLInputFactory inputFactory = XMLInputFactory.newInstance();
+
+ // This property prevents XXE attacks by disallowing DTD.
+ inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
reader = inputFactory.createXMLEventReader(fsStream);
fieldNameStack = new Stack<>();
rowWriterStack = new Stack<>();
diff --git
a/contrib/format-xml/src/test/java/org/apache/drill/exec/store/xml/TestXMLReader.java
b/contrib/format-xml/src/test/java/org/apache/drill/exec/store/xml/TestXMLReader.java
index a0d3ac913e..ca3e4bd58f 100644
---
a/contrib/format-xml/src/test/java/org/apache/drill/exec/store/xml/TestXMLReader.java
+++
b/contrib/format-xml/src/test/java/org/apache/drill/exec/store/xml/TestXMLReader.java
@@ -19,6 +19,7 @@
package org.apache.drill.exec.store.xml;
import org.apache.drill.categories.RowSetTest;
+import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.common.types.TypeProtos.DataMode;
import org.apache.drill.common.types.TypeProtos.MinorType;
import org.apache.drill.exec.physical.rowSet.RowSet;
@@ -41,6 +42,8 @@ import static
org.apache.drill.test.rowSet.RowSetUtilities.mapArray;
import static org.apache.drill.test.rowSet.RowSetUtilities.objArray;
import static org.apache.drill.test.rowSet.RowSetUtilities.strArray;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
@Category(RowSetTest.class)
public class TestXMLReader extends ClusterTest {
@@ -86,6 +89,17 @@ public class TestXMLReader extends ClusterTest {
new RowSetComparison(expected).verifyAndClearAll(results);
}
+ @Test
+ public void testXXE() throws Exception {
+ String sql = "SELECT * FROM cp.`xml/bad.xml`";
+ try {
+ client.queryBuilder().sql(sql).rowSet();
+ fail();
+ } catch (UserException e) {
+ assertTrue(e.getMessage().contains("DATA_READ ERROR: Error parsing XML
file"));
+ }
+ }
+
@Test
public void testAllTextMode() throws Exception {
String sql = "SELECT attributes, int_field, bigint_field, float_field,
double_field, " +
diff --git a/contrib/format-xml/src/test/resources/xml/bad.xml
b/contrib/format-xml/src/test/resources/xml/bad.xml
new file mode 100644
index 0000000000..94625f362f
--- /dev/null
+++ b/contrib/format-xml/src/test/resources/xml/bad.xml
@@ -0,0 +1,29 @@
+<!--
+
+ 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.
+
+-->
+
+<?xml version="1.0" encoding="ISO-8859-1"?>
+<!DOCTYPE foo [
+ <!ELEMENT foo ANY >
+ <!ENTITY xxe SYSTEM "/etc/passwd" >]
+ >
+<creds>
+ <user>&xxe;</user>
+ <pass>mypass</pass>
+</creds>
\ No newline at end of file