Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Prevent XXE attacks by disabling external entities resolution in the …

…default parser

Signed-off-by: Ryan Senior <ryan.senior@puppetlabs.com>
  • Loading branch information...
commit 14d0af1cef8c8af497a2602dc710ef65967537b6 1 parent 320eda8
@skuro skuro authored senior committed
View
25 src/main/clojure/clojure/data/xml.clj
@@ -54,7 +54,7 @@
(= s "")))
(defn emit-cdata [^String cdata-str ^javax.xml.stream.XMLStreamWriter writer]
- (when-not (str-empty? cdata-str)
+ (when-not (str-empty? cdata-str)
(let [idx (.indexOf cdata-str "]]>")]
(if (= idx -1)
(.writeCData writer cdata-str )
@@ -97,7 +97,7 @@
(if-let [r (seq (rest coll))]
(cons (next-events (first coll) r) next-items)
(next-events (first coll) next-items)))
-
+
String
(gen-event [s]
(Event. :chars nil nil s))
@@ -121,13 +121,13 @@
(Event. :cdata nil nil (:content cdata)))
(next-events [_ next-items]
next-items)
-
+
Comment
(gen-event [comment]
(Event. :comment nil nil (:content comment)))
(next-events [_ next-items]
next-items)
-
+
nil
(gen-event [_]
(Event. :chars nil nil ""))
@@ -279,7 +279,7 @@
; Note, sreader is mutable and mutated here in pull-seq, but it's
; protected by a lazy-seq so it's thread-safe.
(defn- pull-seq
- "Creates a seq of events. The XMLStreamConstants/SPACE clause below doesn't seem to
+ "Creates a seq of events. The XMLStreamConstants/SPACE clause below doesn't seem to
be triggered by the JDK StAX parser, but is by others. Leaving in to be more complete."
[^XMLStreamReader sreader]
(lazy-seq
@@ -289,7 +289,7 @@
(cons (event :start-element
(keyword (.getLocalName sreader))
(attr-hash sreader) nil)
- (pull-seq sreader))
+ (pull-seq sreader))
XMLStreamConstants/END_ELEMENT
(cons (event :end-element
(keyword (.getLocalName sreader)) nil nil)
@@ -327,9 +327,13 @@
"Parses the XML InputSource source using a pull-parser. Returns
a lazy sequence of Event records. Accepts key pairs
with XMLInputFactory options, see http://docs.oracle.com/javase/6/docs/api/javax/xml/stream/XMLInputFactory.html
- and xml-input-factory-props for more information. Defaults coalescing true."
+ and xml-input-factory-props for more information.
+ Defaults coalescing true and supporting-external-entities false."
[s & {:as props}]
- (let [fac (new-xml-input-factory (merge {:coalescing true} props))
+ (let [merged-props (merge {:coalescing true
+ :supporting-external-entities false}
+ props)
+ fac (new-xml-input-factory merged-props)
;; Reflection on following line cannot be eliminated via a
;; type hint, because s is advertised by fn parse to be an
;; InputStream or Reader, and there are different
@@ -373,7 +377,7 @@
(when (instance? java.io.OutputStreamWriter stream)
(check-stream-encoding stream (or (:encoding opts) "UTF-8")))
-
+
(.writeStartDocument writer (or (:encoding opts) "UTF-8") "1.0")
(doseq [event (flatten-elements [e])]
(emit-event event writer))
@@ -395,7 +399,7 @@
(defn indent
"Emits the XML and indents the result. WARNING: this is slow
- it will emit the XML and read it in again to indent it. Intended for
+ it will emit the XML and read it in again to indent it. Intended for
debugging/testing only."
[e ^java.io.Writer stream & {:as opts}]
(let [sw (java.io.StringWriter.)
@@ -410,4 +414,3 @@
(let [^java.io.StringWriter sw (java.io.StringWriter.)]
(indent e sw)
(.toString sw)))
-
View
49 src/test/clojure/clojure/data/xml/test_entities.clj
@@ -0,0 +1,49 @@
+; Copyright (c) Rich Hickey. All rights reserved.
+; The use and distribution terms for this software are covered by the
+; Eclipse Public License 1.0 (http://opensource.org/licenses/eclipse-1.0.php)
+; which can be found in the file epl-v10.html at the root of this distribution.
+; By using this software in any fashion, you are agreeing to be bound by
+; the terms of this license.
+; You must not remove this notice, or any other, from this software.
+
+(ns ^{:doc "Test that external entities are not resolved by default, see https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing"
+ :author "Carlo Sciolla"}
+ clojure.data.xml.test-entities
+ (:use clojure.test
+ clojure.data.xml)
+ (:require [clojure.java.io :as io]))
+
+(defn vulnerable-input
+ "Creates an XML with an external entity referring to the given URL"
+ [file-url]
+ (str "<?xml version=\"1.0\" encoding=\"ISO-8859-1\"?>"
+ "<!DOCTYPE foo ["
+ " <!ELEMENT foo ANY >"
+ " <!ENTITY xxe SYSTEM \"" file-url "\" >]>"
+ "<foo>&xxe;</foo>"))
+
+(defn secret-file
+ "Returns the URL to the secret file containing the server root password"
+ []
+ (io/resource "secret.txt"))
+
+(defn parse-vulnerable-file
+ "Parses the vulnerable file, optionally passing the given options to the parser"
+ ([] (parse-str (vulnerable-input (secret-file))))
+ ([& options] (apply parse-str (vulnerable-input (secret-file)) options)))
+
+(deftest prevent-xxe-by-default
+ (testing "To prevent XXE attacks, exernal entities by default resolve to nil"
+ (let [parsed (parse-vulnerable-file)
+ expected #clojure.data.xml.Element{:tag :foo
+ :attrs {}
+ :content ()}]
+ (is (= expected parsed)))))
+
+(deftest allow-external-entities-if-required
+ (testing "If explicitly enabled, external entities are property resolved"
+ (let [parsed (parse-vulnerable-file :supporting-external-entities true)
+ expected #clojure.data.xml.Element{:tag :foo
+ :attrs {}
+ :content ("root_password\n")}]
+ (is (= expected parsed)))))
View
1  src/test/resources/secret.txt
@@ -0,0 +1 @@
+root_password
Please sign in to comment.
Something went wrong with that request. Please try again.