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

🐛 Confine upload paths to temp dirs (#12772)

Browse Source 
* 🐛 Confine upload paths to temp dirs

Reject upload maps whose path leaves the server temp dirs.
Transit bodies can forge a Path such as /etc/passwd and the
handlers open it directly. Centralize the realpath prefix
check in media validation and apply it to all nine upload
fields. Legit multipart files under /tmp/undertow/ or
/tmp/penpot/ still pass.

AI-assisted-by: muse-spark-1.3-contributor

* ♻️ Rename upload validators to check-*

Drop the validate prefix and the bang from the media
validation helpers for a consistent check naming. Move
the temp dir confinement into validation and cover all
nine upload fields. Add unit tests for each helper.

AI-assisted-by: muse-spark-1.3-contributor

* 🐛 Fix svg upload tests to use penpot tmp dir

SVG upload tests wrote tempfiles to the system tmp dir,
outside the allowed upload prefixes, so check-upload-path
rejected them. Point write-temp-svg at /tmp/penpot like
the other upload tests.

AI-assisted-by: muse-spark-1.3-contributor
This commit is contained in:
Andrey Antukh 2026-11-03 12:07:52 +01:00 committed by GitHub
parent 41be5bfb5f
commit afe6c1b99c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 162 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
backend/src/app/media.clj
View File

@ -59,8 +59,8 @@
:hint "seems like the url points to resource with unknown size"))
(-> {:size size :mtype mtype}
(validation/validate-media-type!)
(validation/validate-media-size!))))]
(validation/check-media-type)
(validation/check-media-size))))]
(let [{:keys [body] :as response}
(try

28
backend/src/app/media/validation.clj
View File

@ -32,8 +32,26 @@
(def check-input
(sm/check-fn schema:input))
(defn validate-media-type!
([upload] (validate-media-type! upload cm/image-types))
(def allowed-upload-prefixes
;; Upload temp files are created server-side by the multipart parser
;; (yetti `default-temp-dir`, `/tmp/undertow/`) plus the tmp dir used
;; by tests and storage tmp files (`/tmp/penpot/`). Anything else
;; (e.g. `/etc/passwd` smuggled via Transit `~#path`) is rejected.
["/tmp/undertow/" "/tmp/penpot/"])
(defn check-upload-path
[upload]
(let [real (try (str (fs/real (:path upload)))
(catch Exception _ nil))]
(when (or (nil? real)
(not (some #(str/starts-with? real %) allowed-upload-prefixes)))
(ex/raise :type :validation
:code :params-validation
:hint "invalid upload path"))
upload))
(defn check-media-type
([upload] (check-media-type upload cm/image-types))
([upload allowed]
(when-not (contains? allowed (:mtype upload))
(ex/raise :type :validation
@ -42,7 +60,7 @@
upload))
(defn validate-media-size!
(defn check-media-size
[upload]
(let [max-size (cf/get :media-max-file-size)]
(when (> (:size upload) max-size)
@ -53,10 +71,10 @@
max-size)))
upload))
(defn validate-font-size!
(defn check-font-size
"Validates that the font file `upload` does not exceed the configured
`:font-max-file-size` limit. Accepts the same map shape as
`validate-media-size!` — requires a `:size` key in bytes."
`check-media-size` — requires a `:size` key in bytes."
[upload]
(let [max-size (cf/get :font-max-file-size)]
(when (> (:size upload) max-size)

2
backend/src/app/rpc/commands/binfile.clj
View File

@ -159,6 +159,8 @@
(assoc params :file file))
params)
_ (media.v/check-upload-path (:file params))
version (or version
(case (bfc/parse-file-format (-> params :file :path))
:binfile-v1 1

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

@ -289,8 +289,9 @@
::sm/params schema:create-file-object-thumbnail}
[cfg {:keys [::rpc/profile-id file-id object-id media tag]}]
(media.v/validate-media-type! media)
(media.v/validate-media-size! media)
(media.v/check-media-type media)
(media.v/check-media-size media)
(media.v/check-upload-path media)
(db/run! cfg files/check-edition-permissions! profile-id file-id)
(when-let [file (files/get-minimal-file cfg file-id {::db/check-deleted false})]
@ -394,8 +395,9 @@
::sm/params schema:create-file-thumbnail}
[cfg {:keys [::rpc/profile-id file-id] :as params}]
(media.v/validate-media-type! (:media params))
(media.v/validate-media-size! (:media params))
(media.v/check-media-type (:media params))
(media.v/check-media-size (:media params))
(media.v/check-upload-path (:media params))
(db/run! cfg files/check-edition-permissions! profile-id file-id)

4
backend/src/app/rpc/commands/fonts.clj
View File

@ -123,8 +123,8 @@
(fn [acc mtype session-id]
(let [assembled (assemble-chunks cfg profile-id session-id)]
(-> {:mtype mtype :size (:size assembled)}
(media.v/validate-media-type! cm/font-types)
(media.v/validate-font-size!))
(media.v/check-media-type cm/font-types)
(media.v/check-font-size))
(assoc acc mtype (:path assembled))))
{}
uploads)]

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

@ -63,8 +63,9 @@
[:process-image/global]]}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id file-id content] :as params}]
(files/check-edition-permissions! pool profile-id file-id)
(media.v/validate-media-type! content)
(media.v/validate-media-size! content)
(media.v/check-media-type content)
(media.v/check-media-size content)
(media.v/check-upload-path content)
;; TEMPORARILY DISABLED (2026-10-19): media-storage-bytes-per-team
;; quote performs a full SUM over storage_object per upload (3-12s in
@ -365,6 +366,7 @@
::sm/params schema:upload-chunk
::sm/result schema:upload-chunk-result}
[cfg {:keys [::rpc/profile-id session-id index content]}]
(media.v/check-upload-path content)
(let [session (db/tx-run! cfg check-upload-chunk-slot session-id profile-id index content)]
(l/trc :hint "upload-chunk"
:session-id session-id
@ -513,8 +515,8 @@
content (-> content
(assoc :filename (str "upload:" name))
(assoc :mtype mtype)
(media.v/validate-media-type!)
(media.v/validate-media-size!))
(media.v/check-media-type)
(media.v/check-media-size))
mobj (create-file-media-object cfg (assoc params
:id id
:from-chunks? true

5
backend/src/app/rpc/commands/profile.clj
View File

@ -297,8 +297,9 @@
::sm/result :nil}
[cfg {:keys [::rpc/profile-id file] :as params}]
;; Validate incoming mime type
(media.v/validate-media-type! file #{"image/jpeg" "image/png" "image/webp"})
(media.v/validate-media-size! file)
(media.v/check-media-type file #{"image/jpeg" "image/png" "image/webp"})
(media.v/check-media-size file)
(media.v/check-upload-path file)
(update-profile-photo cfg (assoc params :profile-id profile-id)))
(defn update-profile-photo

5
backend/src/app/rpc/commands/teams.clj
View File

@ -1032,8 +1032,9 @@
[cfg {:keys [::rpc/profile-id file] :as params}]
;; Validate incoming mime type
(media.v/validate-media-type! file #{"image/jpeg" "image/png" "image/webp"})
(media.v/validate-media-size! file)
(media.v/check-media-type file #{"image/jpeg" "image/png" "image/webp"})
(media.v/check-media-size file)
(media.v/check-upload-path file)
(update-team-photo cfg (assoc params :profile-id profile-id)))
(defn update-team-photo

3
backend/src/app/rpc/management/exporter.clj
View File

@ -33,7 +33,8 @@
::sm/params schema:upload-tempfile-params
::sm/result schema:upload-tempfile-result}
[cfg {:keys [::rpc/profile-id content]}]
(media.v/validate-media-type! content cm/tempfile-types)
(media.v/check-media-type content cm/tempfile-types)
(media.v/check-upload-path content)
(let [storage (sto/resolve cfg)
hash (sto/calculate-hash (:path content))
data (-> (sto/content (:path content))

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

@ -138,7 +138,8 @@
::sm/result schema:upload-organization-logo-result
::nitrate/sso false}
[{:keys [::sto/storage]} {:keys [content organization-id previous-id]}]
(media.v/validate-media-type! content cm/image-types)
(media.v/check-media-type content cm/image-types)
(media.v/check-upload-path content)
(when previous-id
(sto/touch-object! storage previous-id))
(let [hash (sto/calculate-hash (:path content))

2
backend/test/backend_tests/binfile_svg_test.clj
View File

@ -146,7 +146,7 @@
(defn- write-temp-svg
"Write `text` to a tempfile and return `[path size]`."
[text]
(let [path (fs/create-tempfile :prefix "penpot-svg-import-test-" :suffix ".svg")
(let [path (fs/create-tempfile :dir "/tmp/penpot" :prefix "penpot-svg-import-test-" :suffix ".svg")
data (utf8bytes text)]
(spit (str path) text :encoding "UTF-8")
[path (alength data)]))

68
backend/test/backend_tests/media_validation_test.clj Normal file
View File

@ -0,0 +1,68 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS INC Sucursal en España SL
(ns backend-tests.media-validation-test
(:require
[app.media.validation :as media.v]
[backend-tests.helpers :as th]
[clojure.test :as t]
[datoteka.fs :as fs]))
(t/deftest check-media-type
(t/testing "allowed mtype passes through"
(let [upload {:filename "a.png" :size 1 :path (fs/path "/tmp/penpot/a.png") :mtype "image/png"}]
(t/is (= upload (media.v/check-media-type upload)))))
(t/testing "disallowed mtype is rejected"
(let [upload {:filename "a.txt" :size 1 :path (fs/path "/tmp/penpot/a.txt") :mtype "text/plain"}
error (try (media.v/check-media-type upload) 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 :media-type-not-allowed)))))
(t/deftest check-media-size
(t/testing "small file passes through"
(let [upload {:filename "a.png" :size 1 :path (fs/path "/tmp/penpot/a.png") :mtype "image/png"}]
(t/is (= upload (media.v/check-media-size upload)))))
(t/testing "oversized file is rejected"
(let [upload {:filename "a.png" :size (* 1024 1024 31) :path (fs/path "/tmp/penpot/a.png") :mtype "image/png"}
error (try (media.v/check-media-size upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :restriction))
(t/is (th/ex-of-code? error :media-max-file-size-reached)))))
(t/deftest check-font-size
(t/testing "small font passes through"
(let [upload {:filename "a.ttf" :size 1 :path (fs/path "/tmp/penpot/a.ttf") :mtype "font/ttf"}]
(t/is (= upload (media.v/check-font-size upload)))))
(t/testing "oversized font is rejected"
(let [upload {:filename "a.ttf" :size (* 1024 1024 31) :path (fs/path "/tmp/penpot/a.ttf") :mtype "font/ttf"}
error (try (media.v/check-font-size upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :restriction))
(t/is (th/ex-of-code? error :font-max-file-size-reached)))))
(t/deftest check-upload-path
(t/testing "tempfile under /tmp/penpot passes through"
(let [path (th/tempfile "backend_tests/test_files/sample.jpg")
upload {:filename "sample.jpg" :size 7923 :path path :mtype "image/jpeg"}]
(t/is (= upload (media.v/check-upload-path upload)))))
(t/testing "system file outside temp is rejected"
(let [upload {:filename "x.png" :size 1 :path (fs/path "/etc/passwd") :mtype "image/png"}
error (try (media.v/check-upload-path upload) 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 :params-validation))))
(t/testing "missing file is rejected"
(let [upload {:filename "x.png" :size 1 :path (fs/path "/tmp/penpot/no-such-file-12345.png") :mtype "image/png"}
error (try (media.v/check-upload-path upload) 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 :params-validation)))))

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

@ -915,3 +915,45 @@
(t/is (= 1 (count rows2)))
(t/is (nil? (:deleted-at (first rows2)))))))
(t/deftest rejects-forged-path-on-create-file-thumbnail
(let [profile (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})
data {::th/type :create-file-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:revn 1
:media {:filename "x.png"
:size 1
:path (fs/path "/etc/passwd")
:mtype "image/png"}}
out (th/command! data)
error (:error out)]
(t/is (nil? (:result out)))
(t/is (th/ex-info? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :params-validation))
(t/is (= 0 (count (th/db-query :file-thumbnail {:file-id (:id file)}))))))
(t/deftest rejects-forged-path-on-create-file-object-thumbnail
(let [profile (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})
data {::th/type :create-file-object-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:object-id "test-key-forged"
:media {:filename "x.png"
:size 1
:path (fs/path "/etc/passwd")
:mtype "image/png"}}
out (th/command! data)
error (:error out)]
(t/is (nil? (:result out)))
(t/is (th/ex-info? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :params-validation))
(t/is (= 0 (count (th/db-query :file-tagged-object-thumbnail {:file-id (:id file)}))))))