CVE-2018-1000632: XML Injection (aka Blind XPath Injection) (CWE-91)
value: dom4j version prior to version 2.1.1 contains a CWE-91: XML Injection vulnerability in Class: Element. Methods: addElement, addAttribute that can result in an attacker tampering with XML documents through XML injection. This attack appear to be exploitable via an attacker specifying attributes or elements in the XML document. This vulnerability appears to have been fixed in 2.1.1 or later.
CWE description: The product does not properly neutralize special elements that are used in XML, allowing attackers to modify the syntax, content, or commands of the XML before it is processed by an end system.
CVSS v3 base score: 7.5
Published: 2018-08-20T19:31Z
Repository: https://github.com/dom4j/dom4j
Language: Java
Files changed: src/main/java/org/dom4j/Namespace.java, src/main/java/org/dom4j/QName.java, src/main/java/org/dom4j/tree/QNameCache.java, src/test/java/org/dom4j/AllowedCharsTest.java
Commit date: 2018-07-01 11:27:15 +0200
Commit message: #48 Validate QName inputs - throw IllegalArgumentException when qualified name contains disallowed character.

Diff: diff --git a/src/main/java/org/dom4j/Namespace.java b/src/main/java/org/dom4j/Namespace.java
index fd123b9..8f948ad 100644
--- a/src/main/java/org/dom4j/Namespace.java
+++ b/src/main/java/org/dom4j/Namespace.java
@@ -47,10 +47,14 @@ public class Namespace extends AbstractNode {
      *            is the URI for this namespace
      */
     public Namespace(String prefix, String uri) {
         this.prefix = (prefix != null) ? prefix : "";
         this.uri = (uri != null) ? uri : "";
+
+        if (!this.prefix.isEmpty()) {
+            QName.validateNCName(this.prefix);
+        }
     }
 
     /**
      * A helper method to return the Namespace instance for the given prefix and
      * URI
diff --git a/src/main/java/org/dom4j/QName.java b/src/main/java/org/dom4j/QName.java
index 9ac0d4d..e9b2170 100644
--- a/src/main/java/org/dom4j/QName.java
+++ b/src/main/java/org/dom4j/QName.java
@@ -9,25 +9,101 @@ package org.dom4j;
 
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
+import java.util.regex.Pattern;
 
 import org.dom4j.tree.QNameCache;
 import org.dom4j.util.SingletonStrategy;
 
 /**
  * <code>QName</code> represents a qualified name value of an XML element or
  * attribute. It consists of a local name and a {@link Namespace}instance. This
  * object is immutable.
  *
  * @author <a href="mailto:jstrachan@apache.org">James Strachan </a>
+ * @author Filip Jirsák
  */
 public class QName implements Serializable {
     /** The Singleton instance */
     private static SingletonStrategy<QNameCache> singleton = null;
 
+    /**
+     * {@code NameStartChar} without colon.
+     *
+     * <pre>NameStartChar	::=	":" | [A-Z] | "_" | [a-z] | [#xC0-#xD6] | [#xD8-#xF6] | [#xF8-#x2FF] | [#x370-#x37D] | [#x37F-#x1FFF] | [#x200C-#x200D] | [#x2070-#x218F] | [#x2C00-#x2FEF] | [#x3001-#xD7FF] | [#xF900-#xFDCF] | [#xFDF0-#xFFFD] | [#x10000-#xEFFFF]</pre>
+     *
+     * @see <a href="https://www.w3.org/TR/xml/#sec-common-syn">XML 1.0 – 2.3 Common Syntactic Constructs</a>
+     * @see <a href="https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-common-syn">XML 1.1 – 2.3 Common Syntactic Constructs</a>
+     */
+    private static final String NAME_START_CHAR = "_A-Za-z\u00C0-\u00D6\u00D8-\u00F6\u00F8-\u02FF\u0370-\u037D\u037F-\u1FFF\u200C-\u200D\u2070-\u218F\u2C00-\u2FEF\u3001-\uD7FF\uF900-\uFDCF\uFDF0-\uFFFD";
+
+    /**
+     * {@code NameChar} without colon.
+     *
+     * <pre>NameChar	::=	NameStartChar | "-" | "." | [0-9] | #xB7 | [#x0300-#x036F] | [#x203F-#x2040]</pre>
+     *
+     * @see <a href="https://www.w3.org/TR/xml/#sec-common-syn">XML 1.0 – 2.3 Common Syntactic Constructs</a>
+     * @see <a href="https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-common-syn">XML 1.1 – 2.3 Common Syntactic Constructs</a>
+     */
+    private static final String NAME_CHAR = NAME_START_CHAR + "-.0-9\u00B7\u0300-\u036F\u203F-\u2040";
+
+    /**
+     * {@code NCName}
+     *
+     * <pre>
+     * NCName		::=	NCNameStartChar NCNameChar*	(An XML Name, minus the ":")
+     * NCNameChar	::=	NameChar -':'
+     * NCNameStartChar	::=	NameStartChar -':'
+     * </pre>
+     *
+     * @see <a href="https://www.w3.org/TR/xml-names/#ns-qualnames">Namespaces in XML 1.0 – 4 Qualified Names</a>
+     * @see <a href="https://www.w3.org/TR/2006/REC-xml-names11-20060816/#ns-qualnames">Namespaces in XML 1.1 – 4 Qualified Names</a>
+     */
+    private static final String NCNAME = "["+NAME_START_CHAR+"]["+NAME_CHAR+"]*";
+
+    /**
+     * Regular expression for {@code Name} (with colon).
+     *
+     * <pre>Name	::=	NameStartChar (NameChar)*</pre>
+     *
+     * @see <a href="https://www.w3.org/TR/xml/#sec-common-syn">XML 1.0 – 2.3 Common Syntactic Constructs</a>
+     * @see <a href="https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-common-syn">XML 1.1 – 2.3 Common Syntactic Constructs</a>
+     */
+    private static final Pattern RE_NAME = Pattern.compile("[:"+NAME_START_CHAR+"][:"+NAME_CHAR+"]*");
+
+    /**
+     * Regular expression for {@code NCName}.
+     *
+     * <pre>
+     * NCName		::=	NCNameStartChar NCNameChar*	(An XML Name, minus the ":")
+     * NCNameChar	::=	NameChar -':'
+     * NCNameStartChar	::=	NameStartChar -':'
+     * </pre>
+     *
+     * @see <a href="https://www.w3.org/TR/xml-names/#ns-qualnames">Namespaces in XML 1.0 – 4 Qualified Names</a>
+     * @see <a href="https://www.w3.org/TR/2006/REC-xml-names11-20060816/#ns-qualnames">Namespaces in XML 1.1 – 4 Qualified Names</a>
+     */
+    private static final Pattern RE_NCNAME = Pattern.compile(NCNAME);
+
+    /**
+     * Regular expression for {@code QName}.
+     *
+     * <pre>
+     * QName		::=	PrefixedName | UnprefixedName
+     * PrefixedName	::=	Prefix ':' LocalPart
+     * UnprefixedName	::=	LocalPart
+     * Prefix		::=	NCName
+     * LocalPart	::=	NCName
+     * </pre>
+     *
+     * @see <a href="https://www.w3.org/TR/xml-names/#ns-qualnames">Namespaces in XML 1.0 – 4 Qualified Names</a>
+     * @see <a href="https://www.w3.org/TR/2006/REC-xml-names11-20060816/#ns-qualnames">Namespaces in XML 1.1 – 4 Qualified Names</a>
+     */
+    private static final Pattern RE_QNAME = Pattern.compile("(?:"+NCNAME+":)?"+NCNAME);
+
     static {
         try {
             String defaultSingletonClass = "org.dom4j.util.SimpleSingleton";
             Class<SingletonStrategy> clazz = null;
             try {
@@ -69,17 +145,24 @@ public class QName implements Serializable {
 
     public QName(String name, Namespace namespace) {
         this.name = (name == null) ? "" : name;
         this.namespace = (namespace == null) ? Namespace.NO_NAMESPACE
                 : namespace;
+        if (this.namespace.equals(Namespace.NO_NAMESPACE)) {
+            validateName(this.name);
+        } else {
+            validateNCName(this.name);
+        }
     }
 
     public QName(String name, Namespace namespace, String qualifiedName) {
         this.name = (name == null) ? "" : name;
         this.qualifiedName = qualifiedName;
         this.namespace = (namespace == null) ? Namespace.NO_NAMESPACE
                 : namespace;
+        validateNCName(this.name);
+        validateQName(this.qualifiedName);
     }
 
     public static QName get(String name) {
         return getCache().get(name);
     }
@@ -249,10 +332,28 @@ public class QName implements Serializable {
 
     private static QNameCache getCache() {
         QNameCache cache = singleton.instance();
         return cache;
     }
+
+    private static void validateName(String name) {
+        if (!RE_NAME.matcher(name).matches()) {
+            throw new IllegalArgumentException(String.format("Illegal character in name: '%s'.", name));
+        }
+    }
+
+    protected static void validateNCName(String ncname) {
+        if (!RE_NCNAME.matcher(ncname).matches()) {
+            throw new IllegalArgumentException(String.format("Illegal character in local name: '%s'.", ncname));
+        }
+    }
+
+    private static void validateQName(String qname) {
+        if (!RE_QNAME.matcher(qname).matches()) {
+            throw new IllegalArgumentException(String.format("Illegal character in qualified name: '%s'.", qname));
+        }
+    }
 }
 
 
 
 /*
diff --git a/src/main/java/org/dom4j/tree/QNameCache.java b/src/main/java/org/dom4j/tree/QNameCache.java
index 330f379..d37e8aa 100644
--- a/src/main/java/org/dom4j/tree/QNameCache.java
+++ b/src/main/java/org/dom4j/tree/QNameCache.java
@@ -150,10 +150,12 @@ public class QNameCache {
     public QName get(String qualifiedName, String uri) {
         int index = qualifiedName.indexOf(':');
 
         if (index < 0) {
             return get(qualifiedName, Namespace.get(uri));
+        } else if (index == 0){
+            throw new IllegalArgumentException("Qualified name cannot start with ':'.");
         } else {
             String name = qualifiedName.substring(index + 1);
             String prefix = qualifiedName.substring(0, index);
 
             return get(name, Namespace.get(prefix, uri));
diff --git a/src/test/java/org/dom4j/AllowedCharsTest.java b/src/test/java/org/dom4j/AllowedCharsTest.java
new file mode 100644
index 0000000..20c1de0
--- /dev/null
+++ b/src/test/java/org/dom4j/AllowedCharsTest.java
@@ -0,0 +1,78 @@
+package org.dom4j;
+
+import org.testng.annotations.Test;
+
+/**
+ * @author Filip Jirsák
+ */
+public class AllowedCharsTest {
+    @Test
+    public void localName() {
+        QName.get("element");
+        QName.get(":element");
+        QName.get("elem:ent");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void localNameFail() {
+        QName.get("!element");
+    }
+
+    @Test
+    public void qname() {
+        QName.get("element", "http://example.com/namespace");
+        QName.get("ns:element", "http://example.com/namespace");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void qnameFail1() {
+        QName.get("ns:elem:ent", "http://example.com/namespace");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void qnameFail2() {
+        QName.get(":nselement", "http://example.com/namespace");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void createElementLT() {
+        DocumentHelper.createElement("element<name");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void createElementGT() {
+        DocumentHelper.createElement("element>name");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void createElementAmpersand() {
+        DocumentHelper.createElement("element&name");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void addElement() {
+        Element root = DocumentHelper.createElement("root");
+        root.addElement("element>name");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void addElementQualified() {
+        Element root = DocumentHelper.createElement("root");
+        root.addElement("element>name", "http://example.com/namespace");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void addElementQualifiedPrefix() {
+        Element root = DocumentHelper.createElement("root");
+        root.addElement("ns:element>name", "http://example.com/namespace");
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void addElementPrefix() {
+        Element root = DocumentHelper.createElement("root");
+        root.addElement("ns>:element", "http://example.com/namespace");
+    }
+
+    //TODO It is illegal to create element or attribute with namespace prefix and empty namespace IRI.
+    //See https://www.w3.org/TR/2006/REC-xml-names11-20060816/#scoping
+}

Fixed code: 
        if (!this.prefix.isEmpty()) {
            QName.validateNCName(this.prefix);
        }
import java.util.regex.Pattern;
 * @author Filip Jirsák
    /**
     * {@code NameStartChar} without colon.
     *
     * <pre>NameStartChar	::=	":" | [A-Z] | "_" | [a-z] | [#xC0-#xD6] | [#xD8-#xF6] | [#xF8-#x2FF] | [#x370-#x37D] | [#x37F-#x1FFF] | [#x200C-#x200D] | [#x2070-#x218F] | [#x2C00-#x2FEF] | [#x3001-#xD7FF] | [#xF900-#xFDCF] | [#xFDF0-#xFFFD] | [#x10000-#xEFFFF]</pre>
     *
     * @see <a href="https://www.w3.org/TR/xml/#sec-common-syn">XML 1.0 – 2.3 Common Syntactic Constructs</a>
     * @see <a href="https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-common-syn">XML 1.1 – 2.3 Common Syntactic Constructs</a>
     */
    private static final String NAME_START_CHAR = "_A-Za-z\u00C0-\u00D6\u00D8-\u00F6\u00F8-\u02FF\u0370-\u037D\u037F-\u1FFF\u200C-\u200D\u2070-\u218F\u2C00-\u2FEF\u3001-\uD7FF\uF900-\uFDCF\uFDF0-\uFFFD";

    /**
     * {@code NameChar} without colon.
     *
     * <pre>NameChar	::=	NameStartChar | "-" | "." | [0-9] | #xB7 | [#x0300-#x036F] | [#x203F-#x2040]</pre>
     *
     * @see <a href="https://www.w3.org/TR/xml/#sec-common-syn">XML 1.0 – 2.3 Common Syntactic Constructs</a>
     * @see <a href="https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-common-syn">XML 1.1 – 2.3 Common Syntactic Constructs</a>
     */
    private static final String NAME_CHAR = NAME_START_CHAR + "-.0-9\u00B7\u0300-\u036F\u203F-\u2040";

    /**
     * {@code NCName}
     *
     * <pre>
     * NCName		::=	NCNameStartChar NCNameChar*	(An XML Name, minus the ":")
     * NCNameChar	::=	NameChar -':'
     * NCNameStartChar	::=	NameStartChar -':'
     * </pre>
     *
     * @see <a href="https://www.w3.org/TR/xml-names/#ns-qualnames">Namespaces in XML 1.0 – 4 Qualified Names</a>
     * @see <a href="https://www.w3.org/TR/2006/REC-xml-names11-20060816/#ns-qualnames">Namespaces in XML 1.1 – 4 Qualified Names</a>
     */
    private static final String NCNAME = "["+NAME_START_CHAR+"]["+NAME_CHAR+"]*";

    /**
     * Regular expression for {@code Name} (with colon).
     *
     * <pre>Name	::=	NameStartChar (NameChar)*</pre>
     *
     * @see <a href="https://www.w3.org/TR/xml/#sec-common-syn">XML 1.0 – 2.3 Common Syntactic Constructs</a>
     * @see <a href="https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-common-syn">XML 1.1 – 2.3 Common Syntactic Constructs</a>
     */
    private static final Pattern RE_NAME = Pattern.compile("[:"+NAME_START_CHAR+"][:"+NAME_CHAR+"]*");

    /**
     * Regular expression for {@code NCName}.
     *
     * <pre>
     * NCName		::=	NCNameStartChar NCNameChar*	(An XML Name, minus the ":")
     * NCNameChar	::=	NameChar -':'
     * NCNameStartChar	::=	NameStartChar -':'
     * </pre>
     *
     * @see <a href="https://www.w3.org/TR/xml-names/#ns-qualnames">Namespaces in XML 1.0 – 4 Qualified Names</a>
     * @see <a href="https://www.w3.org/TR/2006/REC-xml-names11-20060816/#ns-qualnames">Namespaces in XML 1.1 – 4 Qualified Names</a>
     */
    private static final Pattern RE_NCNAME = Pattern.compile(NCNAME);

    /**
     * Regular expression for {@code QName}.
     *
     * <pre>
     * QName		::=	PrefixedName | UnprefixedName
     * PrefixedName	::=	Prefix ':' LocalPart
     * UnprefixedName	::=	LocalPart
     * Prefix		::=	NCName
     * LocalPart	::=	NCName
     * </pre>
     *
     * @see <a href="https://www.w3.org/TR/xml-names/#ns-qualnames">Namespaces in XML 1.0 – 4 Qualified Names</a>
     * @see <a href="https://www.w3.org/TR/2006/REC-xml-names11-20060816/#ns-qualnames">Namespaces in XML 1.1 – 4 Qualified Names</a>
     */
    private static final Pattern RE_QNAME = Pattern.compile("(?:"+NCNAME+":)?"+NCNAME);

        if (this.namespace.equals(Namespace.NO_NAMESPACE)) {
            validateName(this.name);
        } else {
            validateNCName(this.name);
        }
        validateNCName(this.name);
        validateQName(this.qualifiedName);

    private static void validateName(String name) {
        if (!RE_NAME.matcher(name).matches()) {
            throw new IllegalArgumentException(String.format("Illegal character in name: '%s'.", name));
        }
    }

    protected static void validateNCName(String ncname) {
        if (!RE_NCNAME.matcher(ncname).matches()) {
            throw new IllegalArgumentException(String.format("Illegal character in local name: '%s'.", ncname));
        }
    }

    private static void validateQName(String qname) {
        if (!RE_QNAME.matcher(qname).matches()) {
            throw new IllegalArgumentException(String.format("Illegal character in qualified name: '%s'.", qname));
        }
    }
        } else if (index == 0){
            throw new IllegalArgumentException("Qualified name cannot start with ':'.");
package org.dom4j;

import org.testng.annotations.Test;

/**
 * @author Filip Jirsák
 */
public class AllowedCharsTest {
    @Test
    public void localName() {
        QName.get("element");
        QName.get(":element");
        QName.get("elem:ent");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void localNameFail() {
        QName.get("!element");
    }

    @Test
    public void qname() {
        QName.get("element", "http://example.com/namespace");
        QName.get("ns:element", "http://example.com/namespace");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void qnameFail1() {
        QName.get("ns:elem:ent", "http://example.com/namespace");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void qnameFail2() {
        QName.get(":nselement", "http://example.com/namespace");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void createElementLT() {
        DocumentHelper.createElement("element<name");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void createElementGT() {
        DocumentHelper.createElement("element>name");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void createElementAmpersand() {
        DocumentHelper.createElement("element&name");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void addElement() {
        Element root = DocumentHelper.createElement("root");
        root.addElement("element>name");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void addElementQualified() {
        Element root = DocumentHelper.createElement("root");
        root.addElement("element>name", "http://example.com/namespace");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void addElementQualifiedPrefix() {
        Element root = DocumentHelper.createElement("root");
        root.addElement("ns:element>name", "http://example.com/namespace");
    }

    @Test(expectedExceptions = IllegalArgumentException.class)
    public void addElementPrefix() {
        Element root = DocumentHelper.createElement("root");
        root.addElement("ns>:element", "http://example.com/namespace");
    }

    //TODO It is illegal to create element or attribute with namespace prefix and empty namespace IRI.
    //See https://www.w3.org/TR/2006/REC-xml-names11-20060816/#scoping
}