Skip to content

Commit bf7f55e

Browse files
authored
feat(template): extract CSP to a function, systematically use nonces, and use default-src 'none' instead of self
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.
1 parent b8bc367 commit bf7f55e

File tree

3 files changed

+131
-11
lines changed

3 files changed

+131
-11
lines changed

internal/template/functions.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ type funcMap struct {
3333
func (f *funcMap) Map() template.FuncMap {
3434
return template.FuncMap{
3535
"contains": strings.Contains,
36+
"csp": csp,
3637
"startsWith": strings.HasPrefix,
3738
"formatFileSize": formatFileSize,
3839
"dict": dict,
@@ -116,6 +117,40 @@ func (f *funcMap) Map() template.FuncMap {
116117
}
117118
}
118119

120+
func csp(user *model.User, nonce string) string {
121+
policies := map[string]string{
122+
"default-src": "'none'",
123+
"frame-src": "*",
124+
"img-src": "* data:",
125+
"manifest-src": "'self'",
126+
"media-src": "*",
127+
"require-trusted-types-for": "'script'",
128+
"script-src": "'nonce-" + nonce + "' 'strict-dynamic'",
129+
"style-src": "'nonce-" + nonce + "'",
130+
"trusted-types": "html url",
131+
"connect-src": "'self'",
132+
}
133+
134+
if user != nil {
135+
if user.ExternalFontHosts != "" {
136+
policies["font-src"] = user.ExternalFontHosts
137+
if user.Stylesheet != "" {
138+
policies["style-src"] += " " + user.ExternalFontHosts
139+
}
140+
}
141+
}
142+
143+
var policy strings.Builder
144+
for key, value := range policies {
145+
policy.WriteString(key)
146+
policy.WriteString(" ")
147+
policy.WriteString(value)
148+
policy.WriteString("; ")
149+
}
150+
151+
return `<meta http-equiv="Content-Security-Policy" content="` + policy.String() + `">`
152+
}
153+
119154
func dict(values ...any) (map[string]any, error) {
120155
if len(values)%2 != 0 {
121156
return nil, fmt.Errorf("dict expects an even number of arguments")

internal/template/functions_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
package template // import "miniflux.app/v2/internal/template"
55

66
import (
7+
"strings"
78
"testing"
89
"time"
910

1011
"miniflux.app/v2/internal/locale"
12+
"miniflux.app/v2/internal/model"
1113
)
1214

1315
func TestDict(t *testing.T) {
@@ -159,3 +161,92 @@ func TestFormatFileSize(t *testing.T) {
159161
}
160162
}
161163
}
164+
165+
func TestCSPExternalFont(t *testing.T) {
166+
want := []string{
167+
`default-src 'none';`,
168+
`img-src * data:;`,
169+
`media-src *;`,
170+
`frame-src *;`,
171+
`style-src 'nonce-1234';`,
172+
`script-src 'nonce-1234'`,
173+
`'strict-dynamic';`,
174+
`font-src test.com;`,
175+
`require-trusted-types-for 'script';`,
176+
`trusted-types html url;`,
177+
`manifest-src 'self';`,
178+
}
179+
got := csp(&model.User{ExternalFontHosts: "test.com"}, "1234")
180+
181+
for _, value := range want {
182+
if !strings.Contains(got, value) {
183+
t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
184+
}
185+
}
186+
}
187+
188+
func TestCSPNoUser(t *testing.T) {
189+
want := []string{
190+
`default-src 'none';`,
191+
`img-src * data:;`,
192+
`media-src *;`,
193+
`frame-src *;`,
194+
`style-src 'nonce-1234';`,
195+
`script-src 'nonce-1234'`,
196+
`'strict-dynamic';`,
197+
`require-trusted-types-for 'script';`,
198+
`trusted-types html url;`,
199+
`manifest-src 'self';`,
200+
}
201+
got := csp(nil, "1234")
202+
203+
for _, value := range want {
204+
if !strings.Contains(got, value) {
205+
t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
206+
}
207+
}
208+
}
209+
210+
func TestCSPCustomJSExternalFont(t *testing.T) {
211+
want := []string{
212+
`default-src 'none';`,
213+
`img-src * data:;`,
214+
`media-src *;`,
215+
`frame-src *;`,
216+
`style-src 'nonce-1234';`,
217+
`script-src 'nonce-1234'`,
218+
`'strict-dynamic';`,
219+
`require-trusted-types-for 'script';`,
220+
`trusted-types html url;`,
221+
`manifest-src 'self';`,
222+
}
223+
got := csp(&model.User{ExternalFontHosts: "test.com", CustomJS: "alert(1)"}, "1234")
224+
225+
for _, value := range want {
226+
if !strings.Contains(got, value) {
227+
t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
228+
}
229+
}
230+
}
231+
232+
func TestCSPExternalFontStylesheet(t *testing.T) {
233+
want := []string{
234+
`default-src 'none';`,
235+
`img-src * data:;`,
236+
`media-src *;`,
237+
`frame-src *;`,
238+
`style-src 'nonce-1234' test.com;`,
239+
`script-src 'nonce-1234'`,
240+
`'strict-dynamic';`,
241+
`require-trusted-types-for 'script';`,
242+
`trusted-types html url;`,
243+
`manifest-src 'self';`,
244+
}
245+
got := csp(&model.User{ExternalFontHosts: "test.com", Stylesheet: "a {color: red;}"}, "1234")
246+
247+
for _, value := range want {
248+
if !strings.Contains(got, value) {
249+
t.Errorf(`Unexpected result, didn't find %q in %q`, value, got)
250+
}
251+
}
252+
}

internal/template/templates/common/layout.html

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,18 @@
2525
<link rel="apple-touch-icon" sizes="167x167" href="{{ route "appIcon" "filename" "icon-167.png" }}">
2626
<link rel="apple-touch-icon" sizes="180x180" href="{{ route "appIcon" "filename" "icon-180.png" }}">
2727

28-
<link rel="stylesheet" type="text/css" href="{{ route "stylesheet" "name" .theme "checksum" .theme_checksum }}">
29-
30-
{{ if .user }}
31-
{{ $cspNonce := nonce }}
32-
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; img-src * data:; media-src *; frame-src *; {{ if .user.ExternalFontHosts }}font-src {{ .user.ExternalFontHosts }}; {{ end }}style-src 'self'{{ if .user.Stylesheet }}{{ if .user.ExternalFontHosts }} {{ .user.ExternalFontHosts }}{{ end }} 'nonce-{{ $cspNonce }}'{{ end }}{{ if .user.CustomJS }}; script-src 'self' 'nonce-{{ $cspNonce }}'{{ end }}; require-trusted-types-for 'script'; trusted-types html url;">
33-
28+
{{ $cspNonce := nonce }}
29+
{{ csp .user $cspNonce | safeHTML }}
30+
<link rel="stylesheet" nonce="{{ $cspNonce }}" type="text/css" href="{{ route "stylesheet" "name" .theme "checksum" .theme_checksum }}">
31+
<script nonce="{{ $cspNonce }}" src="{{ route "javascript" "name" "app" "checksum" .app_js_checksum }}" type="module"></script>
32+
{{ if .user -}}
3433
{{ if .user.Stylesheet -}}
3534
<style nonce="{{ $cspNonce }}">{{ .user.Stylesheet | safeCSS }}</style>
3635
{{ end -}}
37-
3836
{{ if .user.CustomJS -}}
3937
<script type="module" nonce="{{ $cspNonce }}">{{ .user.CustomJS | safeJS }}</script>
4038
{{ end -}}
41-
{{ else -}}
42-
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; img-src * data:; media-src *; frame-src *; require-trusted-types-for 'script'; trusted-types html url;">
4339
{{ end -}}
44-
45-
<script src="{{ route "javascript" "name" "app" "checksum" .app_js_checksum }}" type="module"></script>
4640
</head>
4741
<body
4842
data-service-worker-url="{{ route "javascript" "name" "service-worker" "checksum" .sw_js_checksum }}"

0 commit comments

Comments
 (0)