From f784224aefab7368a6738a243a535818b9e52122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 23 Apr 2026 17:53:28 -0400 Subject: [PATCH] fix: split html sanitization from plaintext handling (#5403) * fix: split html sanitization from plaintext handling Add a dedicated SanitizeHTML helper for HTML-rendered values so entity-encoded markup is decoded before bluemonday sanitization. Use the new helper for the login welcome message and artist biographies while preserving SanitizeText semantics for lyrics and other plaintext callers. Add regression coverage for both helpers and the serveIndex welcomeMessage path. * docs: add SanitizeText and SanitizeHTML godoc Signed-off-by: Deluan * fix: preserve plain text in artist biographies Revert artist biography storage to SanitizeText so entity-encoded plain text remains decoded for Subsonic consumers. This avoids double-escaping values like R&B in XML responses while keeping the new welcomeMessage HTML sanitization in place, and adds a regression test covering the biography storage behavior. --------- Signed-off-by: Deluan --- .../provider_updateartistinfo_test.go | 23 +++++++++++++++ server/serve_index.go | 2 +- server/serve_index_test.go | 12 ++++++++ utils/str/sanitize_strings.go | 9 ++++++ utils/str/sanitize_strings_test.go | 29 +++++++++++++++++++ 5 files changed, 74 insertions(+), 1 deletion(-) diff --git a/core/external/provider_updateartistinfo_test.go b/core/external/provider_updateartistinfo_test.go index e309ece6e..cc9506d1f 100644 --- a/core/external/provider_updateartistinfo_test.go +++ b/core/external/provider_updateartistinfo_test.go @@ -105,6 +105,29 @@ var _ = Describe("Provider - UpdateArtistInfo", func() { ag.AssertExpectations(GinkgoT()) }) + It("preserves decoded plain text in biography storage", func() { + originalArtist := &model.Artist{ + ID: "ar-encoded-bio", + Name: "Encoded Bio Artist", + } + mockArtistRepo.SetData(model.Artists{*originalArtist}) + + expectedMBID := "mbid-encoded-bio" + expectedBio := "R&B" + + ag.On("GetArtistMBID", ctx, "ar-encoded-bio", "Encoded Bio Artist").Return(expectedMBID, nil).Once() + ag.On("GetArtistImages", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return(nil, nil).Maybe() + ag.On("GetArtistBiography", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return(expectedBio, nil).Once() + ag.On("GetArtistURL", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return("", nil).Maybe() + ag.On("GetSimilarArtists", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID, 100).Return(nil, nil).Maybe() + + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-encoded-bio", 10, false) + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) + Expect(updatedArtist.Biography).To(Equal("R&B")) + }) + It("returns cached info when artist exists and info is not expired", func() { now := time.Now() originalArtist := &model.Artist{ diff --git a/server/serve_index.go b/server/serve_index.go index bd5be44f5..734aabc70 100644 --- a/server/serve_index.go +++ b/server/serve_index.go @@ -45,7 +45,7 @@ func serveIndex(ds model.DataStore, fs fs.FS, shareInfo *model.Share) http.Handl "variousArtistsId": consts.VariousArtistsID, "baseURL": str.SanitizeText(strings.TrimSuffix(conf.Server.BasePath, "/")), "loginBackgroundURL": str.SanitizeText(conf.Server.UILoginBackgroundURL), - "welcomeMessage": str.SanitizeText(conf.Server.UIWelcomeMessage), + "welcomeMessage": str.SanitizeHTML(conf.Server.UIWelcomeMessage), "maxSidebarPlaylists": conf.Server.MaxSidebarPlaylists, "enableTranscodingConfig": conf.Server.EnableTranscodingConfig, "enableDownloads": conf.Server.EnableDownloads, diff --git a/server/serve_index_test.go b/server/serve_index_test.go index 7515e7276..31bca02cf 100644 --- a/server/serve_index_test.go +++ b/server/serve_index_test.go @@ -108,6 +108,18 @@ var _ = Describe("serveIndex", func() { Entry("extAuthLogoutURL", func() { conf.Server.ExtAuth.LogoutURL = "https://auth.example.com/logout" }, "extAuthLogoutURL", "https://auth.example.com/logout"), ) + It("sanitizes entity-encoded welcomeMessage as html", func() { + conf.Server.UIWelcomeMessage = `<img src=x onerror=alert(1)><b>Hello</b>` + r := httptest.NewRequest("GET", "/index.html", nil) + w := httptest.NewRecorder() + + serveIndex(ds, fs, nil)(w, r) + + config := extractAppConfig(w.Body.String()) + Expect(config).To(HaveKey("welcomeMessage")) + Expect(config["welcomeMessage"]).To(Equal(`Hello`)) + }) + DescribeTable("sets other UI configuration values", func(configKey string, expectedValueFunc func() any) { r := httptest.NewRequest("GET", "/index.html", nil) diff --git a/utils/str/sanitize_strings.go b/utils/str/sanitize_strings.go index 73608112e..c121aefe7 100644 --- a/utils/str/sanitize_strings.go +++ b/utils/str/sanitize_strings.go @@ -38,11 +38,20 @@ func SanitizeStrings(text ...string) string { var policy = bluemonday.UGCPolicy() +// SanitizeText unescapes the input string before sanitizing it as text. +// This should be used for fields rendered as plain text in the UI (e.g. lyrics, song titles, artist names) func SanitizeText(text string) string { s := policy.Sanitize(text) return html.UnescapeString(s) } +// SanitizeHTML unescapes the input string before sanitizing it as HTML. +// This should be used for fields rendered as HTML by clients (e.g. biographies, welcome messages) +// to prevent XSS bypasses via entity-encoded tags. +func SanitizeHTML(text string) string { + return policy.Sanitize(html.UnescapeString(text)) +} + func SanitizeFieldForSorting(originalValue string) string { v := strings.TrimSpace(sanitize.Accents(originalValue)) return Clear(strings.ToLower(v)) diff --git a/utils/str/sanitize_strings_test.go b/utils/str/sanitize_strings_test.go index ac28fe435..6527f326b 100644 --- a/utils/str/sanitize_strings_test.go +++ b/utils/str/sanitize_strings_test.go @@ -64,6 +64,35 @@ var _ = Describe("Sanitize Strings", func() { }) }) + Describe("SanitizeText", func() { + It("preserves decoded plaintext", func() { + Expect(str.SanitizeText("Tom & Jerry")).To(Equal("Tom & Jerry")) + Expect(str.SanitizeText("Tom & Jerry")).To(Equal("Tom & Jerry")) + }) + + It("keeps entity-encoded html readable", func() { + Expect(str.SanitizeText(`<b>ok</b>`)).To(Equal("ok")) + }) + }) + + Describe("SanitizeHTML", func() { + It("removes dangerous content from raw html", func() { + sanitized := str.SanitizeHTML(`ok`) + + Expect(sanitized).To(ContainSubstring("ok")) + Expect(sanitized).ToNot(ContainSubstring("onerror")) + Expect(sanitized).ToNot(ContainSubstring("ok")) + Expect(sanitized).ToNot(ContainSubstring("onerror")) + Expect(sanitized).ToNot(ContainSubstring("