github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-11-10 20:10:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Escape SVG values and sanitize every SVG upload (#12887)

Browse Source 
* 🐛 Escape SVG values and sanitize every SVG upload

The SVG sanitizer removed dangerous elements and attributes by name and
then wrote the tree back with clojure.xml/emit, which escapes nothing.
A payload hidden in an attribute value or in a text node became live
markup again: an id of "x' onload='alert(1)'" was stored as a real
onload handler, and an entity encoded script tag was stored as a real
script element.

Write the filtered tree with a serializer that escapes text and
attribute values, and route every path that stores an uploaded SVG
through it. The thumbnail writers (create-file-object-thumbnail and
create-file-thumbnail) and the organization logo stored the upload
untouched, so they could plant live markup in a public bucket.

The filter only knows about SVG elements, so it also accepted
documents that are not SVG at all: a page with an iframe or a meta
refresh was stored as an image. The sanitizer now rejects any document
whose root is not an svg element in the SVG namespace, and it compares
the scheme of an url the way the browser does, so a scheme split with
a tab or a line break no longer slips through the javascript: filter.
The attributes are written sorted by name, because the parser hands
them over in the reverse order of the document and the output changed
on every sanitization.

The profile picture of an identity provider is not stored either when
it is an SVG: the avatar is always rendered as an image, so an SVG
would only add the risk of keeping live markup in the storage. The
dimensions of an SVG are now read from its local name, so an SVG with
a namespace prefix can be uploaded like any other.

The logo had no size limit and the sanitizer reads the whole file, so
it now honors the media max file size.

The output shape changes with it: no line breaks, double quotes, and
the single quote of an attribute value escaped as an entity. The hash
of every stored SVG object changes with it.

The media tests use cuerdas.core instead of clojure.string, as the rest
of the codebase does.

Relates to GHSA-qwhj-rchj-58mg

AI-assisted-by: minimax-m3.1-flash

* 🐛 Drop non-SVG namespace subtrees in the SVG sanitizer

The sanitizer only checked the namespace of the root, so a child could
rebind xmlns to XHTML and smuggle live HTML (iframe/srcdoc/meta) past the
blocklist. The serializer escapes attribute values, but the browser decodes
them again, so srcdoc stays live.

Track the namespace bindings through the recursion and drop any element
that is not in the SVG namespace, with its subtree. The root is validated
up front with the same model, so a document that is not an svg document is
rejected with an honest hint instead of a misleading one.

The default namespace is stored under a namespaced key, because a prefix
literally named `default` collided with the internal key and re-opened the
same hole through a prefixed binding. The attribute filter now keeps
`xmlns`/`xmlns:*` declarations, which are not event handlers, so a prefix
named like one does not end up undeclared in the output.

The blocklist also drops HTML-only elements written in the svg namespace
(iframe, meta, embed, link, base).

Add regression tests for the child rebinding, for the prefixed variants,
for the `default` prefix, for the root local name, for the
create-file-media-object-from-url path (mocked download), and for the
HTML-only tags. Assert double-quoted attributes in the clean content test.

Relates to GHSA-qwhj-rchj-58mg

AI-assisted-by: longcat-2.5-free
This commit is contained in:
Andrey Antukh 2026-11-10 15:52:28 +01:00 committed by GitHub
parent 10b08d5224
commit 4874da450f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 857 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.serena/memories/backend/subtleties.md
View File

@ -72,6 +72,7 @@
- The `media-storage-bytes-per-team` quota check on `upload-file-media-object` is TEMPORARILY DISABLED (2026-10-19, kept commented in `app.rpc.commands.media` with its `sql:get-team-id-for-file` lookup still active for the audit props): the quote performs a full `SUM` over `storage_object` per upload and was too slow in production. The `upload-sessions-per-profile` quota is unaffected and stays active. Re-enable only with a cheaper approximation.
- Storage abstraction, backend configuration, logical buckets, object lifecycle, deduplication, access rules, and garbage collection: `mem:backend/storage`.
- SVG validation strips DOCTYPE and uses secure SAX parsing. Basic SVG info falls back to 100x100 dimensions when width/height/viewBox are missing.
- The SVG sanitizer (`app.media.svg`) is a **blocklist**, not an allowlist: it drops dangerous elements and attributes by name and escapes every text and attribute value when it writes the tree back, so a payload hidden in a value cannot become live markup again. The allowlist rework is [#12190](https://github.com/penpot/penpot/issues/12190) and lands in media-processor, not here. `sanitize-svg` also tracks the namespace bindings through the recursion and drops any element that is not in the SVG namespace, so a document whose root is not an `svg` element in the SVG namespace is rejected and a child that rebinds `xmlns` to XHTML (`iframe`/`srcdoc`/`meta`) is dropped with its subtree; it compares the scheme of an URL the way the browser does (dropping tab/CR/LF/spaces), so `java	script:` does not slip through the `javascript:` filter. The paths that store an uploaded SVG as its final object go through `app.media.svg/sanitize-svg-upload` (media upload, the URL download, both thumbnail writers and the organization logo); the three binfile imports go through `app.binfile.common/sanitize-imported-svg`, which calls `sanitize-svg` directly. The profile picture is not an SVG: `import-profile-picture` ignores an `image/svg+xml` download, because the avatar is always rendered as an image. The caller checks the size before calling it: the helper reads the whole file, and the size limit applies to the upload, not to the sanitized copy, which can be up to six times bigger.
- Raster metadata is shell-derived with ImageMagick `identify`, verifies detected MIME against the supplied MIME, and swaps dimensions for EXIF orientations 6/8.
- Remote image download requires 2xx status, `content-length`, a known MIME, and size under the configured maximum before writing the temp file; mismatched byte count is an internal error.
- Font processing shells out to FontForge and WOFF conversion tools and can derive TTF/OTF/WOFF variants from uploaded fonts.

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

@ -12,8 +12,10 @@
[app.common.data.macros :as dm]
[app.common.exceptions :as ex]
[app.common.logging :as l]
[app.storage.tmp :as tmp]
[clojure.xml :as xml]
[cuerdas.core :as str])
[cuerdas.core :as str]
[datoteka.fs :as fs])
(:import
clojure.lang.XMLHandler
java.io.InputStream
@ -46,15 +48,85 @@
(xml/parse istream secure-parser-factory))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; SVG SERIALIZATION
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(def ^:private xml-declaration
"Declaration written at the top of every sanitized document."
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>")
(defn- escape-text
"Escape the characters that are special inside a XML text node."
[s]
(-> s
(str/replace "&" "&amp;")
(str/replace "<" "&lt;")
(str/replace ">" "&gt;")))
(defn- escape-attr
"Escape the characters that are special inside a XML attribute value.
The value is written between double quotes, so the quote itself is
escaped too.
Tabs and line breaks become spaces: that is what the XML parser does
with them on the next read, and it keeps the output stable."
[s]
(-> (str s)
(str/replace #"[\t\r\n]" " ")
(str/replace "&" "&amp;")
(str/replace "<" "&lt;")
(str/replace ">" "&gt;")
(str/replace "\"" "&quot;")
(str/replace "'" "&apos;")))
(defn- emit-element
"Serialize a parsed element, escaping every text and attribute value.
The tag and attribute names come from the parser, so they are valid
XML names; only their values can carry markup.
The attributes are sorted by name: the parser hands them over in the
reverse order of the document, so writing them as they come would
make the output change on every sanitization."
[{:keys [tag attrs content]}]
(let [attrs-str (apply str
(for [[k v] (sort-by (comp name key) attrs)]
(str " " (name k) "=\"" (escape-attr (str v)) "\"")))
content-str (apply str
(for [child content]
(if (map? child)
(emit-element child)
(escape-text child))))]
(if (seq content)
(str "<" (name tag) attrs-str ">" content-str "</" (name tag) ">")
(str "<" (name tag) attrs-str "/>"))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; SVG SANITIZATION
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(def ^:private dangerous-attrs-pattern #"(?i)^on\w+$")
(def ^:private javascript-href-pattern #"(?i)^javascript:")
(def ^:private url-ignored-chars
"Characters that the URL parser discards before it interprets the
scheme of an URL."
#"[\u0000-\u0020\u007f]")
(defn- javascript-url?
"True when the value is a javascript: URL.
The comparison drops the characters the URL parser drops (tab, CR, LF
and spaces), so a scheme split with one of them (`java&#x09;script:`)
cannot slip through."
[v]
(let [v (str/replace (str v) url-ignored-chars "")]
(boolean (re-find javascript-href-pattern (str/trim v)))))
(def ^:private dangerous-tags
#{"script" "foreignObject" "set"
"animate" "animateTransform" "animateColor" "animateMotion"})
"animate" "animateTransform" "animateColor" "animateMotion"
"iframe" "meta" "embed" "link" "base"})
(defn- local-name
"Return the local name of a parsed tag or attribute keyword.
@ -70,24 +142,82 @@
(last (str/split n ":"))
n)))
(def ^:private svg-namespace "http://www.w3.org/2000/svg")
(defn- namespace-bindings
"Return the namespace bindings in scope for an element: `::default`
plus the keyword prefixes declared in `attrs`, merged over the
inherited ones. The default binding is stored under a namespaced
keyword because a prefix cannot contain `/`, so no prefixed binding
can produce the same key.
The XML parser does not process namespaces, so the declarations are
plain attributes (`:xmlns`, `:xmlns:svg`) and the prefix is part of
the tag name (`:svg:svg`)."
[attrs inherited]
(let [own (reduce-kv (fn [acc k v]
(let [n (name k)]
(cond
(= n "xmlns") (assoc acc ::default v)
(str/starts-with? n "xmlns:") (assoc acc (keyword (subs n 6)) v)
:else acc)))
{}
attrs)]
(merge inherited own)))
(defn- element-namespace
"Return the namespace URI of `tag` under `bindings`: the default
namespace when the tag has no prefix, the prefixed binding otherwise.
The prefix is part of the tag name (`:svg:svg`), so the binding key
is the prefix."
[tag bindings]
(let [n (name tag)
idx (str/last-index-of n ":")]
(if (nil? idx)
(::default bindings)
(get bindings (keyword (subs n 0 idx))))))
(defn- svg-root?
"True when the element is an `svg` element in the SVG namespace.
The namespace is resolved with the same model as the recursion
(`namespace-bindings` + `element-namespace`), so the check does not
introduce a second rule."
[{:keys [tag attrs]}]
(let [bindings (namespace-bindings attrs {})]
(and (= "svg" (local-name tag))
(= svg-namespace (element-namespace tag bindings)))))
(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)
(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 {}))
"Recursively sanitize an SVG element by removing dangerous tags and
attributes.
The namespace bindings are tracked through the recursion: an element
that is not in the SVG namespace is dropped with its whole subtree,
because the filter only knows about SVG elements and a child that
rebinds the namespace to XHTML can carry live HTML."
[element bindings]
(let [{:keys [tag attrs content]} element
bindings (namespace-bindings attrs bindings)]
(when (and (map? element) tag
(not (contains? dangerous-tags (local-name tag)))
(= svg-namespace (element-namespace tag bindings)))
(let [clean-attrs (->> attrs
(remove (fn [[k v]]
(let [n (name k)]
(or (and (not (str/starts-with? n "xmlns"))
(re-matches dangerous-attrs-pattern (local-name k)))
(and (= "href" (local-name k))
(string? v)
(javascript-url? v))))))
(into {}))
clean-content (when content
(->> content
(filter #(or (string? %) (map? %)))
(map (fn [child]
(if (map? child)
(sanitize-svg-element child)
(sanitize-svg-element child bindings)
child)))
(filter some?)
vec))]
@ -97,22 +227,65 @@
(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."
and javascript: URLs from href attributes.
The document must be an SVG document: any element that is not in the
SVG namespace is dropped with its subtree, so a root that is not an
`svg` element in the SVG namespace is rejected, because the filter
only knows about SVG elements and attributes.
The filtered tree is written back with an escaping serializer: a
payload hidden in an attribute value or in a text node would become
live markup again if the values were written verbatim."
[svg-text]
(try
(let [parsed (parse-svg svg-text)
sanitized (sanitize-svg-element parsed)]
(if sanitized
(with-out-str (xml/emit sanitized))
(ex/raise :type :validation
:code :invalid-svg-file
:hint "SVG sanitization produced no output")))
(catch Exception e
(l/warn :hint "SVG sanitization failed, rejecting upload" :cause e)
(let [parsed (try
(parse-svg svg-text)
(catch Exception e
(l/warn :hint "SVG parsing failed, rejecting upload" :cause e)
(ex/raise :type :validation
:code :invalid-svg-file
:hint "SVG parsing failed during sanitization"
:cause e)))
_ (when-not (svg-root? parsed)
(l/warn :hint "SVG sanitization rejected a document that is not an svg document")
(ex/raise :type :validation
:code :invalid-svg-file
:hint "the uploaded document is not an svg document"))
sanitized (sanitize-svg-element parsed {})]
(when-not sanitized
(l/warn :hint "SVG sanitization produced no output, rejecting upload")
(ex/raise :type :validation
:code :invalid-svg-file
:hint "SVG parsing failed during sanitization"
:cause e))))
:hint "SVG sanitization produced no output"))
(str xml-declaration (emit-element sanitized))))
(defn sanitize-svg-upload
"Return the upload untouched when its mtype is not `image/svg+xml`.
When it is, write a sanitized copy to a temporary file and return the
upload pointing to it, with the size of the sanitized content.
The caller must check the size of the upload before calling this: the
whole file is read into memory to sanitize it. The size limit applies
to the upload, not to the sanitized copy, which can be up to six times
bigger (a single quote becomes `&apos;`).
Every path that stores an uploaded SVG as its final object must go
through this function, or the stored file can carry live markup. Two
paths store raw bytes on purpose and do not: `upload-tempfile` (a
channel between processes, not publicly accessible) and
`upload-chunk` (the chunks of an upload; the assembled object goes
through this function). The profile picture is not an SVG either:
`import-profile-picture` ignores an `image/svg+xml` download."
[{:keys [mtype path] :as upload}]
(if (= "image/svg+xml" mtype)
(let [content (slurp (str path) :encoding "UTF-8")
clean (sanitize-svg content)
tmp-path (tmp/tempfile :prefix "penpot-svg-" :suffix ".svg" :min-age "1h")]
(spit (str tmp-path) clean :encoding "UTF-8")
(assoc upload :path tmp-path :size (fs/size tmp-path)))
upload))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; SVG INFO EXTRACTION
@ -120,7 +293,11 @@
(defn get-basic-info-from-svg
[{:keys [tag attrs] :as data}]
(when (not= tag :svg)
;; The local name is enough here: this function only reads the
;; dimensions. Whether the document is a valid SVG is decided by
;; `sanitize-svg`, which rejects anything that is not an `svg` element
;; in the SVG namespace.
(when (not= "svg" (local-name tag))
(ex/raise :type :validation
:code :unable-to-parse-svg
:hint "uploaded svg has invalid content"))

27
backend/src/app/rpc/commands/auth.clj
View File

@ -373,16 +373,23 @@
[cfg uri]
(try
(let [storage (sto/resolve cfg)
input (media/download-image cfg uri)
input (media/run cfg {:cmd :info :input input})
hash (sto/calculate-hash (:path input))
content (-> (sto/content (:path input) (:size input))
(sto/wrap-with-hash hash))
sobject (sto/put-object! storage {::sto/content content
::sto/deduplicate? true
:bucket "profile"
:content-type (:mtype input)})]
(:id sobject))
input (media/download-image cfg uri)]
(if (= "image/svg+xml" (:mtype input))
;; The avatar is always rendered as an image, so an SVG would only
;; add the risk of storing live markup: it is not allowed here.
(do
(l/wrn :hint "ignoring svg profile picture" :uri uri)
nil)
(let [input (media/run cfg {:cmd :info :input input})
hash (sto/calculate-hash (:path input))
content (-> (sto/content (:path input) (:size input))
(sto/wrap-with-hash hash))
sobject (sto/put-object! storage {::sto/content content
::sto/deduplicate? true
:bucket "profile"
:content-type (:mtype input)})]
(:id sobject))))
(catch Throwable cause
(l/wrn :hint "unable to import profile picture"
:uri uri

6
backend/src/app/rpc/commands/files_thumbnails.clj
View File

@ -21,6 +21,7 @@
[app.db.sql :as-alias sql]
[app.loggers.audit :as-alias audit]
[app.loggers.webhooks :as-alias webhooks]
[app.media.svg :as svg]
[app.media.validation :as media.v]
[app.rpc :as-alias rpc]
[app.rpc.climit :as-alias climit]
@ -235,8 +236,7 @@
(defn- persist-thumbnail!
[storage media created-at]
(let [path (:path media)
mtype (:mtype media)
(let [{:keys [path mtype]} (svg/sanitize-svg-upload media)
hash (sto/calculate-hash path)
data (-> (sto/content path)
(sto/wrap-with-hash hash))]
@ -403,7 +403,7 @@
(let [storage (::sto/storage cfg)
file (bfc/get-file cfg file-id :include-deleted? true :load-data? false)
props (db/tjson (or (:props params) {}))
{:keys [path mtype]} (:media params)
{:keys [path mtype]} (svg/sanitize-svg-upload (:media params))
hash (sto/calculate-hash path)
data (-> (sto/content path)
(sto/wrap-with-hash hash))

10
backend/src/app/rpc/commands/media.clj
View File

@ -137,15 +137,7 @@
(defn- process-main-image
[info]
(let [path (:path info)
mtype (:mtype info)
path (if (= mtype "image/svg+xml")
(let [content (slurp path)
sanitized (svg/sanitize-svg content)
temp-path (tmp/tempfile :prefix "penpot-svg-" :suffix ".svg" :min-age "5m")]
(spit (str temp-path) sanitized)
temp-path)
path)
(let [{:keys [path mtype]} (svg/sanitize-svg-upload info)
hash (sto/calculate-hash path)
data (-> (sto/content path)
(sto/wrap-with-hash hash))]

9
backend/src/app/rpc/management/nitrate.clj
View File

@ -26,6 +26,7 @@
[app.http :as-alias http]
[app.http.session :as session]
[app.loggers.audit :as audit]
[app.media.svg :as svg]
[app.media.validation :as media.v]
[app.nitrate :as nitrate]
[app.rpc :as rpc]
@ -140,16 +141,18 @@
::nitrate/sso false}
[{:keys [::sto/storage]} {:keys [content organization-id previous-id]}]
(media.v/check-media-type content cm/image-types)
(media.v/check-media-size content)
(media.v/check-upload-path content)
(when previous-id
(sto/touch-object! storage previous-id))
(let [hash (sto/calculate-hash (:path content))
data (-> (sto/content (:path content))
(let [{:keys [path mtype]} (svg/sanitize-svg-upload content)
hash (sto/calculate-hash path)
data (-> (sto/content path)
(sto/wrap-with-hash hash))
obj (sto/put-object! storage {::sto/content data
::sto/deduplicate? true
:bucket "organization"
:content-type (:mtype content)
:content-type mtype
:organization-id organization-id})]
{:id (:id obj)}))

15
backend/test/backend_tests/helpers.clj
View File

@ -686,6 +686,21 @@
(io/file tmp))
tmp))
(defmacro with-temp-file
"Evaluate `body` with `sym` bound to the path of a temporary file that
holds `content`. The file is created under /tmp/penpot, one of the two
places the upload validation accepts (/tmp/undertow is the other one),
and it is deleted when the body ends, also when it throws."
[[sym content] & body]
`(let [path# (fs/create-tempfile :dir "/tmp/penpot" :prefix "penpot-test-")]
(try
(spit (str path#) ~content :encoding "UTF-8")
(let [~sym path#]
~@body)
(finally
(when (fs/exists? path#)
(fs/delete path#))))))
(defn pause
[]
(let [^java.io.Console cnsl (System/console)]

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

@ -11,6 +11,7 @@
[app.media.svg :as svg]
[backend-tests.helpers :as th]
[clojure.test :as t]
[cuerdas.core :as str]
[datoteka.fs :as fs]))
(t/use-fixtures :once th/state-init)
@ -60,45 +61,43 @@
(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 (svg/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/is (not (str/includes? result "<script>")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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 (svg/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/is (not (str/includes? result "onload")))
(t/is (not (str/includes? result "onmouseover")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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 (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/is (not (str/includes? result "javascript:")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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 (svg/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/is (not (str/includes? result "foreignObject")))
(t/is (not (str/includes? result "<script>")))
(t/is (str/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 (svg/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/is (str/includes? result "<rect"))
(t/is (str/includes? result "<circle"))
(t/is (str/includes? result "fill=\"red\""))
(t/is (str/includes? result "fill=\"blue\"")))))
(t/deftest sanitize-svg-invalid-svg-rejected
(t/testing "sanitize-svg rejects malformed SVG input"
@ -110,98 +109,381 @@
(t/testing "sanitize-svg preserves legitimate xlink:href attributes"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"100\" height=\"100\"><use xlink:href=\"#icon\"/></svg>"
result (svg/sanitize-svg svg)]
(t/is (clojure.string/includes? result "xlink:href"))
(t/is (clojure.string/includes? result "#icon")))))
(t/is (str/includes? result "xlink:href"))
(t/is (str/includes? result "#icon")))))
(t/deftest sanitize-svg-javascript-href-whitespace
(t/testing "sanitize-svg catches javascript: URLs with leading whitespace"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><a 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/is (not (str/includes? result "javascript:")))
(t/is (not (str/includes? result "alert")))
(t/is (str/includes? result "<a")))))
(t/deftest sanitize-svg-nested-script
(t/testing "sanitize-svg removes script tags from nested elements"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><g><script>alert('xss')</script></g></svg>"
result (svg/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 "<g")))))
(t/is (not (str/includes? result "<script")))
(t/is (not (str/includes? result "alert")))
(t/is (str/includes? result "<g")))))
(t/deftest sanitize-svg-smil-bypass
(t/testing "sanitize-svg removes SMIL animation elements that can set on* attrs"
(let [svg "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><rect width=\"100\" height=\"100\" id=\"r\"/><set attributeName=\"onmouseover\" to=\"alert('xss')\" xlink:href=\"#r\" begin=\"0s\"/></svg>"
result (svg/sanitize-svg svg)]
(t/is (not (clojure.string/includes? result "<set")))
(t/is (not (clojure.string/includes? result "onmouseover")))
(t/is (clojure.string/includes? result "<rect")))))
(t/is (not (str/includes? result "<set")))
(t/is (not (str/includes? result "onmouseover")))
(t/is (str/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/is (not (str/includes? result "<x:script")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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/is (not (str/includes? result "<p:script")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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/is (not (str/includes? result "<x:script")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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/is (not (str/includes? result "foreignObject")))
(t/is (not (str/includes? result "<x:set")))
(t/is (not (str/includes? result "<x:animate")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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/is (not (str/includes? result "javascript:")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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/is (not (str/includes? result "onload")))
(t/is (not (str/includes? result "onclick")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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/is (not (str/includes? result "javascript:")))
(t/is (not (str/includes? result "alert")))
(t/is (str/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/is (str/includes? result "xml:space"))
(t/is (str/includes? result "hola")))))
;; --- SVG sanitization: escaping on serialization
;;
;; The sanitizer filters dangerous elements and attributes by name, so the
;; escaping happens when the tree is written back. A payload hidden in an
;; attribute value or in a text node must not become live markup again.
(def ^:private clean-svg
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"10\" height=\"10\"><circle r=\"4\"/></svg>")
(def ^:private evil-attribute-svg
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\" id=\"x' onload='alert(1)\"><rect width=\"10\" height=\"10\"/></svg>")
(def ^:private evil-text-svg
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><text>&lt;/text&gt;&lt;script&gt;alert(1)&lt;/script&gt;&lt;text&gt;</text></svg>")
(def ^:private evil-cdata-svg
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><text><![CDATA[</text><script>alert(1)</script><text>]]></text></svg>")
(def ^:private evil-javascript-svg
"An SVG with a javascript: URL obfuscated with a tab inside the scheme."
"<svg xmlns=\"http://www.w3.org/2000/svg\"><a href=\"java&#x09;script:alert(1)\">x</a></svg>")
(def ^:private xhtml-document
"A document that is not an SVG at all, with HTML only elements."
"<html xmlns=\"http://www.w3.org/1999/xhtml\"><body><iframe srcdoc=\"&lt;script&gt;alert(1)&lt;/script&gt;\"/><object data=\"data:text/html,x\"/><meta http-equiv=\"refresh\" content=\"0;url=javascript:alert(1)\"/></body></html>")
(t/deftest sanitize-svg-escapes-attribute-values
(t/testing "an event handler smuggled in an attribute value is escaped"
(let [result (svg/sanitize-svg evil-attribute-svg)
parsed (svg/parse-svg result)]
(t/is (not (contains? (:attrs parsed) :onload)))
(t/is (= "x' onload='alert(1)"
(get (:attrs parsed) :id)))
(t/is (str/includes? result "&apos;")))))
(t/deftest sanitize-svg-escapes-text-content
(t/testing "entity encoded markup in a text node is escaped"
(let [result (svg/sanitize-svg evil-text-svg)]
(t/is (not (str/includes? result "<script")))
(t/is (str/includes? result "&lt;script&gt;")))))
(t/deftest sanitize-svg-escapes-cdata-content
(t/testing "markup hidden in a CDATA node is escaped"
(let [result (svg/sanitize-svg evil-cdata-svg)]
(t/is (not (str/includes? result "<script")))
(t/is (str/includes? result "&lt;script&gt;")))))
(t/deftest sanitize-svg-escapes-ampersands
(t/testing "ampersands in text and attribute values are escaped"
(let [result (svg/sanitize-svg "<svg xmlns=\"http://www.w3.org/2000/svg\" id=\"a&amp;b\"><text>a &amp; b</text></svg>")]
(t/is (str/includes? result "a &amp; b"))
(t/is (= "a&b"
(get-in (svg/parse-svg result) [:attrs :id]))))))
(t/deftest sanitize-svg-output-is-valid-xml
(t/testing "the sanitized output can be parsed back"
(doseq [payload [clean-svg
evil-attribute-svg
evil-text-svg
evil-cdata-svg]]
(t/is (some? (svg/parse-svg (svg/sanitize-svg payload)))))))
(t/deftest sanitize-svg-rejects-non-svg-documents
(t/testing "a document that is not an svg document is rejected"
(doseq [payload [xhtml-document
;; an svg element declared in the xhtml namespace
"<svg xmlns=\"http://www.w3.org/1999/xhtml\"><rect/></svg>"
;; an svg element without any namespace
"<svg><rect/></svg>"]]
(let [error (try (svg/sanitize-svg payload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :invalid-svg-file))))))
(t/deftest sanitize-svg-rejects-non-svg-root-local-name
(t/testing "a root in the svg namespace whose local name is not svg is rejected"
(doseq [payload ["<rect xmlns=\"http://www.w3.org/2000/svg\" width=\"10\" height=\"10\"/>"
"<html xmlns=\"http://www.w3.org/2000/svg\"></html>"]]
(let [error (try (svg/sanitize-svg payload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-code? error :invalid-svg-file))))))
(t/deftest sanitize-svg-reports-non-svg-root
(t/testing "a root without the svg namespace reports it, not a serializer failure"
(let [error (try (svg/sanitize-svg "<svg><rect/></svg>") nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-code? error :invalid-svg-file))
(t/is (str/includes? (ex-message error) "not an svg document")))))
(t/deftest sanitize-svg-accepts-svg-roots
(t/testing "an svg root in the svg namespace is accepted"
(t/is (some? (svg/sanitize-svg "<svg xmlns=\"http://www.w3.org/2000/svg\"><rect/></svg>"))))
(t/testing "an svg root with a namespace prefix is accepted"
(t/is (some? (svg/sanitize-svg "<svg:svg xmlns:svg=\"http://www.w3.org/2000/svg\"><svg:rect/></svg:svg>")))))
(t/deftest sanitize-svg-upload-rejects-invalid-svg
(t/testing "the helper rejects an upload that does not hold a valid svg"
(doseq [content ["<svg xmlns=\"http://www.w3.org/2000/svg\"><rect>"
xhtml-document]]
(th/with-temp-file [path content]
(let [upload {:filename "evil.svg"
:path path
:mtype "image/svg+xml"
:size (fs/size path)}
error (try (svg/sanitize-svg-upload upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-code? error :invalid-svg-file)))))))
(t/deftest sanitize-svg-rejects-doctype
(t/testing "a document with a doctype and an internal subset is rejected"
(let [error (try (svg/sanitize-svg
"<!DOCTYPE svg [<!ENTITY x \"y\">]><svg xmlns=\"http://www.w3.org/2000/svg\"><text>&x;</text></svg>")
nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-code? error :invalid-svg-file)))))
(t/deftest sanitize-svg-escapes-double-quote
(t/testing "a double quote in an attribute value is escaped"
(let [result (svg/sanitize-svg "<svg xmlns=\"http://www.w3.org/2000/svg\" id=\"a&quot;b\"/>")
parsed (svg/parse-svg result)]
(t/is (str/includes? result "&quot;"))
(t/is (= "a\"b" (get (:attrs parsed) :id))))))
(t/deftest sanitize-svg-filters-prefixed-root-children
(t/testing "dangerous elements and attributes are removed inside a prefixed root"
(let [result (svg/sanitize-svg "<svg:svg xmlns:svg=\"http://www.w3.org/2000/svg\"><svg:script>alert(1)</svg:script><svg:rect onload=\"alert(1)\"/></svg:svg>")
parsed (svg/parse-svg result)]
(t/is (not (str/includes? result "script")))
(t/is (not (str/includes? result "onload")))
(t/is (= 1 (count (:content parsed)))))))
(t/deftest sanitize-svg-rejects-wrong-namespace
(t/testing "a root that is not in the svg namespace is rejected"
(doseq [payload ["<svg xmlns:foo=\"http://www.w3.org/2000/svg\"><rect/></svg>"
"<svg xmlns=\"\"><rect/></svg>"
"<svg xmlns=\" http://www.w3.org/2000/svg \"><rect/></svg>"
"<svg xmlns=\"http://www.w3.org/2000/svg&#9;\"><rect/></svg>"]]
(let [error (try (svg/sanitize-svg payload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-code? error :invalid-svg-file))))))
;; --- SVG sanitization: namespace in scope
;;
;; The XML parser does not process namespaces, so a child can rebind the
;; default namespace to XHTML and smuggle live HTML (iframe/srcdoc/meta).
;; The sanitizer drops any element that is not in the SVG namespace.
(def ^:private xhtml-child-svg
"An SVG whose root is in the SVG namespace but whose child rebinds the
default namespace to XHTML and embeds live HTML."
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"100\" height=\"100\"><g xmlns=\"http://www.w3.org/1999/xhtml\"><iframe srcdoc=\"&lt;script&gt;alert(1)&lt;/script&gt;\"/><meta http-equiv=\"refresh\" content=\"0;url=javascript:alert(1)\"/></g><rect width=\"10\" height=\"10\"/></svg>")
(t/deftest sanitize-svg-drops-non-svg-namespace-subtrees
(t/testing "a child that rebinds the default namespace to XHTML is dropped"
(let [result (svg/sanitize-svg xhtml-child-svg)]
(t/is (not (str/includes? result "iframe")))
(t/is (not (str/includes? result "meta")))
(t/is (not (str/includes? result "srcdoc")))
(t/is (not (str/includes? result "http-equiv")))
(t/is (str/includes? result "<rect")))))
(t/deftest sanitize-svg-drops-prefixed-non-svg-namespace-subtrees
(t/testing "a child whose prefix is bound to XHTML is dropped"
(let [result (svg/sanitize-svg
"<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:h=\"http://www.w3.org/1999/xhtml\"><h:iframe srcdoc=\"&lt;script&gt;alert(1)&lt;/script&gt;\"/></svg>")]
(t/is (not (str/includes? result "iframe")))
(t/is (not (str/includes? result "srcdoc"))))))
(t/deftest sanitize-svg-keeps-prefixed-svg-namespace-subtrees
(t/testing "a child whose prefix is bound to the SVG namespace is kept"
(let [result (svg/sanitize-svg
"<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:s=\"http://www.w3.org/2000/svg\"><s:rect width=\"10\" height=\"10\"/></svg>")]
(t/is (str/includes? result "<s:rect")))))
(def ^:private default-prefix-xhtml-svg
"An SVG whose root declares a prefix named `default` bound to XHTML."
"<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:default=\"http://www.w3.org/1999/xhtml\"><default:iframe srcdoc=\"&lt;script&gt;alert(1)&lt;/script&gt;\"/><default:meta http-equiv=\"refresh\" content=\"0;url=javascript:alert(1)\"/></svg>")
(t/deftest sanitize-svg-drops-default-prefix-xhtml-subtrees
(t/testing "a prefix named default bound to XHTML is not the default namespace"
(let [result (svg/sanitize-svg default-prefix-xhtml-svg)]
(t/is (not (str/includes? result "default:iframe")))
(t/is (not (str/includes? result "default:meta")))
(t/is (not (str/includes? result "srcdoc")))
(t/is (not (str/includes? result "http-equiv"))))))
(def ^:private xmlns-onload-svg
"An SVG that declares a prefix named like an event handler."
"<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:onload=\"http://www.w3.org/2000/svg\"><onload:rect width=\"10\" height=\"10\"/></svg>")
(t/deftest sanitize-svg-keeps-xmlns-declarations
(t/testing "a namespace declaration whose local name looks like an event handler is kept"
(let [result (svg/sanitize-svg xmlns-onload-svg)]
(t/is (str/includes? result "xmlns:onload"))
(t/is (str/includes? result "<onload:rect")))))
(t/deftest sanitize-svg-drops-html-only-tags
(t/testing "HTML-only elements written in the svg namespace are dropped"
(doseq [tag ["iframe" "meta" "embed" "link" "base"]]
(let [result (svg/sanitize-svg
(str "<svg xmlns=\"http://www.w3.org/2000/svg\"><" tag " src=\"https://evil\"/></svg>"))]
(t/is (not (str/includes? result (str "<" tag))))))))
(t/deftest sanitize-svg-javascript-href-control-char
(t/testing "a javascript: URL with a control char inside the scheme is removed"
(let [result (svg/sanitize-svg evil-javascript-svg)
parsed (svg/parse-svg result)]
(t/is (not (contains? (get-in parsed [:content 0 :attrs]) :href)))
(t/is (not (str/includes? result "script:alert")))))
(t/testing "a javascript: URL with a line break inside the scheme is removed"
(let [result (svg/sanitize-svg "<svg xmlns=\"http://www.w3.org/2000/svg\"><a href=\"java&#x0a;script:alert(1)\">x</a></svg>")]
(t/is (not (str/includes? result "script:alert"))))))
(t/deftest sanitize-svg-preserves-safe-href
(t/testing "a safe href is preserved"
(let [parsed (svg/parse-svg (svg/sanitize-svg "<svg xmlns=\"http://www.w3.org/2000/svg\"><a href=\"https://example.com/\">x</a></svg>"))]
(t/is (= "https://example.com/"
(get-in parsed [:content 0 :attrs :href]))))))
(t/deftest sanitize-svg-rejects-script-root
(t/testing "a document whose root is a script is rejected as not an svg document"
(let [error (try (svg/sanitize-svg "<script>alert(1)</script>") nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :invalid-svg-file))
(t/is (str/includes? (ex-message error) "not an svg document")))))
(t/deftest sanitize-svg-is-idempotent
(t/testing "sanitizing an already sanitized document does not change it"
(doseq [payload [clean-svg
evil-attribute-svg
evil-text-svg
evil-cdata-svg
evil-javascript-svg]]
(let [once (svg/sanitize-svg payload)]
(t/is (= once (svg/sanitize-svg once)))))))
(t/deftest get-basic-info-from-svg-accepts-prefixed-root
(t/testing "an svg root with a namespace prefix provides its dimensions"
(let [info (svg/get-basic-info-from-svg
(svg/parse-svg "<svg:svg xmlns:svg=\"http://www.w3.org/2000/svg\" width=\"120\" height=\"80\"/>"))]
(t/is (= 120 (:width info)))
(t/is (= 80 (:height info))))))
(t/deftest get-basic-info-from-svg-rejects-non-svg
(t/testing "a document whose root is not an svg element is rejected"
(let [error (try (svg/get-basic-info-from-svg
(svg/parse-svg "<html xmlns=\"http://www.w3.org/1999/xhtml\"/>"))
nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-code? error :unable-to-parse-svg)))))
(t/deftest sanitize-svg-upload-sanitizes-svg
(t/testing "an SVG upload is replaced by a sanitized copy"
(th/with-temp-file [path evil-attribute-svg]
(let [upload {:filename "evil.svg"
:path path
:mtype "image/svg+xml"
:size (fs/size path)}
result (svg/sanitize-svg-upload upload)
parsed (svg/parse-svg (slurp (str (:path result))))]
(t/is (not= path (:path result)))
(t/is (not (contains? (:attrs parsed) :onload)))
(t/is (= "x' onload='alert(1)"
(get (:attrs parsed) :id)))
(t/is (= (fs/size (:path result)) (:size result)))))))
(t/deftest sanitize-svg-upload-passes-non-svg
(t/testing "an upload that is not an SVG is returned untouched"
(let [path (th/tempfile "backend_tests/test_files/sample.jpg")
upload {:filename "sample.jpg"
:path path
:mtype "image/jpeg"
:size 312043}]
(t/is (= upload (svg/sanitize-svg-upload upload))))))
(t/deftest info-invalid-image
(t/testing "info on invalid image raises error"

32
backend/test/backend_tests/rpc_auth_test.clj
View File

@ -14,9 +14,13 @@
[app.http.session :as session]
[app.loggers.audit :as-alias audit]
[app.redis :as rds]
[app.rpc.commands.auth :as auth]
[app.rpc.commands.ldap :as ldap-cmd]
[app.storage :as sto]
[backend-tests.helpers :as th]
[clojure.test :as t]
[datoteka.io :as io]
[mockery.core :refer [with-mocks]]
[yetti.response :as yres])
(:import
java.lang.AutoCloseable))
@ -253,3 +257,31 @@
"session must not be bound to the typed-email profile")))))
;; --- Profile picture import
(def ^:private avatar-svg
"An SVG that smuggles a script element."
"<svg xmlns=\"http://www.w3.org/2000/svg\"><script>alert(1)</script></svg>")
(defn- mock-download
"Mock the http client so that any url answers with `body` and `mtype`."
[mtype body]
{:target 'app.http.client/req-with-redirects
:return {:status 200
:headers {"content-type" mtype
"content-length" (str (alength ^bytes body))}
:body (java.io.ByteArrayInputStream. ^bytes body)}})
(t/deftest import-profile-picture-rejects-svg
(t/testing "an svg profile picture is not imported"
(with-mocks [http-mock (mock-download "image/svg+xml"
(.getBytes ^String avatar-svg "UTF-8"))]
(let [result (auth/import-profile-picture th/*system* "https://example.com/avatar.svg")]
(t/is (nil? result)))))
(t/testing "a png profile picture is imported"
(let [body (io/read* (io/resource "backend_tests/test_files/sample.png"))]
(with-mocks [http-mock (mock-download "image/png" body)]
(let [result (auth/import-profile-picture th/*system* "https://example.com/avatar.png")]
(t/is (some? result))
(t/is (some? (sto/get-object (:app.storage/storage th/*system*) result))))))))

69
backend/test/backend_tests/rpc_file_thumbnails_test.clj
View File

@ -13,6 +13,7 @@
[app.common.uuid :as uuid]
[app.config :as cf]
[app.db :as db]
[app.media.svg :as svg]
[app.rpc :as-alias rpc]
[app.rpc.commands.auth :as cauth]
[app.setup.clock :as clock]
@ -28,6 +29,17 @@
(t/use-fixtures :once th/state-init)
(t/use-fixtures :each th/database-reset)
(def ^:private evil-svg
"An SVG that smuggles an event handler inside an attribute value."
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"64\" height=\"64\" id=\"x' onload='alert(1)\"><rect width=\"10\" height=\"10\"/></svg>")
(defn- stored-svg-attrs
"Return the attributes of the SVG stored in the storage object `id`."
[storage id]
(let [sobject (sto/get-object storage id)
content (slurp (sto/get-object-data storage sobject))]
(:attrs (svg/parse-svg content))))
(t/deftest upsert-file-object-thumbnail
(let [storage (::sto/storage th/*system*)
profile (th/create-profile* 1)
@ -254,6 +266,63 @@
(t/is (some? (sto/get-object storage (:media-id row2)))))))
(t/deftest create-file-object-thumbnail-sanitizes-svg
(let [storage (::sto/storage th/*system*)
profile (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})]
(th/with-temp-file [path evil-svg]
(let [data {::th/type :create-file-object-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:object-id "test-key-svg"
:media {:filename "evil.svg"
:size (fs/size path)
:path path
:mtype "image/svg+xml"}}]
(let [out (th/multipart-command! data)]
(t/is (nil? (:error out))))
(let [[row] (th/db-query :file-tagged-object-thumbnail
{:file-id (:id file)}
{:order-by [[:created-at :asc]]})]
(t/is (some? row))
(let [attrs (stored-svg-attrs storage (:media-id row))]
(t/is (not (contains? attrs :onload)))
(t/is (= "x' onload='alert(1)" (get attrs :id)))))))))
(t/deftest create-file-thumbnail-sanitizes-svg
(let [storage (::sto/storage th/*system*)
profile (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false
:revn 1})]
(th/with-temp-file [path evil-svg]
(let [data {::th/type :create-file-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:revn 1
:media {:filename "evil.svg"
:size (fs/size path)
:path path
:mtype "image/svg+xml"}}]
(let [out (th/multipart-command! data)]
(t/is (nil? (:error out))))
(let [[row] (th/db-query :file-thumbnail
{:file-id (:id file)}
{:order-by [[:revn :asc]]})]
(t/is (some? row))
(let [attrs (stored-svg-attrs storage (:media-id row))]
(t/is (not (contains? attrs :onload)))
(t/is (= "x' onload='alert(1)" (get attrs :id)))))))))
(t/deftest create-file-thumbnail-requires-edit-permissions
(let [owner (th/create-profile* 1)
viewer (th/create-profile* 2)

43
backend/test/backend_tests/rpc_management_nitrate_test.clj
View File

@ -14,9 +14,11 @@
[app.db :as db]
[app.email :as eml]
[app.http :as-alias http]
[app.media.svg :as svg]
[app.msgbus :as mbus]
[app.nitrate :as nitrate]
[app.rpc :as-alias rpc]
[app.storage :as sto]
[app.util.ssrf :as ssrf]
[app.worker :as wrk]
[backend-tests.helpers :as th]
@ -2086,3 +2088,44 @@
:size 123}}
out (th/management-command! params)]
(t/is (th/ex-of-code? (:error out) :params-validation)))))
(t/deftest upload-organization-logo-sanitizes-svg
;; The logo lands in a public bucket, so an SVG with live markup must
;; be sanitized before it is stored.
(with-mocks [nitrate-mock {:target 'app.nitrate/call :return nil}]
(let [profile (th/create-profile* 1 {:is-active true})
content "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"64\" height=\"64\" id=\"x' onload='alert(1)\"><rect width=\"10\" height=\"10\"/></svg>"]
(th/with-temp-file [path content]
(let [params {::th/type :upload-organization-logo
::rpc/profile-id (:id profile)
:organization-id (uuid/random)
:content {:filename "evil.svg"
:path path
:mtype "image/svg+xml"
:size (fs/size path)}}
out (th/multipart-management-command! params)]
(t/is (th/success? out))
(let [storage (:app.storage/storage th/*system*)
sobject (sto/get-object storage (get-in out [:result :id]))
stored (slurp (sto/get-object-data storage sobject))
parsed (svg/parse-svg stored)]
(t/is (not (contains? (:attrs parsed) :onload)))
(t/is (= "x' onload='alert(1)" (get (:attrs parsed) :id)))))))))
(t/deftest upload-organization-logo-rejects-oversized-file
;; The sanitizer reads the whole logo, so the logo must honor the media
;; size limit before it is read.
(with-mocks [nitrate-mock {:target 'app.nitrate/call :return nil}]
(let [profile (th/create-profile* 1 {:is-active true})]
(th/with-temp-file [path "<svg xmlns=\"http://www.w3.org/2000/svg\"><rect/></svg>"]
(with-redefs [app.config/config (assoc app.config/config :media-max-file-size 1)]
(let [params {::th/type :upload-organization-logo
::rpc/profile-id (:id profile)
:organization-id (uuid/random)
:content {:filename "big.svg"
:path path
:mtype "image/svg+xml"
:size (fs/size path)}}
out (th/multipart-management-command! params)]
(t/is (th/ex-of-type? (:error out) :restriction))
(t/is (th/ex-of-code? (:error out) :media-max-file-size-reached))))))))

127
backend/test/backend_tests/rpc_media_test.clj
View File

@ -10,10 +10,12 @@
[app.db :as db]
[app.http.client :as http]
[app.media :as media]
[app.media.svg :as svg]
[app.rpc :as-alias rpc]
[app.storage :as sto]
[backend-tests.helpers :as th]
[clojure.test :as t]
[cuerdas.core :as str]
[datoteka.fs :as fs]
[datoteka.io :as io]
[mockery.core :refer [with-mocks]])
@ -23,6 +25,14 @@
(t/use-fixtures :once th/state-init)
(t/use-fixtures :each th/database-reset)
(def ^:private evil-svg
"An SVG that smuggles an event handler inside an attribute value."
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"64\" height=\"64\" id=\"x' onload='alert(1)\"><rect width=\"10\" height=\"10\"/></svg>")
(def ^:private evil-text-svg
"An SVG that smuggles a script element inside a text node."
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"64\" height=\"64\"><text>&lt;/text&gt;&lt;script&gt;alert(1)&lt;/script&gt;</text></svg>")
(t/deftest media-object-from-url
(let [prof (th/create-profile* 1)
proj (th/create-project* 1 {:profile-id (:id prof)
@ -94,6 +104,123 @@
(t/is (= 3890 (:size mobj2)))))))
(t/deftest upload-file-media-object-sanitizes-svg
(let [prof (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id prof)
:project-id (:default-project-id prof)
:is-shared false})]
(th/with-temp-file [path evil-svg]
(let [params {::th/type :upload-file-media-object
::rpc/profile-id (:id prof)
:file-id (:id file)
:is-local true
:name "evil.svg"
:content {:filename "evil.svg"
:path path
:mtype "image/svg+xml"
:size (fs/size path)}}
out (th/multipart-command! params)]
(t/is (nil? (:error out)))
(let [{:keys [media-id]} (:result out)
storage (:app.storage/storage th/*system*)
sobject (sto/get-object storage media-id)
content (slurp (sto/get-object-data storage sobject))
parsed (svg/parse-svg content)]
(t/is (not (contains? (:attrs parsed) :onload)))
(t/is (= "x' onload='alert(1)" (get (:attrs parsed) :id)))
(t/is (str/includes? content "&apos;")))))))
(t/deftest upload-file-media-object-sanitizes-svg-text
(let [prof (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id prof)
:project-id (:default-project-id prof)
:is-shared false})]
(th/with-temp-file [path evil-text-svg]
(let [params {::th/type :upload-file-media-object
::rpc/profile-id (:id prof)
:file-id (:id file)
:is-local true
:name "evil.svg"
:content {:filename "evil.svg"
:path path
:mtype "image/svg+xml"
:size (fs/size path)}}
out (th/multipart-command! params)]
(t/is (nil? (:error out)))
(let [{:keys [media-id]} (:result out)
storage (:app.storage/storage th/*system*)
sobject (sto/get-object storage media-id)
content (slurp (sto/get-object-data storage sobject))
parsed (svg/parse-svg content)]
(t/is (not (str/includes? content "<script")))
(t/is (= "</text><script>alert(1)</script>"
(get-in parsed [:content 0 :content 0]))))))))
(t/deftest create-file-media-object-from-url-sanitizes-svg
(let [prof (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id prof)
:project-id (:default-project-id prof)
:is-shared false})
body (.getBytes ^String evil-svg "UTF-8")]
(with-mocks [http-mock {:target 'app.http.client/req-with-redirects
:return {:status 200
:headers {"content-type" "image/svg+xml"
"content-length" (str (alength ^bytes body))}
:body (java.io.ByteArrayInputStream. ^bytes body)}}]
(let [params {::th/type :create-file-media-object-from-url
::rpc/profile-id (:id prof)
:file-id (:id file)
:is-local true
:url "https://example.com/evil.svg"}
out (th/command! params)]
(t/is (nil? (:error out)))
(let [{:keys [media-id]} (:result out)
storage (:app.storage/storage th/*system*)
sobject (sto/get-object storage media-id)
content (slurp (sto/get-object-data storage sobject))
parsed (svg/parse-svg content)]
(t/is (not (contains? (:attrs parsed) :onload)))
(t/is (not (str/includes? content "<script>")))
(t/is (= "x' onload='alert(1)" (get (:attrs parsed) :id)))
(t/is (str/includes? content "&apos;")))))))
(def ^:private xhtml-child-svg
"An SVG whose root is in the SVG namespace but whose child rebinds the
default namespace to XHTML and embeds live HTML."
"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"64\" height=\"64\"><g xmlns=\"http://www.w3.org/1999/xhtml\"><iframe srcdoc=\"&lt;script&gt;alert(1)&lt;/script&gt;\"/><meta http-equiv=\"refresh\" content=\"0;url=javascript:alert(1)\"/></g><rect width=\"10\" height=\"10\"/></svg>")
(t/deftest create-file-media-object-from-url-sanitizes-xhtml-child
(let [prof (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id prof)
:project-id (:default-project-id prof)
:is-shared false})
body (.getBytes ^String xhtml-child-svg "UTF-8")]
(with-mocks [http-mock {:target 'app.http.client/req-with-redirects
:return {:status 200
:headers {"content-type" "image/svg+xml"
"content-length" (str (alength ^bytes body))}
:body (java.io.ByteArrayInputStream. ^bytes body)}}]
(let [params {::th/type :create-file-media-object-from-url
::rpc/profile-id (:id prof)
:file-id (:id file)
:is-local true
:url "https://example.com/evil.svg"}
out (th/command! params)]
(t/is (nil? (:error out)))
(let [{:keys [media-id]} (:result out)
storage (:app.storage/storage th/*system*)
sobject (sto/get-object storage media-id)
content (slurp (sto/get-object-data storage sobject))]
(t/is (not (str/includes? content "iframe")))
(t/is (not (str/includes? content "srcdoc")))
(t/is (not (str/includes? content "http-equiv"))))))))
(t/deftest media-object-from-url-command
(let [prof (th/create-profile* 1)
proj (th/create-project* 1 {:profile-id (:id prof)