diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index dc23e3483c..ae20de7502 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -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 diff --git a/backend/src/app/media/validation.clj b/backend/src/app/media/validation.clj index 17dbd80e71..dc657de647 100644 --- a/backend/src/app/media/validation.clj +++ b/backend/src/app/media/validation.clj @@ -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) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index fee8a98557..720ed512f9 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -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 diff --git a/backend/src/app/rpc/commands/files_thumbnails.clj b/backend/src/app/rpc/commands/files_thumbnails.clj index f4d9b538cb..519c7b5002 100644 --- a/backend/src/app/rpc/commands/files_thumbnails.clj +++ b/backend/src/app/rpc/commands/files_thumbnails.clj @@ -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) diff --git a/backend/src/app/rpc/commands/fonts.clj b/backend/src/app/rpc/commands/fonts.clj index 4ab02627bc..701e35d934 100644 --- a/backend/src/app/rpc/commands/fonts.clj +++ b/backend/src/app/rpc/commands/fonts.clj @@ -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)] diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index ce75f53994..1b66f5b6b7 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -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 diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 572dcc62a4..0571df5966 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -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 diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 1a329773f2..6d4d624c5a 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -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 diff --git a/backend/src/app/rpc/management/exporter.clj b/backend/src/app/rpc/management/exporter.clj index f6cdbdd820..1de20c7150 100644 --- a/backend/src/app/rpc/management/exporter.clj +++ b/backend/src/app/rpc/management/exporter.clj @@ -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)) diff --git a/backend/src/app/rpc/management/nitrate.clj b/backend/src/app/rpc/management/nitrate.clj index 4224ca7264..0f4c8e28e0 100644 --- a/backend/src/app/rpc/management/nitrate.clj +++ b/backend/src/app/rpc/management/nitrate.clj @@ -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)) diff --git a/backend/test/backend_tests/binfile_svg_test.clj b/backend/test/backend_tests/binfile_svg_test.clj index 8b63de4ab8..d3affc5b74 100644 --- a/backend/test/backend_tests/binfile_svg_test.clj +++ b/backend/test/backend_tests/binfile_svg_test.clj @@ -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)])) diff --git a/backend/test/backend_tests/media_validation_test.clj b/backend/test/backend_tests/media_validation_test.clj new file mode 100644 index 0000000000..f4431e1b71 --- /dev/null +++ b/backend/test/backend_tests/media_validation_test.clj @@ -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))))) diff --git a/backend/test/backend_tests/rpc_file_thumbnails_test.clj b/backend/test/backend_tests/rpc_file_thumbnails_test.clj index 4fb4ab12e1..ce41d828eb 100644 --- a/backend/test/backend_tests/rpc_file_thumbnails_test.clj +++ b/backend/test/backend_tests/rpc_file_thumbnails_test.clj @@ -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)})))))) +