CLJ-2611 Disallow XXE by default in clojure.xml/parse

Signed-off-by: Alex Miller <alex.miller@cognitect.com>

Author: puredanger

src/clj/clojure/xml.clj

@@ -72,18 +72,50 @@
            (skippedEntity [name])
            ))))
 
-(defn startparse-sax [s ch]
-  (.. SAXParserFactory (newInstance) (newSAXParser) (parse s ch)))
+(defn sax-parser
+  "Create a new SAXParser"
+  {:added "1.11"}
+  ^SAXParser []
+  (.newSAXParser (SAXParserFactory/newInstance)))
+
+(defn disable-external-entities
+  "Modifies a SAXParser to disable external entity resolution to prevent XXE attacks"
+  {:added "1.11"}
+  ^SAXParser [^SAXParser parser]
+  (let [reader (.getXMLReader parser)]
+    ;; as per https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
+    (.setFeature reader "http://apache.org/xml/features/nonvalidating/load-external-dtd" false)
+    (.setFeature reader "http://xml.org/sax/features/external-general-entities", false)
+    (.setFeature reader "http://xml.org/sax/features/external-parameter-entities" false)
+    parser))
+
+(defn startparse-sax
+  "A startparse function suitable for use with clojure.xml/parse.
+  Note that this function is open to XXE entity attacks, see startparse-sax-safe."
+  {:added "1.0"}
+  [s ch]
+  (.parse (sax-parser) s ch))
+
+(defn startparse-sax-safe
+  "A startparse function suitable for use with clojure.xml/parse.
+  External entity resolution is disabled to prevent XXE entity attacks."
+  {:added "1.11"}
+  [s ch]
+  (.parse (disable-external-entities (sax-parser)) s ch))
 
 (defn parse
   "Parses and loads the source s, which can be a File, InputStream or
   String naming a URI. Returns a tree of the xml/element struct-map,
   which has the keys :tag, :attrs, and :content. and accessor fns tag,
   attrs, and content. Other parsers can be supplied by passing
   startparse, a fn taking a source and a ContentHandler and returning
-  a parser"
+  a parser.
+
+  Prior to 1.11, used startparse-sax by default. As of 1.11, uses
+  startparse-sax-safe, which disables XXE (XML External Entity)
+  processing. Pass startparse-sax to revert to prior behavior."
   {:added "1.0"}
-  ([s] (parse s startparse-sax))
+  ([s] (parse s startparse-sax-safe))
   ([s startparse]
     (binding [*stack* nil
               *current* (struct element)

test/clojure/test_clojure/clojure_xml.clj

@@ -11,9 +11,18 @@
 
 (ns clojure.test-clojure.clojure-xml
   (:use clojure.test)
-  (:require [clojure.xml :as xml]))
-
+  (:require [clojure.xml :as xml])
+  (:import [java.io ByteArrayInputStream]))
 
+(deftest CLJ-2611-avoid-XXE
+  (let [xml-str "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>
+<!DOCTYPE foo [
+  <!ELEMENT foo ANY >
+  <!ENTITY xxe SYSTEM \"file:///etc/hostname\" >]>
+<foo>&xxe;</foo>"]
+    (is (= {:tag :foo, :attrs nil, :content nil}
+           (with-open [input (ByteArrayInputStream. (.getBytes xml-str))]
+             (xml/parse input))))))
 ; parse
 
 ; emit-element