diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index eab49e1eb3..436be3b943 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -42,6 +42,7 @@ [datoteka.io :as io]) (:import java.io.File + java.io.FilterInputStream java.io.InputStream java.io.OutputStreamWriter java.lang.AutoCloseable @@ -430,6 +431,31 @@ [^ZipFile input ^ZipEntry entry] (.getInputStream input entry)) +(defn- size-limiting-stream + "Wraps an InputStream to enforce a maximum number of decompressed bytes. + Raises :validation :max-file-size-reached when the limit is exceeded." + ^InputStream + [^InputStream input ^long max-size] + (let [counter (atom 0)] + (proxy [FilterInputStream] [input] + (read + ([] + (let [b (.read input)] + (when (pos? b) + (when (> (swap! counter inc) max-size) + (ex/raise :type :validation + :code :max-file-size-reached + :hint (str "stream exceeded max size: " max-size)))) + b)) + ([buf off len] + (let [n (.read input buf off len)] + (when (pos? n) + (when (> (swap! counter + (long n)) max-size) + (ex/raise :type :validation + :code :max-file-size-reached + :hint (str "stream exceeded max size: " max-size)))) + n)))))) + (defn- zip-entry-reader [^ZipFile input ^ZipEntry entry] (-> (zip-entry-stream input entry) @@ -438,10 +464,12 @@ (defn- zip-entry-storage-content "Wraps a ZipFile and ZipEntry into a penpot storage compatible object and avoid creating temporal objects" - [input entry] - (let [hash (delay (->> entry - (zip-entry-stream input) - (sto.impl/calculate-hash)))] + [input entry & {:keys [max-size]}] + (let [stream-fn (fn [] + (cond-> (zip-entry-stream input entry) + max-size (size-limiting-stream max-size))) + hash (delay (->> (stream-fn) + (sto.impl/calculate-hash)))] (reify sto.impl/IContentObject (get-size [_] @@ -458,7 +486,7 @@ (throw (UnsupportedOperationException. "not implemented"))) (make-input-stream [_ _] - (zip-entry-stream input entry)) + (stream-fn)) (make-output-stream [_ _] (throw (UnsupportedOperationException. "not implemented")))))) @@ -846,9 +874,9 @@ ext (cmedia/mtype->extension (:content-type object)) path (str "objects/" id ext) - content (->> path - (get-zip-entry input) - (zip-entry-storage-content input))] + content (zip-entry-storage-content input + (get-zip-entry input path) + :max-size (::bfc/import-max-object-size cfg))] (when (not= (:size object) (sto/get-size content)) (ex/raise :type :validation @@ -858,6 +886,15 @@ :expected-size (:size object) :found-size (sto/get-size content))) + (when-let [max (::bfc/import-max-object-size cfg)] + (when (> (sto/get-size content) max) + (ex/raise :type :validation + :code :max-file-size-reached + :hint (str "storage object exceeds maximum size: " (sto/get-size content)) + :path path + :max max + :found (sto/get-size content)))) + (when-let [hash (get object :hash)] (when (not= hash (sto/get-hash content)) (ex/raise :type :validation @@ -940,6 +977,15 @@ (let [manifest (-> (read-manifest input) (validate-manifest)) entries (read-zip-entries input) + + _ (when-let [max (::bfc/import-max-zip-entries cfg)] + (when (> (count entries) max) + (ex/raise :type :validation + :code :too-many-zip-entries + :hint (str "zip file has too many entries: " (count entries)) + :max max + :found (count entries)))) + cfg (-> cfg (assoc ::entries entries) (assoc ::manifest manifest) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 3979f399d7..d514c5c9f4 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -94,7 +94,11 @@ ;; SSRF protection :ssrf-allowed-hosts #{} - :ssrf-extra-blocked-cidrs #{}}) + :ssrf-extra-blocked-cidrs #{} + + ;; Binfile import limits + :binfile-import-max-object-size (* 1024 1024 100) ;; 100 MiB + :binfile-import-max-zip-entries (* 500 1000)}) ;; 500,000 (def schema:config (do #_sm/optional-keys @@ -151,6 +155,10 @@ [:media-processing-service-uri {:optional true} ::sm/uri] [:media-processing-service-timeout {:optional true} ::sm/int] + ;; Binfile import limits (PENPOT_BINFILE_IMPORT_*) + [:binfile-import-max-object-size {:optional true} ::sm/int] + [:binfile-import-max-zip-entries {:optional true} ::sm/int] + [:deletion-delay {:optional true} ::ct/duration] [:file-clean-delay {:optional true} ::ct/duration] [:telemetry-enabled {:optional true} ::sm/boolean] diff --git a/backend/src/app/http/debug.clj b/backend/src/app/http/debug.clj index e86cc1ffda..69a8366cdc 100644 --- a/backend/src/app/http/debug.clj +++ b/backend/src/app/http/debug.clj @@ -322,7 +322,9 @@ ::bfc/overwrite false ::bfc/profile-id profile-id ::bfc/project-id project-id - ::bfc/input path)] + ::bfc/input path + ::bfc/import-max-object-size (cf/get :binfile-import-max-object-size) + ::bfc/import-max-zip-entries (cf/get :binfile-import-max-zip-entries))] (bf.v3/import-files! cfg) {::yres/status 200 ::yres/headers {"content-type" "text/plain"} @@ -358,7 +360,9 @@ ::bfc/profile-id profile-id ::bfc/project-id project-id ::bfc/input path - ::bfc/features (cfeat/get-team-enabled-features cf/flags team))] + ::bfc/features (cfeat/get-team-enabled-features cf/flags team) + ::bfc/import-max-object-size (cf/get :binfile-import-max-object-size) + ::bfc/import-max-zip-entries (cf/get :binfile-import-max-zip-entries))] (if (= format :binfile-v3) (bf.v3/import-files! cfg) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 44b7014968..fee8a98557 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -93,7 +93,9 @@ (assoc ::bfc/features (cfeat/get-team-enabled-features cf/flags team)) (assoc ::bfc/project-id project-id) (assoc ::bfc/profile-id profile-id) - (assoc ::bfc/name name)) + (assoc ::bfc/name name) + (assoc ::bfc/import-max-object-size (cf/get :binfile-import-max-object-size)) + (assoc ::bfc/import-max-zip-entries (cf/get :binfile-import-max-zip-entries))) input-path (:path file) owned? (some? upload-id) diff --git a/backend/src/app/rpc/commands/management.clj b/backend/src/app/rpc/commands/management.clj index 41931f53ec..902ff49a50 100644 --- a/backend/src/app/rpc/commands/management.clj +++ b/backend/src/app/rpc/commands/management.clj @@ -426,7 +426,9 @@ (assoc ::bfc/project-id project-id) (assoc ::bfc/profile-id profile-id) (assoc ::bfc/input template) - (assoc ::bfc/features (cfeat/get-team-enabled-features cf/flags team))) + (assoc ::bfc/features (cfeat/get-team-enabled-features cf/flags team)) + (assoc ::bfc/import-max-object-size (cf/get :binfile-import-max-object-size)) + (assoc ::bfc/import-max-zip-entries (cf/get :binfile-import-max-zip-entries))) result (if (= format :binfile-v3) (bf.v3/import-files! cfg) diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index c0e45d5429..a57624fa00 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -23,6 +23,7 @@ [app.storage :as sto] [app.storage.tmp :as tmp] [backend-tests.helpers :as th] + [backend-tests.storage-test :as stt] [clojure.test :as t] [cuerdas.core :as str] [datoteka.fs :as fs] @@ -252,3 +253,88 @@ ;; With the guard, it raises :validation :max-file-size-reached. (t/is (= :validation (:type out))) (t/is (= :max-file-size-reached (:code out)))))))) + +(t/deftest import-rejects-too-many-zip-entries + ;; import must reject ZIP files exceeding max-zip-entries + (let [profile (th/create-profile* 1) + file (prepare-simple-file profile) + output (tmp/tempfile :suffix ".zip")] + + (v3/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/embed-assets false) + (assoc ::bfc/include-libraries false)) + (io/output-stream output)) + + ;; Import with max-zip-entries=1 — the exported ZIP has more entries + (let [cfg (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input output) + (assoc ::bfc/import-max-zip-entries 1)) + out (try + (v3/import-files! cfg) + :no-error + (catch Throwable e + (let [d (or (ex-data e) (some-> (ex-cause e) ex-data))] + d)))] + (t/is (= :validation (:type out))) + (t/is (= :too-many-zip-entries (:code out)))))) + +(defn- prepare-file-with-media + "Creates a file with a media object backed by a real storage object, + so that v3 export produces objects/ entries." + [profile] + (let [storage (-> (:app.storage/storage th/*system*) + (stt/configure-storage-backend)) + + sobject (sto/put-object! storage {::sto/content (sto/content "media-bytes") + :content-type "image/svg+xml" + :bucket "file-media-object"}) + + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + + mobj (th/create-file-media-object* {:file-id (:id file) + :is-local true + :media-id (:id sobject)})] + (update-file! + :file-id (:id file) + :profile-id (:id profile) + :revn 0 + :vern 0 + :changes + [{:type :add-media + :object mobj}]) + + (dissoc file :data))) + +(t/deftest import-rejects-oversized-object + ;; import must reject storage objects exceeding max-object-size + (let [profile (th/create-profile* 1) + file (prepare-file-with-media profile) + output (tmp/tempfile :suffix ".zip")] + + (v3/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/embed-assets false) + (assoc ::bfc/include-libraries false)) + (io/output-stream output)) + + ;; Import with max-object-size=1 — the media object will exceed this + (let [cfg (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input output) + (assoc ::bfc/import-max-object-size 1)) + out (try + (v3/import-files! cfg) + :no-error + (catch Throwable e + (let [d (or (ex-data e) (some-> (ex-cause e) ex-data))] + d)))] + (t/is (= :validation (:type out))) + (t/is (= :max-file-size-reached (:code out))))))