diff --git a/backend/resources/climit.edn b/backend/resources/climit.edn index 7d8234499b..66ac82b174 100644 --- a/backend/resources/climit.edn +++ b/backend/resources/climit.edn @@ -39,4 +39,10 @@ {:permits 3} :create-file-snapshot/by-profile - {:permits 1 :queue 2 :timeout 60000}} + {:permits 1 :queue 2 :timeout 60000} + + :send-user-feedback/global + {:permits 4} + + :send-user-feedback/by-profile + {:permits 1 :queue 3}} diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index 09d34532b9..ecf8b658a5 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -776,7 +776,7 @@ (defn prepare-organization-sso-provider "Build an OIDC provider map dynamically from the Nitrate organization SSO config. - Uses OIDC discovery via :issuer when token/auth/user URIs are absent." + Uses OIDC discovery via :issuer when token/auth/user URIs are absent." [cfg {:keys [client-id client-secret issuer]}] (prepare-oidc-provider cfg {:type "oidc" @@ -785,8 +785,7 @@ :base-uri (some-> (non-blank-uri issuer) (str/rtrim "/") (str "/")) - :scopes default-oidc-scopes - :skip-ssrf-check? true})) + :scopes default-oidc-scopes})) (defn build-organization-sso-auth-redirect-uri "Build the OIDC authorization redirect URI for an organization SSO config. diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index a5b73564ea..f984a98550 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -748,9 +748,17 @@ (fmigr/upsert-migrations! conn file)) (let [file (encode-file cfg file)] - (db/insert! conn :file - (file->params file) - (assoc opts ::db/return-keys false)) + (try + (db/insert! conn :file + (file->params file) + (assoc opts ::db/return-keys false)) + (catch org.postgresql.util.PSQLException cause + (if (db/duplicate-key-error? cause) + (ex/raise :type :not-found + :code :object-not-found + :hint "file already exists" + :cause cause) + (throw cause)))) (->> (file->file-data-params file) (fdata/upsert! cfg)) diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index 8dc4120159..5f1834cb74 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -174,6 +174,10 @@ (assert-mark m :obj) (let [size (read-long! input)] (assert (pos? size) "incorrect header size found on reading header") + (when (> size bfc/max-object-size) + (ex/raise :type :validation + :code :max-file-size-reached + :hint (dm/str "unable to import object with size " size " bytes"))) (let [buff (byte-array size)] (read-bytes! input buff) (fres/decode buff))))) diff --git a/backend/src/app/http/assets.clj b/backend/src/app/http/assets.clj index 1458b06d27..04dd7842ca 100644 --- a/backend/src/app/http/assets.clj +++ b/backend/src/app/http/assets.clj @@ -7,6 +7,7 @@ (ns app.http.assets "Assets related handlers." (:require + [app.binfile.common :as bfc] [app.common.data :as d] [app.common.exceptions :as ex] [app.common.time :as ct] @@ -42,7 +43,7 @@ (defn- get-file-media-object [pool id] - (db/get pool :file-media-object {:id id} {::db/remove-deleted false})) + (db/get* pool :file-media-object {:id id} {::db/remove-deleted false})) (defn- serve-object-from-s3 [{:keys [::sto/storage ::signature-max-age ::cache-max-age] :as cfg} obj] @@ -109,13 +110,21 @@ (defn- generic-handler "A generic handler helper/common code for file-media based handlers." [{:keys [::sto/storage] :as cfg} request kf] - (let [pool (::db/pool storage) - id (get-id request) - mobj (get-file-media-object pool id) - sobj (sto/get-object storage (kf mobj))] - (if sobj - (serve-object cfg sobj) - {::yres/status 404}))) + (let [pool (::db/pool storage) + id (get-id request) + mobj (get-file-media-object pool id)] + (if (nil? mobj) + {::yres/status 404} + (let [file-id (:file-id mobj) + profile-id (or (::session/profile-id request) + (::actoken/profile-id request)) + perms (bfc/get-file-permissions pool profile-id file-id)] + (if-not (:can-read perms) + {::yres/status 404} + (let [sobj (sto/get-object storage (kf mobj))] + (if sobj + (serve-object cfg sobj) + {::yres/status 404}))))))) (defn file-objects-handler "Handler that serves storage objects by file media id." diff --git a/backend/src/app/media/local.clj b/backend/src/app/media/local.clj index b53c5a5f6d..f86e46c02e 100644 --- a/backend/src/app/media/local.clj +++ b/backend/src/app/media/local.clj @@ -7,30 +7,22 @@ (ns app.media.local "Local media processing via ImageMagick and FontForge shell commands." (:require - [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.logging :as l] [app.common.media :as cm] [app.common.schema :as sm] [app.common.time :as ct] [app.config :as cf] + [app.media.svg :as svg] [app.media.validation :as validation] [app.storage.tmp :as tmp] [app.util.shell :as shell] [buddy.core.bytes :as bb] [buddy.core.codecs :as bc] [clojure.string] - [clojure.xml :as xml] [cuerdas.core :as str] [datoteka.fs :as fs] - [datoteka.io :as io]) - (:import - clojure.lang.XMLHandler - java.io.InputStream - javax.xml.parsers.SAXParserFactory - javax.xml.XMLConstants - org.apache.commons.io.IOUtils)) + [datoteka.io :as io])) (defmulti process (fn [_system params] (:cmd params))) @@ -40,30 +32,6 @@ :code :not-implemented :hint (str/fmt "No impl found for local process cmd: %s" cmd))) -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; SVG PARSING -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -(defn- secure-parser-factory - [^InputStream input ^XMLHandler handler] - (.. (doto (SAXParserFactory/newInstance) - (.setFeature XMLConstants/FEATURE_SECURE_PROCESSING true) - (.setFeature "http://apache.org/xml/features/disallow-doctype-decl" true)) - (newSAXParser) - (parse input handler))) - -(defn- strip-doctype - [data] - (cond-> data - (str/includes? data "]*>" ""))) - -(defn parse-svg - [text] - (let [text (strip-doctype text)] - (dm/with-open [istream (IOUtils/toInputStream ^String text "UTF-8")] - (xml/parse istream secure-parser-factory)))) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; IMAGE THUMBNAILS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -167,34 +135,6 @@ "-extent" (str width "x" height) "-quality" (str quality)])))) -(defn get-basic-info-from-svg - [{:keys [tag attrs] :as data}] - (when (not= tag :svg) - (ex/raise :type :validation - :code :unable-to-parse-svg - :hint "uploaded svg has invalid content")) - (reduce (fn [default f] - (if-let [res (f attrs)] - (reduced res) - default)) - {:width 100 :height 100} - [(fn parse-width-and-height - [{:keys [width height]}] - (when (and (string? width) - (string? height)) - (let [width (d/parse-double width) - height (d/parse-double height)] - (when (and width height) - {:width (int width) - :height (int height)})))) - (fn parse-viewbox - [{:keys [viewBox]}] - (let [[x y width height] (->> (str/split viewBox #"\s+" 4) - (map d/parse-double))] - (when (and x y width height) - {:width (int width) - :height (int height)})))])) - (defn- get-dimensions-with-orientation [system ^String path] ;; Image magick doesn't give info about exif rotation so we use the identify command ;; If we are processing an animated gif we use the first frame with -scene 0 @@ -217,7 +157,7 @@ [system {:keys [input] :as params}] (let [{:keys [path mtype] :as input} (validation/check-input input)] (if (= mtype "image/svg+xml") - (let [info (some-> path slurp parse-svg get-basic-info-from-svg)] + (let [info (some-> path slurp svg/parse-svg svg/get-basic-info-from-svg)] (when-not info (ex/raise :type :validation :code :invalid-svg-file diff --git a/backend/src/app/media/remote.clj b/backend/src/app/media/remote.clj index 447d5f2e55..0b5a0a4a42 100644 --- a/backend/src/app/media/remote.clj +++ b/backend/src/app/media/remote.clj @@ -13,7 +13,7 @@ [app.common.uri :as uri] [app.config :as cf] [app.http.client :as http] - [app.media.local :as local] + [app.media.svg :as svg] [app.media.validation :as validation] [app.setup :as-alias setup] [app.storage.tmp :as tmp] @@ -182,7 +182,7 @@ (let [{:keys [path mtype]} (validation/check-input input)] (if (= mtype "image/svg+xml") ;; SVG: parse locally (Sharp doesn't support SVG) - (let [info (some-> path slurp local/parse-svg local/get-basic-info-from-svg)] + (let [info (some-> path slurp svg/parse-svg svg/get-basic-info-from-svg)] (when-not info (ex/raise :type :validation :code :invalid-svg-file diff --git a/backend/src/app/media/svg.clj b/backend/src/app/media/svg.clj new file mode 100644 index 0000000000..1de52d4030 --- /dev/null +++ b/backend/src/app/media/svg.clj @@ -0,0 +1,130 @@ +;; 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 app.media.svg + "SVG parsing, sanitization, and info extraction. + Centralizes all SVG-related security concerns." + (:require + [app.common.data :as d] + [app.common.data.macros :as dm] + [app.common.exceptions :as ex] + [app.common.logging :as l] + [clojure.xml :as xml] + [cuerdas.core :as str]) + (:import + clojure.lang.XMLHandler + java.io.InputStream + javax.xml.parsers.SAXParserFactory + javax.xml.XMLConstants + org.apache.commons.io.IOUtils)) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; SVG PARSING +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn- secure-parser-factory + [^InputStream input ^XMLHandler handler] + (.. (doto (SAXParserFactory/newInstance) + (.setFeature XMLConstants/FEATURE_SECURE_PROCESSING true) + (.setFeature "http://apache.org/xml/features/disallow-doctype-decl" true)) + (newSAXParser) + (parse input handler))) + +(defn- strip-doctype + [data] + (cond-> data + (str/includes? data "]*>" ""))) + +(defn parse-svg + [text] + (let [text (strip-doctype text)] + (dm/with-open [istream (IOUtils/toInputStream ^String text "UTF-8")] + (xml/parse istream secure-parser-factory)))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; SVG SANITIZATION +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(def ^:private dangerous-attrs-pattern #"(?i)^on\w+$") +(def ^:private javascript-href-pattern #"(?i)^javascript:") + +(defn- sanitize-svg-element + "Recursively sanitize an SVG element by removing dangerous tags and attributes." + [{:keys [tag attrs content] :as element}] + (when (and (map? element) tag) + (let [dangerous-tags #{:script :foreignObject :set :animate :animateTransform :animateColor :animateMotion}] + (when-not (contains? dangerous-tags tag) + (let [clean-attrs (->> attrs + (remove (fn [[k v]] + (or (re-matches dangerous-attrs-pattern (name k)) + (and (#{:href :xlink:href} k) + (string? v) + (re-find javascript-href-pattern (str/trim v)))))) + (into {})) + clean-content (when content + (->> content + (filter #(or (string? %) (map? %))) + (map (fn [child] + (if (map? child) + (sanitize-svg-element child) + child))) + (filter some?) + vec))] + (cond-> {:tag tag :attrs clean-attrs} + (seq clean-content) (assoc :content clean-content))))))) + +(defn sanitize-svg + "Sanitize SVG content by removing dangerous elements and attributes. + Removes " + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "foreignObject"))) + (t/is (not (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result " s diff --git a/common/src/app/common/fressian.clj b/common/src/app/common/fressian.clj index b16d233b42..3de4cc54a7 100644 --- a/common/src/app/common/fressian.clj +++ b/common/src/app/common/fressian.clj @@ -31,6 +31,11 @@ ([^String s, ^String encoding] (.getBytes s encoding))) +;; --- DEPTH TRACKING + +(def ^:dynamic *read-depth* 0) +(def ^:const max-read-depth 128) + ;; --- LOW LEVEL FRESSIAN API (defn write-object! @@ -41,7 +46,13 @@ (defn read-object! [^Reader r] - (.readObject r)) + (when (>= *read-depth* max-read-depth) + (throw (ex-info "maximum Fressian read depth exceeded" + {:type :validation + :code :max-read-depth-reached + :hint "maximum Fressian read depth exceeded"}))) + (binding [*read-depth* (inc *read-depth*)] + (.readObject r))) (defn write-tag! ([^Writer w ^String n] diff --git a/common/test/common_tests/data_test.cljc b/common/test/common_tests/data_test.cljc index 39f3370de8..46f12fd8fb 100644 --- a/common/test/common_tests/data_test.cljc +++ b/common/test/common_tests/data_test.cljc @@ -36,6 +36,24 @@ (t/is (= "" (d/get-initials nil))) (t/is (= "" (d/get-initials "!!! ???")))) +(t/deftest normalize-string-test + ;; nil input returns empty string + (t/is (= "" (d/normalize-string nil))) + ;; empty string returns empty string + (t/is (= "" (d/normalize-string ""))) + ;; leading whitespace is trimmed + (t/is (= "hello" (d/normalize-string " hello"))) + ;; trailing whitespace is trimmed + (t/is (= "hello" (d/normalize-string "hello "))) + ;; both leading and trailing whitespace are trimmed + (t/is (= "hello" (d/normalize-string " hello "))) + ;; internal whitespace is preserved + (t/is (= "hello world" (d/normalize-string " hello world "))) + ;; non-string input is returned unchanged + (t/is (= 42 (d/normalize-string 42))) + (t/is (= :keyword (d/normalize-string :keyword))) + (t/is (= true (d/normalize-string true)))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Ordered Data Structures ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/common/test/common_tests/fressian_test.clj b/common/test/common_tests/fressian_test.clj index 9af54464a5..3eda0f34d4 100644 --- a/common/test/common_tests/fressian_test.clj +++ b/common/test/common_tests/fressian_test.clj @@ -21,7 +21,8 @@ (:import java.time.Instant java.time.OffsetDateTime - java.time.ZoneOffset)) + java.time.ZoneOffset + java.util.UUID)) ;; --------------------------------------------------------------------------- ;; Helpers @@ -524,3 +525,18 @@ (t/is (d/ordered-map? rt)) (t/is (= om rt)) (t/is (= (keys om) (keys rt))))) + +(t/deftest decode-rejects-excessive-recursion-depth + ;; N2-01: deeply nested structures must be rejected before stack overflow + (let [depth (+ fres/max-read-depth 50) + data (reduce (fn [acc _i] [acc]) + :leaf + (range depth)) + encoded (fres/encode data)] + (try + (fres/decode encoded) + (t/is false "expected exception for excessive recursion depth") + (catch clojure.lang.ExceptionInfo e + (let [d (ex-data e)] + (t/is (= :validation (:type d))) + (t/is (= :max-read-depth-reached (:code d)))))))) diff --git a/scripts/ci b/scripts/ci index 3d3c7ed9a8..026bdb7d02 100755 --- a/scripts/ci +++ b/scripts/ci @@ -23,7 +23,7 @@ ALL_MODULES=("frontend" "backend" "common" "render-wasm" "exporter" "mcp" "plugi # Module commands declare -A LINT_CMD=( [frontend]="pnpm run lint:clj && pnpm run lint:js && pnpm run lint:scss" - [backend]="pnpm run lint" + [backend]="pnpm run lint:clj" [common]="pnpm run lint:clj" [render-wasm]="./lint" [exporter]="pnpm run lint"