From fd3ec5a9aa54585d2ab172bad390f77699ee1ea0 Mon Sep 17 00:00:00 2001 From: Nico <32375741+nulmete@users.noreply.github.com> Date: Wed, 6 May 2026 15:20:53 +0200 Subject: [PATCH] Add SVG support for custom organization logos (#44748) **Related issue:** Follow-up to #44390 (BE/FE) and #44550 (GitOps). Parent story #39016. ## Summary Accepts `.svg` for organization logo uploads in addition to PNG/JPEG/WebP, with strict server-side validation since SVGs can carry scripts. # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually https://github.com/user-attachments/assets/318d320e-ff78-41fe-ad3a-55d6dace8dc0 ## Summary by CodeRabbit * **New Features** * Organization logos now accept SVG in addition to PNG, JPEG, and WebP. * Stored SVG logos are re-validated when served. * **Security** * Server applies strict SVG sanitization to block scripts, unsafe elements, event handlers, and unsafe URL schemes. * SVG logo responses include headers to prevent content-type sniffing and restrict execution. * **Tests** * Added tests covering SVG detection, validation, allowed/rejected cases, and serving behavior. --- changes/add-svg-support-custom-logos | 1 + frontend/utilities/file/orgLogoFile.ts | 34 +++- server/fleet/org_logo.go | 147 ++++++++++++++- server/fleet/org_logo_test.go | 183 +++++++++++++++++++ server/fleet/testdata/icons/org_logo_css.svg | 8 + server/service/client_appconfig_test.go | 8 +- server/service/org_logo.go | 15 +- 7 files changed, 379 insertions(+), 17 deletions(-) create mode 100644 changes/add-svg-support-custom-logos create mode 100644 server/fleet/org_logo_test.go create mode 100644 server/fleet/testdata/icons/org_logo_css.svg diff --git a/changes/add-svg-support-custom-logos b/changes/add-svg-support-custom-logos new file mode 100644 index 0000000000..8fb3ad2b79 --- /dev/null +++ b/changes/add-svg-support-custom-logos @@ -0,0 +1 @@ +* Added SVG support for custom organization logos, with strict server-side sanitization to reject scripts and other unsafe SVG content. diff --git a/frontend/utilities/file/orgLogoFile.ts b/frontend/utilities/file/orgLogoFile.ts index 8584d886f3..1034f80a45 100644 --- a/frontend/utilities/file/orgLogoFile.ts +++ b/frontend/utilities/file/orgLogoFile.ts @@ -1,8 +1,8 @@ -export const ORG_LOGO_ACCEPT = ".png,.jpg,.jpeg,.webp"; +export const ORG_LOGO_ACCEPT = ".png,.jpg,.jpeg,.webp,.svg"; export const ORG_LOGO_MAX_SIZE_BYTES = 100 * 1024; // 100 KB export const ORG_LOGO_HELP_TEXT = "Personalize Fleet with your brand. For best results, use a square image at least 150px wide."; -export const ORG_LOGO_ALLOWED_TYPES = ["png", "jpeg", "webp"] as const; +export const ORG_LOGO_ALLOWED_TYPES = ["png", "jpeg", "webp", "svg"] as const; export type ImageFileType = typeof ORG_LOGO_ALLOWED_TYPES[number]; @@ -11,6 +11,11 @@ const ORG_LOGO_ALLOWED_TYPES_LABEL = `${upperAllowedTypes .slice(0, -1) .join(", ")}, or ${upperAllowedTypes[upperAllowedTypes.length - 1]}`; +// Larger than any non-SVG magic-byte prefix so a single read covers all +// formats. SVG detection scans the head for " { ) { return "webp"; } + // SVG is text. Require sail past the FE check and only get rejected after + // the upload round-trips. Strip BOM, leading whitespace, an optional + // declaration, and any leading comments / DOCTYPE / PIs, + // then look for \s*/, ""); + // Comments / DOCTYPE / other PIs may all stack before the root. + let prev: string; + do { + prev = text; + text = text.replace(/^\s*/, ""); + text = text.replace(/^]*>\s*/i, ""); + text = text.replace(/^<\?[\s\S]*?\?>\s*/, ""); + } while (text !== prev); + if (/^ ORG_LOGO_MAX_SIZE_BYTES) { return { valid: false, error: "Logo must be 100 KB or less." }; } - const headerBuf = await file.slice(0, 12).arrayBuffer(); + const headerBuf = await file.slice(0, SNIFF_BYTES).arrayBuffer(); const detected = detectImageType(new Uint8Array(headerBuf)); if (!detected || !ORG_LOGO_ALLOWED_TYPES.includes(detected)) { return { diff --git a/server/fleet/org_logo.go b/server/fleet/org_logo.go index ed6a223f7c..1310510ff5 100644 --- a/server/fleet/org_logo.go +++ b/server/fleet/org_logo.go @@ -3,10 +3,15 @@ package fleet import ( "bytes" "context" + "encoding/xml" + "errors" + "fmt" "image" _ "image/jpeg" _ "image/png" "io" + "net/url" + "strings" _ "golang.org/x/image/webp" ) @@ -61,8 +66,24 @@ func hasWebPMagic(b []byte) bool { return len(b) >= 12 && bytes.Equal(b[0:4], []byte("RIFF")) && bytes.Equal(b[8:12], []byte("WEBP")) } +// looksLikeSVG only decides which validator to run; validateSVG is what +// actually rejects unsafe content. +func looksLikeSVG(b []byte) bool { + // Editors may prepend a UTF-8 BOM (byte-order mark) or whitespace. + if bytes.HasPrefix(b, []byte{0xEF, 0xBB, 0xBF}) { + b = b[3:] + } + b = bytes.TrimLeft(b, " \t\r\n") + // 512B keeps routing cheap; the root is always near the top. + const window = 512 + if len(b) > window { + b = b[:window] + } + return bytes.Contains(bytes.ToLower(b), []byte(" OrgLogoMaxFileSize { return &BadRequestError{Message: "logo must be 100KB or less"} } - _, format, err := image.DecodeConfig(bytes.NewReader(b)) + switch ContentTypeForOrgLogo(b) { + case "image/png", "image/jpeg", "image/webp": + _, format, err := image.DecodeConfig(bytes.NewReader(b)) + if err != nil { + return &BadRequestError{ + Message: "logo must be a valid PNG, JPEG, WebP, or SVG image", + InternalErr: err, + } + } + switch format { + case "png", "jpeg", "webp": + return nil + } + case "image/svg+xml": + return validateSVG(b) + } + return &BadRequestError{Message: "logo must be a PNG, JPEG, WebP, or SVG file"} +} + +func isSafeSVGURL(raw string) bool { + raw = strings.TrimSpace(raw) + if raw == "" || strings.HasPrefix(raw, "#") { + return true + } + u, err := url.Parse(raw) if err != nil { - return &BadRequestError{ - Message: "logo must be a valid PNG, JPEG, or WebP image", - InternalErr: err, + return false + } + if u.Scheme == "" { + // Bare relative path or fragment is fine; protocol-relative + // "//host/x" parses to empty Scheme + non-empty Host and a + // browser resolves it against the page's scheme — reject. + return u.Host == "" + } + s := strings.ToLower(u.Scheme) + return s == "http" || s == "https" +} + +// Elements that can run scripts or load foreign content. -rendered +// SVGs are script-sandboxed, but pasting the URL loads it as a document +// — so reject structurally instead of trusting the renderer. SMIL +// animation tags are blocked because they can mutate href/xlink:href at +// runtime and bypass the static href allowlist. +var disallowedSVGElements = map[string]struct{}{ + "script": {}, + "foreignobject": {}, + "iframe": {}, + "object": {}, + "embed": {}, + "set": {}, + "animate": {}, + "animatetransform": {}, + "animatemotion": {}, +} + +// validateSVG rejects unsafe SVG content. Leaving decoder.Entity nil and +// rejecting DOCTYPE neutralizes XXE (external entities reading local +// files) and billion-laughs (DoS via recursive entity expansion). +func validateSVG(b []byte) error { + decoder := xml.NewDecoder(bytes.NewReader(b)) + decoder.Strict = true + + sawRoot := false + for { + tok, err := decoder.Token() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return &BadRequestError{ + Message: "logo is not valid SVG", + InternalErr: err, + } + } + switch t := tok.(type) { + case xml.StartElement: + name := strings.ToLower(t.Name.Local) + if !sawRoot { + if name != "svg" { + return &BadRequestError{Message: "logo SVG must have as the root element"} + } + sawRoot = true + } + if _, bad := disallowedSVGElements[name]; bad { + return &BadRequestError{Message: fmt.Sprintf("SVG element <%s> is not allowed", name)} + } + for _, attr := range t.Attr { + attrName := strings.ToLower(attr.Name.Local) + // on* (onclick, onload, …) is SVG's main XSS vector. + if strings.HasPrefix(attrName, "on") { + return &BadRequestError{Message: "SVG event-handler attributes are not allowed"} + } + // Name.Local matches both href and xlink:href. xml:base + // is here too: a hostile base ("javascript:") would + // re-anchor every relative href and bypass this check. + if attrName == "href" || attrName == "src" || attrName == "base" { + if !isSafeSVGURL(attr.Value) { + return &BadRequestError{Message: "SVG href/src/xml:base must be a fragment, relative path, or http(s):// URL"} + } + } + } + case xml.Directive: + // DOCTYPE / ENTITY (XXE, billion-laughs vectors). + return &BadRequestError{Message: "SVG DTD/DOCTYPE declarations are not allowed"} + case xml.ProcInst: + // The leading `` declaration is reported as a + // ProcInst with Target=="xml". Anything else (most notably + // ``) pulls external resources + // when the SVG is opened as a document — block it. + if t.Target != "xml" { + return &BadRequestError{Message: fmt.Sprintf("SVG processing instruction is not allowed", t.Target)} + } } } - switch format { - case "png", "jpeg", "webp": - return nil + if !sawRoot { + return &BadRequestError{Message: "logo SVG missing root element"} } - return &BadRequestError{Message: "logo must be a PNG, JPEG, or WebP file"} + return nil } diff --git a/server/fleet/org_logo_test.go b/server/fleet/org_logo_test.go new file mode 100644 index 0000000000..0b99e7c6cc --- /dev/null +++ b/server/fleet/org_logo_test.go @@ -0,0 +1,183 @@ +package fleet + +import ( + "bytes" + "fmt" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateOrgLogoBytesSVG(t *testing.T) { + t.Parallel() + + const minSVG = `` + + t.Run("accepts a minimal SVG", func(t *testing.T) { + require.NoError(t, ValidateOrgLogoBytes([]byte(minSVG))) + }) + + t.Run("accepts an SVG with XML declaration and inline style", func(t *testing.T) { + body := ` + + + +` + require.NoError(t, ValidateOrgLogoBytes([]byte(body))) + }) + + t.Run("accepts xlink:href fragment references", func(t *testing.T) { + body := `` + require.NoError(t, ValidateOrgLogoBytes([]byte(body))) + }) + + t.Run("accepts a real-world SVG (CSS logo from the public web)", func(t *testing.T) { + body, err := os.ReadFile("testdata/icons/org_logo_css.svg") + require.NoError(t, err) + require.NoError(t, ValidateOrgLogoBytes(body)) + }) + + t.Run("rejects oversized SVG before parsing", func(t *testing.T) { + // Bytes don't need to be a real SVG — the size gate fires + // first regardless of looksLikeSVG. + body := append([]byte(""), bytes.Repeat([]byte("a"), int(OrgLogoMaxFileSize))...) + err := ValidateOrgLogoBytes(body) + require.Error(t, err) + assert.Contains(t, err.Error(), "100KB or less") + }) + + t.Run("rejects