From 7807e341517f23ca431477d9fda819900a6edd3e Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 28 Jul 2026 09:06:41 +0000 Subject: [PATCH] :bug: Scope assemble-chunks session lookup to profile-id Prevent BOLA in chunked upload assembly by verifying session ownership. The assemble-chunks function now requires a profile-id parameter and scopes the upload_session lookup accordingly, matching the pattern already used by upload-chunk. All three callers (assemble-file-media-object, create-font-variant, import-binfile) updated to pass the authenticated profile-id. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/rpc/commands/binfile.clj | 2 +- backend/src/app/rpc/commands/fonts.clj | 4 +-- backend/src/app/rpc/commands/media.clj | 7 ++-- .../test/backend_tests/rpc_binfile_test.clj | 8 ++--- backend/test/backend_tests/rpc_media_test.clj | 35 +++++++++++++++++++ 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 9c852e94ab..a900d52203 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -152,7 +152,7 @@ params (if (some? upload-id) - (let [file (db/tx-run! cfg media-cmd/assemble-chunks upload-id)] + (let [file (db/tx-run! cfg media-cmd/assemble-chunks profile-id upload-id)] (assoc params :file file)) params) diff --git a/backend/src/app/rpc/commands/fonts.clj b/backend/src/app/rpc/commands/fonts.clj index 4d9eb77636..2ad9a57216 100644 --- a/backend/src/app/rpc/commands/fonts.clj +++ b/backend/src/app/rpc/commands/fonts.clj @@ -113,10 +113,10 @@ "Assembles each chunked-upload session in `uploads` (a `{mtype → session-id}` map) into a temp file, validates the media type and size of every entry, and returns a `{mtype → path}` data map." - [cfg {:keys [uploads] :as params}] + [cfg {:keys [::rpc/profile-id uploads] :as params}] (let [data (reduce-kv (fn [acc mtype session-id] - (let [assembled (assemble-chunks cfg session-id)] + (let [assembled (assemble-chunks cfg profile-id session-id)] (-> {:mtype mtype :size (:size assembled)} (media/validate-media-type! cm/font-types) (media/validate-font-size!)) diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index ff8add456a..11057a846b 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -391,9 +391,10 @@ Raises a :validation/:missing-chunks error when the number of stored chunks does not match `:total-chunks` recorded in the session row. + Raises :not-found when the session does not belong to `profile-id`. Deletes the session row from `upload_session` on success." - [{:keys [::db/conn] :as cfg} session-id] - (let [session (db/get conn :upload-session {:id session-id}) + [{:keys [::db/conn] :as cfg} profile-id session-id] + (let [session (db/get conn :upload-session {:id session-id :profile-id profile-id}) chunks (get-upload-chunks conn session-id)] (when (not= (count chunks) (:total-chunks session)) @@ -436,7 +437,7 @@ (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (let [content (assemble-chunks cfg session-id) + (let [content (assemble-chunks cfg profile-id session-id) content (-> content (assoc :filename (str "upload:" name)) (assoc :mtype mtype) diff --git a/backend/test/backend_tests/rpc_binfile_test.clj b/backend/test/backend_tests/rpc_binfile_test.clj index ca78dca95c..5ebf83bf18 100644 --- a/backend/test/backend_tests/rpc_binfile_test.clj +++ b/backend/test/backend_tests/rpc_binfile_test.clj @@ -22,20 +22,20 @@ ;; The schema should not accept file-id as a valid parameter (let [schema @#'binfile/schema:import-binfile validator (sm/lazy-validator schema) - + ;; Valid params without file-id valid-params {:name "test" :project-id (uuid/random) :version 3 :upload-id (uuid/random)} - + ;; Params with file-id (should be rejected after fix) params-with-file-id (assoc valid-params :file-id (uuid/random))] - + ;; Valid params without file-id should pass (t/is (true? (validator valid-params)) "params without file-id should be valid") - + ;; Params with file-id should fail validation after fix ;; (Currently this will fail because file-id is still in schema) (t/is (false? (validator params-with-file-id)) diff --git a/backend/test/backend_tests/rpc_media_test.clj b/backend/test/backend_tests/rpc_media_test.clj index ff38aee470..75f8adccfe 100644 --- a/backend/test/backend_tests/rpc_media_test.clj +++ b/backend/test/backend_tests/rpc_media_test.clj @@ -548,6 +548,41 @@ (t/is (some? (:error out))) (t/is (= :not-found (-> out :error ex-data :type))))) +(t/deftest chunked-upload-other-profile-cannot-assemble + ;; assemble-chunks must scope the session lookup to the requesting + ;; profile so that a different profile cannot assemble chunks from + ;; a session they do not own (BOLA / CWE-639). + (let [prof1 (th/create-profile* 1) + prof2 (th/create-profile* 2) + session-id (create-session! prof1 1) + source-path (th/tempfile "backend_tests/test_files/sample.jpg") + mfile {:filename "sample.jpg" + :path source-path + :mtype "image/jpeg" + :size 312043}] + + ;; prof1 uploads a chunk into their own session + (let [out (th/command! {::th/type :upload-chunk + ::rpc/profile-id (:id prof1) + :session-id session-id + :index 0 + :content mfile})] + (t/is (nil? (:error out)))) + + ;; prof2 tries to assemble prof1's session via create-font-variant + ;; (which calls assemble-chunks without ownership check) + (let [out (th/command! {::th/type :create-font-variant + ::rpc/profile-id (:id prof2) + :team-id (:default-team-id prof2) + :font-id (uuid/next) + :font-family "TestFont" + :font-weight 400 + :font-style "normal" + :uploads {"font/ttf" session-id}})] + (t/is (some? (:error out))) + (t/is (= :not-found (-> out :error ex-data :type))) + (t/is (= :object-not-found (-> out :error ex-data :code)))))) + (t/deftest chunked-upload-invalid-media-type (let [prof (th/create-profile* 1) _ (th/create-project* 1 {:profile-id (:id prof)