From f102036dc6a4df88ea9e4ef53437f65f2bb7d01d Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 5 Mar 2026 20:56:16 -0500 Subject: [PATCH] fix(server): clear server-managed fields in savePlaylist to prevent injection via REST API Signed-off-by: Deluan --- core/playlists/rest_adapter.go | 8 ++++++- core/playlists/rest_adapter_test.go | 34 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/core/playlists/rest_adapter.go b/core/playlists/rest_adapter.go index 3865d97e..c9cd4c13 100644 --- a/core/playlists/rest_adapter.go +++ b/core/playlists/rest_adapter.go @@ -58,10 +58,16 @@ func (s *playlists) TracksRepository(ctx context.Context, playlistId string, ref } // savePlaylist creates a new playlist, assigning the owner from context. +// Only Name, Comment, Public, and Rules are user-settable via the REST API. func (s *playlists) savePlaylist(ctx context.Context, pls *model.Playlist) (string, error) { usr, _ := request.UserFrom(ctx) pls.OwnerID = usr.ID - pls.ID = "" // Force new creation + pls.ID = "" // Force new creation + pls.Path = "" // Server-managed (M3U file path) + pls.Sync = false // Server-managed (M3U sync flag) + pls.UploadedImage = "" // Managed by image upload endpoint + pls.ExternalImageURL = "" // Managed by M3U import / plugins only + pls.EvaluatedAt = nil // Server-managed err := s.ds.Playlist(ctx).Put(pls) if err != nil { return "", err diff --git a/core/playlists/rest_adapter_test.go b/core/playlists/rest_adapter_test.go index b6509595..29db2fc3 100644 --- a/core/playlists/rest_adapter_test.go +++ b/core/playlists/rest_adapter_test.go @@ -2,10 +2,12 @@ package playlists_test import ( "context" + "time" "github.com/deluan/rest" "github.com/navidrome/navidrome/core/playlists" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/criteria" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -56,6 +58,38 @@ var _ = Describe("REST Adapter", func() { Expect(err).ToNot(HaveOccurred()) Expect(pls.ID).ToNot(Equal("should-be-cleared")) }) + + It("clears server-managed fields to prevent injection via REST API", func() { + ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) + repo = ps.NewRepository(ctx).(rest.Persistable) + now := time.Now() + pls := &model.Playlist{ + Name: "Legit Playlist", + Comment: "A comment", + Public: true, + Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}, + Path: "/some/path/playlist.m3u", + Sync: true, + UploadedImage: "injected-image-path", + ExternalImageURL: "http://evil.example.com/ssrf", + EvaluatedAt: &now, + } + _, err := repo.Save(pls) + Expect(err).ToNot(HaveOccurred()) + + saved := mockPlsRepo.Last + // User-settable fields are preserved + Expect(saved.Name).To(Equal("Legit Playlist")) + Expect(saved.Comment).To(Equal("A comment")) + Expect(saved.Public).To(BeTrue()) + Expect(saved.Rules).ToNot(BeNil()) + // Server-managed fields are cleared + Expect(saved.Path).To(BeEmpty()) + Expect(saved.Sync).To(BeFalse()) + Expect(saved.UploadedImage).To(BeEmpty()) + Expect(saved.ExternalImageURL).To(BeEmpty()) + Expect(saved.EvaluatedAt).To(BeNil()) + }) }) Describe("Update", func() {