github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 05:50:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Sanitize SVG namespace-prefixed elements and attributes (#12073)

Browse Source 
* 🐛 Sanitize SVG namespace-prefixed elements and attributes

The SVG sanitizer compared qualified tag and attribute names against
its blocklists. The XML parser keeps the namespace prefix in the
keyword name (❌script), so a script element written with a prefix
bound to the SVG namespace passed sanitization untouched and was
later served as image/svg+xml, executing in the Penpot origin.

Match tags and attributes on the local name (the part after the last
colon) instead, so dangerous elements and attributes are removed no
matter which prefix they use. Legitimately prefixed attributes such
as xlink:href keep working.

Closes #12071

AI-assisted-by: glm-5.3-flash

* ♻️ Cover prefixed SVG vectors and hoist sanitizer blocklist

Apply the code review findings on top of the sanitizer fix:

- Add a test for the javascript: href vector using a prefix bound to
  the xlink namespace, the other payload variant the old exact-key
  check missed.
- Add a test asserting that legitimately prefixed attributes such as
  xml:space are preserved by the local-name matching.
- Hoist the dangerous-tags blocklist next to the other sanitizer
  policy defs and bind the keyword name once in local-name.

Relates to #12071

AI-assisted-by: glm-5.3-flash
This commit is contained in:
Andrey Antukh 2026-10-05 11:56:59 +02:00 committed by GitHub
parent b5d4cd1f0b
commit 238a4d8613
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 103 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
backend/src/app/media/svg.clj
View File

@ -52,30 +52,47 @@
(def ^:private dangerous-attrs-pattern #"(?i)^on\w+$")
(def ^:private javascript-href-pattern #"(?i)^javascript:")
(def ^:private dangerous-tags
#{"script" "foreignObject" "set"
"animate" "animateTransform" "animateColor" "animateMotion"})
(defn- local-name
"Return the local name of a parsed tag or attribute keyword.
The XML parser interns qualified names with the prefix included in
the keyword name and no keyword namespace: `<x:script>` parses as
the keyword `:x:script`. Dangerous elements and attributes behave
the same in the browser regardless of the prefix used, so matching
is done on the local name: the part after the last `:`."
[k]
(let [n (name k)]
(if (str/includes? n ":")
(last (str/split n ":"))
n)))
(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 :set :animate :animateTransform :animateColor :animateMotion}]
(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 (str/trim 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)))))))
(when-not (contains? dangerous-tags (local-name tag))
(let [clean-attrs (->> attrs
(remove (fn [[k v]]
(or (re-matches dangerous-attrs-pattern (local-name k))
(and (= "href" (local-name k))
(string? v)
(re-find javascript-href-pattern (str/trim 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.

66
backend/test/backend_tests/media_test.clj
View File

@ -137,6 +137,72 @@
(t/is (not (clojure.string/includes? result "onmouseover")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-namespace-prefixed-script
(t/testing "sanitize-svg removes script tags written with a namespace prefix bound to the SVG namespace"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:x=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><x:script>alert('xss')</x:script><rect width=\"50\" height=\"50\"/></svg>"
result (svg/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "<x:script")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-arbitrary-prefixed-script
(t/testing "sanitize-svg removes script tags written with an arbitrary namespace prefix"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:p=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><p:script>alert('xss')</p:script><rect width=\"50\" height=\"50\"/></svg>"
result (svg/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "<p:script")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-nested-prefixed-script
(t/testing "sanitize-svg removes namespace-prefixed script tags from nested elements"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:x=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><g><x:script>alert('xss')</x:script></g></svg>"
result (svg/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "<x:script")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<g")))))
(t/deftest sanitize-svg-prefixed-dangerous-tags
(t/testing "sanitize-svg removes dangerous animation elements written with a namespace prefix"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:x=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><x:foreignObject width=\"10\" height=\"10\"/><x:set attributeName=\"onmouseover\" to=\"alert('xss')\"/><x:animate attributeName=\"fill\" to=\"red\"/><x:animateTransform attributeName=\"transform\"/><rect width=\"50\" height=\"50\"/></svg>"
result (svg/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "foreignObject")))
(t/is (not (clojure.string/includes? result "<x:set")))
(t/is (not (clojure.string/includes? result "<x:animate")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-prefixed-javascript-href
(t/testing "sanitize-svg removes javascript: URLs from href attributes written with a namespace prefix"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:x=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><a x:href=\"javascript:alert('xss')\"><rect width=\"50\" height=\"50\"/></a></svg>"
result (svg/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-prefixed-event-handlers
(t/testing "sanitize-svg removes event handler attributes written with a namespace prefix"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:x=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\" x:onload=\"alert('xss')\"><rect width=\"50\" height=\"50\" x:onclick=\"alert('xss')\"/></svg>"
result (svg/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "onload")))
(t/is (not (clojure.string/includes? result "onclick")))
(t/is (not (clojure.string/includes? result "alert")))
(t/is (clojure.string/includes? result "<rect")))))
(t/deftest sanitize-svg-prefixed-xlink-namespace-href
(t/testing "sanitize-svg removes javascript: URLs from href written with a prefix bound to the xlink namespace"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:y=\"http://www.w3.org/1999/xlink\" width=\"100\" height=\"100\"><a y:href=\"javascript:alert('xss')\"><rect width=\"50\" height=\"50\"/></a></svg>"
result (svg/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-preserved-prefixed-attrs
(t/testing "sanitize-svg preserves legitimate attributes written with a namespace prefix"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xml=\"http://www.w3.org/XML/1998/namespace\"><text xml:space=\"preserve\">hola</text></svg>"
result (svg/sanitize-svg svg)]
(t/is (clojure.string/includes? result "xml:space"))
(t/is (clojure.string/includes? result "hola")))))
(t/deftest info-invalid-image
(t/testing "info on invalid image raises error"
(let [path (fs/create-tempfile :prefix "penpot-test-" :suffix ".jpg")]