eerohele / sigel

XSLT and XPath in your Clojure
Eclipse Public License 1.0
28 stars 3 forks source link

XML External Entity Attack #8

Closed topipoh closed 9 months ago

topipoh commented 9 months ago

Hi!

Thanks for writing sigel, we've been very happy with it.

Today we noticed that the default Saxon configuration might make us vulnerable to XXE attacks:

https://www.illucit.com/en/java/saxon-he-external-entity-processing-xxe

echo secret > /tmp/foo

/tmp/test.xml

<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE foo [  
  <!ELEMENT foo ANY >
  <!ENTITY xxe SYSTEM "file:///tmp/foo" >]>

  <foo>&xxe;</foo>
(-> "/tmp/test.xml" clojure.java.io/file slurp sigel.protocols/build)
=> #object[net.sf.saxon.s9api.XdmNode 0x66805298 "<foo>secret\n</foo>"]

We can fix this by using our own builder instead of sigel.protocols/build:

(let [configuration (net.sf.saxon.Configuration.)
      processor (doto (net.sf.saxon.s9api.Processor. configuration)
                  (.setConfigurationProperty
                    (str net.sf.saxon.lib.FeatureKeys/XML_PARSER_FEATURE
                         "http://xml.org/sax/features/external-general-entities")
                    false))
      builder (.newDocumentBuilder processor)
      xml-string (-> "/tmp/test.xml" clojure.java.io/file slurp)
      source (javax.xml.transform.stream.StreamSource. (java.io.StringReader. xml-string))]
  (.build builder source))

=> #object[net.sf.saxon.s9api.XdmNode 0x78a9c497 "<foo/>"]

Do you think it would be a good idea to disable external general entities by default in sigel.saxon/processor?

Something like:

(def ^Processor processor
  (doto (Processor. configuration)
    (.setConfigurationProperty
      (str FeatureKeys/XML_PARSER_FEATURE "http://xml.org/sax/features/external-general-entities") false)))
eerohele commented 9 months ago

Thanks for letting me know! While this is a breaking change, I think for a security issue, it makes sense. I'll make the change and issue a new release today.

eerohele commented 9 months ago

This is resolved in me.flowthing/sigel {:mvn/version "1.1.0"}:

λ clj -Srepro -Sdeps '{:deps {me.flowthing/sigel {:mvn/version "1.0.3"}}}'
Clojure 1.11.1
user=> (require '[sigel.xpath.core :as xpath])
nil
user=> (xpath/value-of "<!DOCTYPE x [<!ELEMENT x ANY> <!ENTITY xxe SYSTEM 'file:///etc/hostname'>]><x>&xxe;</x>" "/x")
Execution error (FileNotFoundException) at java.io.FileInputStream/open0 (FileInputStream.java:-2).
/etc/hostname (No such file or directory)
user=>
λ clj -Srepro -Sdeps '{:deps {me.flowthing/sigel {:mvn/version "1.1.0"}}}'
Clojure 1.11.1
user=> (require '[sigel.xpath.core :as xpath])
nil
user=> (xpath/value-of "<!DOCTYPE x [<!ELEMENT x ANY> <!ENTITY xxe SYSTEM 'file:///etc/hostname'>]><x>&xxe;</x>" "/x")
""
user=>