sec(server): sanitize user-controlled filenames in Content-Disposition (#5895)

* sec(server): sanitize user-controlled filenames in Content-Disposition

Playlist export, Subsonic download and public share download built the
Content-Disposition header by interpolating a user-controlled name into a
quoted-string with fmt.Sprintf. A name containing a double quote closes the
string early and the rest is parsed as additional parameters, so a playlist
named `party"; filename="evil.html` yielded

    attachment; filename="party"; filename="evil.html.m3u"

letting whoever chose the name decide what the browser saves the download as.
The names come from playlists, album/artist names and media file tags.

Go's net/http already rewrites CR and LF in header values to spaces, so
response splitting was not reachable; parameter injection was.

Add str.ContentDispositionAttachment, which emits a sanitized ASCII-only
quoted `filename` plus an RFC 5987 `filename*` carrying the original UTF-8
name, and use it at all four call sites. The `filename*` parameter also fixes
non-ASCII names, which previously went out raw or were mangled by sanitizing.

Signed-off-by: zapisanchez <zapisanchez@gmail.com>

* fix(server): keep download names intact and sanitize filename*

Rework ContentDispositionAttachment after review. Names with no ASCII
letters now fall back to download.<ext> instead of a bare extension
(東京.mp3 gave filename="mp3"). filename* is built from the same
sanitized name as the ASCII fallback, so path separators, reserved
characters, control and bidi characters, and invalid UTF-8 no longer
reach it. The ASCII fallback transliterates accents and typographic
punctuation (Legião -> Legiao, She’s -> She's) through the existing
sanitize.Accents and str.Clear helpers, keeps leading dots, and only
trims trailing ones. Names are capped at 255 bytes, keeping the
extension.

Pure ASCII names now get only the quoted filename parameter, so the
header for them matches the previous output byte for byte. filename*
is encoded with mime.FormatMediaType instead of a hand-written RFC 5987
encoder. Adds tests for the M3U export and Subsonic download headers.

* fix(server): handle dot-only names and long fake extensions

A name made only of dots trimmed down to an empty filename. It now
falls back to download, like an empty stem does.

path.Ext treats anything after the last dot as the extension, so a long
suffix with no real extension was kept whole and replaced the stem with
download, going past the 255-byte cap. Suffixes longer than 16 bytes
are now treated as part of the stem and truncated with it.

Neither case is reachable from the current call sites, which always
append a short extension.

---------

Signed-off-by: zapisanchez <zapisanchez@gmail.com>
Co-authored-by: Deluan <deluan@navidrome.org>

server/nativeapi/playlists.go

@@ -16,6 +16,7 @@ import (
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/utils/req"
+	"github.com/navidrome/navidrome/utils/str"
 )
 
 type restHandler = func(rest.RepositoryConstructor, ...rest.Logger) http.HandlerFunc
@@ -101,8 +102,7 @@ func handleExportPlaylist(pls playlists.Playlists) http.HandlerFunc {
 
 		log.Debug(ctx, "Exporting playlist as M3U", "playlistId", plsId, "name", playlist.Name)
 		w.Header().Set("Content-Type", "audio/x-mpegurl")
-		disposition := fmt.Sprintf("attachment; filename=\"%s.m3u\"", playlist.Name)
-		w.Header().Set("Content-Disposition", disposition)
+		w.Header().Set("Content-Disposition", str.ContentDispositionAttachment(playlist.Name+".m3u"))
 
 		_, err = w.Write([]byte(playlist.ToM3U8())) //nolint:gosec
 		if err != nil {

server/nativeapi/playlists_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/deluan/rest"
+	"github.com/go-chi/chi/v5"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/consts"
@@ -183,6 +184,37 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
 	})
 })
 
+var _ = Describe("handleExportPlaylist", func() {
+	export := func(name string) *httptest.ResponseRecorder {
+		r := chi.NewRouter()
+		r.Get("/playlist/{playlistId}", handleExportPlaylist(&mockPlaylistsService{
+			playlist: &model.Playlist{ID: "pls-1", Name: name},
+		}))
+		w := httptest.NewRecorder()
+		r.ServeHTTP(w, httptest.NewRequest("GET", "/playlist/pls-1", nil))
+		return w
+	}
+
+	It("names the download after the playlist", func() {
+		w := export("Road Trip")
+
+		Expect(w.Code).To(Equal(http.StatusOK))
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="Road Trip.m3u"`))
+	})
+
+	It("does not let the playlist name inject a second filename parameter", func() {
+		w := export(`party"; filename="evil.html`)
+
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="party_; filename=_evil.html.m3u"`))
+	})
+
+	It("keeps non-ASCII names in filename*", func() {
+		w := export("Кино")
+
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="download.m3u"; filename*=utf-8''%D0%9A%D0%B8%D0%BD%D0%BE.m3u`))
+	})
+})
+
 var _ = Describe("writePlaylistError", func() {
 	DescribeTable("maps a service error to an HTTP status",
 		func(err error, expected int) {
@@ -231,6 +263,7 @@ func (m *mockPlaylistTrackRepo) Read(id string) (any, error) {
 type mockPlaylistsService struct {
 	playlists.Playlists
 	tracksRepo    rest.Repository
+	playlist      *model.Playlist
 	removeImageFn func(ctx context.Context, id string) error
 	setImageFn    func(ctx context.Context, id string, reader io.Reader, ext string) error
 }
@@ -249,6 +282,13 @@ func (m *mockPlaylistsService) SetImage(ctx context.Context, id string, reader i
 	return model.ErrNotFound
 }
 
+func (m *mockPlaylistsService) GetWithTracks(_ context.Context, _ string) (*model.Playlist, error) {
+	if m.playlist == nil {
+		return nil, model.ErrNotFound
+	}
+	return m.playlist, nil
+}
+
 func (m *mockPlaylistsService) TracksRepository(_ context.Context, _ string, _ bool) rest.Repository {
 	return m.tracksRepo
 }

server/public/handle_downloads.go

@@ -2,9 +2,7 @@ package public
 
 import (
 	"cmp"
-	"fmt"
 	"net/http"
-	"strings"
 
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/utils/req"
@@ -31,9 +29,8 @@ func (pub *Router) handleDownloads(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	name := str.SanitizeFilename(cmp.Or(s.Description, s.ID))
-	name = strings.ReplaceAll(name, ",", "_")
-	w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", name+".zip"))
+	name := cmp.Or(s.Description, s.ID)
+	w.Header().Set("Content-Disposition", str.ContentDispositionAttachment(name+".zip"))
 	w.Header().Set("Content-Type", "application/zip")
 
 	err = pub.archiver.ZipShare(ctx, s, w)

server/public/handle_downloads_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"mime"
 	"net/http"
 	"net/http/httptest"
 	"time"
@@ -95,6 +96,26 @@ var _ = Describe("handleDownloads", func() {
 		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="AC_DC_ Live_ 1979.zip"`))
 	})
 
+	It("sanitizes the UTF-8 filename* as well", func() {
+		shareIs(&model.Share{ID: "abc123", Description: "Sigur Rós/Live", Downloadable: true})
+
+		w := makeRequest("abc123")
+
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="Sigur Ros_Live.zip"; filename*=utf-8''Sigur%20R%C3%B3s_Live.zip`))
+	})
+
+	It("does not let the share description inject a second filename parameter", func() {
+		shareIs(&model.Share{ID: "abc123", Description: `mix"; filename="evil.html`, Downloadable: true})
+
+		w := makeRequest("abc123")
+
+		disposition := w.Header().Get("Content-Disposition")
+		Expect(disposition).ToNot(ContainSubstring(`filename="evil.html`))
+		_, params, err := mime.ParseMediaType(disposition)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(params["filename"]).To(Equal(`mix_; filename=_evil.html.zip`))
+	})
+
 	It("returns 403 without invoking the archiver when the share is not downloadable", func() {
 		shareIs(&model.Share{ID: "abc123", Description: "No Download", Downloadable: false})
 

server/subsonic/e2e/subsonic_media_retrieval_test.go

@@ -106,7 +106,7 @@ var _ = Describe("Media Retrieval Endpoints", Ordered, func() {
 	})
 
 	Describe("Download", func() {
-		var trackID string
+		var trackID, trackTitle string
 
 		BeforeAll(func() {
 			// All test tracks are mp3 at 320kbps
@@ -114,6 +114,7 @@ var _ = Describe("Media Retrieval Endpoints", Ordered, func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(songs).ToNot(BeEmpty())
 			trackID = songs[0].ID
+			trackTitle = songs[0].Title
 		})
 
 		It("returns error when id parameter is missing", func() {
@@ -143,6 +144,7 @@ var _ = Describe("Media Retrieval Endpoints", Ordered, func() {
 			Expect(w.Code).To(Equal(http.StatusOK))
 			Expect(streamerSpy.LastRequest.Format).To(Equal("opus"))
 			Expect(streamerSpy.LastRequest.BitRate).To(Equal(128))
+			Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="` + trackTitle + `.opus"`))
 		})
 
 		It("returns error when downloads are disabled", func() {

server/subsonic/stream.go

@@ -3,10 +3,8 @@ package subsonic
 import (
 	"context"
 	"errors"
-	"fmt"
 	"net/http"
 	"strconv"
-	"strings"
 
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/core/stream"
@@ -15,6 +13,7 @@ import (
 	"github.com/navidrome/navidrome/model/request"
 	"github.com/navidrome/navidrome/server/subsonic/responses"
 	"github.com/navidrome/navidrome/utils/req"
+	"github.com/navidrome/navidrome/utils/str"
 )
 
 func (api *Router) Stream(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
@@ -94,9 +93,7 @@ func (api *Router) Download(w http.ResponseWriter, r *http.Request) (*responses.
 	}
 
 	setHeaders := func(name string) {
-		name = strings.ReplaceAll(name, ",", "_")
-		disposition := fmt.Sprintf("attachment; filename=\"%s.zip\"", name)
-		w.Header().Set("Content-Disposition", disposition)
+		w.Header().Set("Content-Disposition", str.ContentDispositionAttachment(name+".zip"))
 		w.Header().Set("Content-Type", "application/zip")
 	}
 
@@ -115,8 +112,7 @@ func (api *Router) Download(w http.ResponseWriter, r *http.Request) (*responses.
 			}
 		}()
 
-		disposition := fmt.Sprintf("attachment; filename=\"%s\"", stream.Name())
-		w.Header().Set("Content-Disposition", disposition)
+		w.Header().Set("Content-Disposition", str.ContentDispositionAttachment(stream.Name()))
 
 		_, err = stream.Serve(ctx, w, r)
 		return nil, err

utils/str/content_disposition.go

@@ -0,0 +1,80 @@
+package str
+
+import (
+	"cmp"
+	"fmt"
+	"mime"
+	"path"
+	"strings"
+	"unicode"
+	"unicode/utf8"
+
+	"github.com/deluan/sanitize"
+)
+
+const (
+	maxFilenameBytes  = 255
+	maxExtensionBytes = 16
+	fallbackFilename  = "download"
+)
+
+// ContentDispositionAttachment builds an RFC 6266 attachment header value for a user-controlled filename.
+// Non-ASCII names also get a filename* parameter, which clients prefer over the ASCII fallback.
+func ContentDispositionAttachment(filename string) string {
+	stem, ext := splitFilename(filename)
+	header := fmt.Sprintf("attachment; filename=%q", joinFilename(toASCII(stem), toASCII(ext)))
+	name := joinFilename(stem, ext)
+	if isASCII(name) {
+		return header
+	}
+	// FormatMediaType emits a percent-encoded filename* for non-ASCII values
+	extended := mime.FormatMediaType("attachment", map[string]string{"filename": name})
+	return header + strings.TrimPrefix(extended, "attachment")
+}
+
+func splitFilename(filename string) (stem, ext string) {
+	name := strings.Map(func(r rune) rune {
+		if unicode.IsControl(r) || unicode.Is(unicode.Bidi_Control, r) {
+			return -1
+		}
+		return r
+	}, SanitizeFilename(strings.ToValidUTF8(filename, "_")))
+	ext = path.Ext(name)
+	if len(ext) > maxExtensionBytes {
+		ext = ""
+	}
+	stem = strings.TrimSuffix(name, ext)
+	if limit := maxFilenameBytes - len(ext); len(stem) > limit {
+		for limit > 0 && !utf8.RuneStart(stem[limit]) {
+			limit--
+		}
+		stem = stem[:limit]
+	}
+	return stem, ext
+}
+
+func joinFilename(stem, ext string) string {
+	stem = cmp.Or(strings.TrimSpace(stem), fallbackFilename)
+	return cmp.Or(strings.TrimRight(stem+ext, " ."), fallbackFilename)
+}
+
+func toASCII(s string) string {
+	return strings.Map(func(r rune) rune {
+		switch {
+		case r == ',':
+			return '_'
+		case r >= ' ' && r <= '~':
+			return r
+		}
+		return -1
+	}, sanitize.Accents(SanitizeFilename(Clear(s))))
+}
+
+func isASCII(s string) bool {
+	for i := 0; i < len(s); i++ {
+		if s[i] >= utf8.RuneSelf {
+			return false
+		}
+	}
+	return true
+}

utils/str/content_disposition_test.go

@@ -0,0 +1,120 @@
+package str_test
+
+import (
+	"mime"
+	"regexp"
+	"strings"
+
+	"github.com/navidrome/navidrome/utils/str"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var asciiFilenameRe = regexp.MustCompile(`(?:^|; )filename="([^"]*)"`)
+
+// asciiName returns the quoted filename parameter, the one used by clients that ignore filename*.
+func asciiName(filename string) string {
+	m := asciiFilenameRe.FindAllStringSubmatch(str.ContentDispositionAttachment(filename), -1)
+	ExpectWithOffset(1, m).To(HaveLen(1), "expected exactly one quoted filename parameter")
+	return m[0][1]
+}
+
+// decodedName returns the name an RFC 6266 client picks, preferring filename* when present.
+func decodedName(filename string) string {
+	disp, params, err := mime.ParseMediaType(str.ContentDispositionAttachment(filename))
+	ExpectWithOffset(1, err).ToNot(HaveOccurred())
+	ExpectWithOffset(1, disp).To(Equal("attachment"))
+	return params["filename"]
+}
+
+var _ = Describe("ContentDispositionAttachment", func() {
+	Describe("parameter injection", func() {
+		const attack = `party"; filename="evil.html.m3u`
+
+		It("does not let a quote in the name open a second parameter", func() {
+			Expect(str.ContentDispositionAttachment(attack)).To(Equal(`attachment; filename="party_; filename=_evil.html.m3u"`))
+		})
+
+		It("keeps the header parseable", func() {
+			Expect(decodedName(attack)).To(Equal("party_; filename=_evil.html.m3u"))
+		})
+
+		It("does not let a quote in a non-ASCII name inject either", func() {
+			const utf8Attack = `東京"; filename="evil.html.m3u`
+			header := str.ContentDispositionAttachment(utf8Attack)
+			Expect(strings.Count(header, `"`)).To(Equal(2))
+			Expect(decodedName(utf8Attack)).To(Equal("東京_; filename=_evil.html.m3u"))
+		})
+	})
+
+	Describe("ASCII names", func() {
+		It("sends only the quoted filename, unchanged", func() {
+			Expect(str.ContentDispositionAttachment("My Playlist.m3u")).To(Equal(`attachment; filename="My Playlist.m3u"`))
+		})
+
+		It("keeps leading dots", func() {
+			Expect(asciiName("...And Justice for All.zip")).To(Equal("...And Justice for All.zip"))
+		})
+
+		It("trims surrounding spaces and trailing dots", func() {
+			Expect(asciiName("  Greatest Hits  .zip")).To(Equal("Greatest Hits.zip"))
+			Expect(asciiName("Loose End. ")).To(Equal("Loose End"))
+		})
+
+		It("falls back to a placeholder when only dots are left", func() {
+			Expect(str.ContentDispositionAttachment("...")).To(Equal(`attachment; filename="download"`))
+		})
+
+		It("caps a long name whose last dot is not an extension", func() {
+			name := asciiName("a." + strings.Repeat("x", 300))
+			Expect(len(name)).To(BeNumerically("<=", 255))
+			Expect(name).To(HavePrefix("a.xxx"))
+		})
+
+		It("replaces path separators and reserved characters", func() {
+			Expect(asciiName("AC/DC: Live, 1979?.zip")).To(Equal("AC_DC_ Live_ 1979_.zip"))
+		})
+
+		It("drops control characters", func() {
+			Expect(asciiName("line\r\nbreak\x00\t.mp3")).To(Equal("linebreak.mp3"))
+		})
+	})
+
+	Describe("non-ASCII names", func() {
+		It("transliterates accents in the ASCII fallback", func() {
+			Expect(asciiName("Legião Urbana.zip")).To(Equal("Legiao Urbana.zip"))
+		})
+
+		It("converts typographic punctuation in the ASCII fallback", func() {
+			Expect(asciiName("She’s a Woman — Live.mp3")).To(Equal("She's a Woman - Live.mp3"))
+			Expect(asciiName("“Heroes”.mp3")).To(Equal("_Heroes_.mp3"))
+			Expect(decodedName("She’s a Woman.mp3")).To(Equal("She’s a Woman.mp3"))
+		})
+
+		It("keeps the extension when no ASCII letters survive", func() {
+			Expect(asciiName("東京.mp3")).To(Equal("download.mp3"))
+			Expect(asciiName("Кино.m3u")).To(Equal("download.m3u"))
+			Expect(asciiName("東京")).To(Equal("download"))
+		})
+
+		DescribeTable("filename* carries the sanitized UTF-8 name",
+			func(filename, expected string) {
+				Expect(decodedName(filename)).To(Equal(expected))
+			},
+			Entry("accented", "Legião Urbana.zip", "Legião Urbana.zip"),
+			Entry("CJK", "東京.zip", "東京.zip"),
+			Entry("emoji", "🎵 mix.zip", "🎵 mix.zip"),
+			Entry("comma and percent", "Sigur Rós, 100%.zip", "Sigur Rós, 100%.zip"),
+			Entry("path separators", "Sigur Rós/Live: Heima.zip", "Sigur Rós_Live_ Heima.zip"),
+			Entry("control characters", "Sigur Rós\r\n\x00.zip", "Sigur Rós.zip"),
+			Entry("bidi override", "Björk\u202Eexe.mp3", "Björkexe.mp3"),
+			Entry("invalid UTF-8", "Bj\xf6rk Café.mp3", "Bj_rk Café.mp3"),
+		)
+
+		It("caps the name length and keeps the extension", func() {
+			name := decodedName(strings.Repeat("東", 300) + ".zip")
+			Expect(len(name)).To(BeNumerically("<=", 255))
+			Expect(name).To(HaveSuffix("東.zip"))
+		})
+	})
+})