🐛 Sanitize SVG files on upload to prevent XSS

Add sanitize-svg function that removes dangerous elements and attributes:
- script tags
- foreignObject elements
- Event handler attributes (onload, onmouseover, etc.)
- javascript: URLs from href/xlink:href attributes

Apply sanitization in process-main-image before storing SVG files.

AI-assisted-by: mimo-v2.5-pro
This commit is contained in:
Andrey Antukh 2026-07-29 10:18:57 +00:00
parent 319a2185c9
commit 31b80f5d30
3 changed files with 98 additions and 3 deletions

View File

@ -124,6 +124,47 @@
(dm/with-open [istream (IOUtils/toInputStream ^String text "UTF-8")]
(xml/parse istream secure-parser-factory))))
(defn- sanitize-svg-element
"Recursively sanitize an SVG element by removing dangerous tags and attributes."
[{:keys [tag attrs content] :as element}]
(when (and (map? element) tag)
(let [dangerous-tags #{:script :foreignObject}
dangerous-attrs-pattern #"(?i)^(on\w+|xmlns:.*)$"
javascript-href-pattern #"(?i)^javascript:"]
(when-not (contains? dangerous-tags tag)
(let [clean-attrs (->> attrs
(remove (fn [[k v]]
(or (re-matches dangerous-attrs-pattern (name k))
(and (#{:href :xlink:href} k)
(string? v)
(re-find javascript-href-pattern v)))))
(into {}))
clean-content (when content
(->> content
(filter #(or (string? %) (map? %)))
(map (fn [child]
(if (map? child)
(sanitize-svg-element child)
child)))
(filter some?)
vec))]
(cond-> {:tag tag :attrs clean-attrs}
(seq clean-content) (assoc :content clean-content)))))))
(defn sanitize-svg
"Sanitize SVG content by removing dangerous elements and attributes.
Removes <script> tags, <foreignObject> elements, event handlers (on*),
and javascript: URLs from href attributes."
[svg-text]
(try
(let [parsed (parse-svg svg-text)
sanitized (sanitize-svg-element parsed)]
(if sanitized
(with-out-str (xml/emit sanitized))
svg-text))
(catch Exception _
svg-text)))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; IMAGE THUMBNAILS
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

View File

@ -24,6 +24,7 @@
[app.storage :as sto]
[app.storage.tmp :as tmp]
[app.util.services :as sv]
[datoteka.fs :as fs]
[datoteka.io :as io])
(:import
java.io.OutputStream))
@ -113,13 +114,22 @@
(defn- process-main-image
[info]
(let [hash (sto/calculate-hash (:path info))
data (-> (sto/content (:path info))
(let [path (:path info)
mtype (:mtype info)
path (if (= mtype "image/svg+xml")
(let [content (slurp path)
sanitized (media/sanitize-svg content)
temp-path (fs/create-tempfile :prefix "penpot-svg-" :suffix ".svg")]
(spit (str temp-path) sanitized)
temp-path)
path)
hash (sto/calculate-hash path)
data (-> (sto/content path)
(sto/wrap-with-hash hash))]
{::sto/content data
::sto/deduplicate? true
::sto/touched-at (:ts info)
:content-type (:mtype info)
:content-type mtype
:bucket "file-media-object"}))
(defn- process-thumb-image

View File

@ -55,6 +55,50 @@
(t/is (pos? (:width info)))
(t/is (pos? (:height info))))))
(t/deftest sanitize-svg-script-tag
(t/testing "sanitize-svg removes script tags"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><script>alert('xss')</script><rect width=\"50\" height=\"50\"/></svg>"
result (media/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "<script>")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-event-handlers
(t/testing "sanitize-svg removes event handler attributes"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\" onload=\"alert('xss')\"><rect width=\"50\" height=\"50\" onmouseover=\"alert('xss')\"/></svg>"
result (media/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "onload")))
(t/is (not (clojure.string/includes? result "onmouseover")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-javascript-href
(t/testing "sanitize-svg removes javascript: URLs from href attributes"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"100\" height=\"100\"><a xlink:href=\"javascript:alert('xss')\"><rect width=\"50\" height=\"50\"/></a></svg>"
result (media/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "javascript:")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<a")))))
(t/deftest sanitize-svg-foreign-object
(t/testing "sanitize-svg removes foreignObject elements"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><foreignObject width=\"100\" height=\"100\"><body xmlns=\"http://www.w3.org/1999/xhtml\"><script>alert('xss')</script></body></foreignObject><rect width=\"50\" height=\"50\"/></svg>"
result (media/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "foreignObject")))
(t/is (not (clojure.string/includes? result "<script>")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-clean-content
(t/testing "sanitize-svg preserves clean SVG content"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><rect width=\"50\" height=\"50\" fill=\"red\"/><circle cx=\"75\" cy=\"75\" r=\"20\" fill=\"blue\"/></svg>"
result (media/sanitize-svg svg)]
(t/is (clojure.string/includes? result "<rect"))
(t/is (clojure.string/includes? result "<circle"))
(t/is (or (clojure.string/includes? result "fill=\"red\"")
(clojure.string/includes? result "fill='red'")))
(t/is (or (clojure.string/includes? result "fill=\"blue\"")
(clojure.string/includes? result "fill='blue'"))))))
(t/deftest info-invalid-image
(t/testing "info on invalid image raises error"
(let [path (fs/create-tempfile :prefix "penpot-test-" :suffix ".jpg")]