mirror of
https://github.com/penpot/penpot.git
synced 2026-08-18 18:58:40 +00:00
🐛 Enforce actual decompressed byte limits on v3 import
The previous object-size check trusted the ZIP entry header's declared size (ZipEntry.getSize()), which a malicious zip-bomb can forge. The check would pass, then the full decompressed payload would be read anyway during hashing and storage persistence. Add size-limiting-stream, a FilterInputStream wrapper that counts actual bytes read and raises :validation :max-file-size-reached when the configured limit is exceeded. Wire it into zip-entry-storage-content so both the hash calculation and storage write paths are bounded by real decompressed bytes, not declared header size. Also wire import limits into management.clj (clone-template) and debug.clj (import-handler + clone path) for defense-in-depth, and add a test that exercises the object-size limit with a real storage object in the exported ZIP. AI-assisted-by: mimo-v2.5-pro
This commit is contained in:
parent
ee8f23fe0a
commit
1e425851b7
@ -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"))))))
|
||||
|
||||
@ -844,9 +872,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
|
||||
|
||||
@ -318,7 +318,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"}
|
||||
@ -354,7 +356,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)
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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]
|
||||
@ -257,3 +258,60 @@
|
||||
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))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user