From 65b86b1bcc9125ca3c22f11339cec8b984a96207 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Fri, 22 Aug 2025 14:05:10 +0200 Subject: [PATCH] refactor(template): extract the CSP in a function and systematically use nonces. Having the CSP built in a function instead of in the template makes it easier to properly construct it. This was also the opportunity to switch from default-src 'self' to default-src 'none', to deny everything that isn't explicitly allowed, instead of allowing everything coming from 'self'. Moreover, as Miniflux is shoving the content of feeds in the same origin as itself, using self doesn't do much security-wise. It's much better to systematically use a nonce-based policy, so that an attacker able to bypass the sanitization will have to guess the nonce to gain arbitrary javascript execution. While the merge-request has been tested locally, it would still be prudent to thoroughly test it before merging, as it has the potential to break the user-interface should weird constructs be used. --- internal/template/functions.go | 37 +++++++++++++++++++ internal/template/functions_test.go | 9 +++++ .../template/templates/common/layout.html | 16 +++----- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/internal/template/functions.go b/internal/template/functions.go index 2a478cc5..68c5b9a8 100644 --- a/internal/template/functions.go +++ b/internal/template/functions.go @@ -33,6 +33,7 @@ type funcMap struct { func (f *funcMap) Map() template.FuncMap { return template.FuncMap{ "contains": strings.Contains, + "csp": csp, "startsWith": strings.HasPrefix, "formatFileSize": formatFileSize, "dict": dict, @@ -116,6 +117,42 @@ func (f *funcMap) Map() template.FuncMap { } } +func csp(user *model.User, nonce string) string { + order := [...]string{"default-src", "img-src", "media-src", "frame-src", "style-src", "script-src", "font-src", "require-trusted-types-for", "trusted-types"} + policies := map[string]string{ + "default-src": "'none'", + "img-src": "* data:", + "media-src": "*", + "frame-src": "*", + "style-src": "'nonce-" + nonce + "'", + "script-src": "'nonce-" + nonce + "' 'strict-dynamic'", + "require-trusted-types-for": "'script'", + "trusted-types": "html url", + } + + if user != nil { + if user.ExternalFontHosts != "" { + policies["font-src"] = user.ExternalFontHosts + if user.Stylesheet != "" { + policies["style-src"] += " " + user.ExternalFontHosts + } + } + } + + var policy strings.Builder + // This is needed to always have the same order. + for _, key := range order { + if value, ok := policies[key]; ok { + policy.WriteString(key) + policy.WriteString(" ") + policy.WriteString(value) + policy.WriteString("; ") + } + } + + return `` +} + func dict(values ...any) (map[string]any, error) { if len(values)%2 != 0 { return nil, fmt.Errorf("dict expects an even number of arguments") diff --git a/internal/template/functions_test.go b/internal/template/functions_test.go index dd71f28f..9c8c8888 100644 --- a/internal/template/functions_test.go +++ b/internal/template/functions_test.go @@ -8,6 +8,7 @@ import ( "time" "miniflux.app/v2/internal/locale" + "miniflux.app/v2/internal/model" ) func TestDict(t *testing.T) { @@ -159,3 +160,11 @@ func TestFormatFileSize(t *testing.T) { } } } + +func TestCSP(t *testing.T) { + want := `` + got := csp(&model.User{ExternalFontHosts: "test.com"}, "1234") + if got != want { + t.Errorf(`Unexpected result, got %q instead of %q`, got, want) + } +} diff --git a/internal/template/templates/common/layout.html b/internal/template/templates/common/layout.html index 8c4f069d..ca3529c0 100644 --- a/internal/template/templates/common/layout.html +++ b/internal/template/templates/common/layout.html @@ -25,24 +25,18 @@ - - - {{ if .user }} - {{ $cspNonce := nonce }} - - + {{ $cspNonce := nonce }} + {{ csp .user $cspNonce | safeHTML }} + + + {{ if .user -}} {{ if .user.Stylesheet -}} {{ end -}} - {{ if .user.CustomJS -}} {{ end -}} - {{ else -}} - {{ end -}} - -