Skip to content

Commit 6df00fc

Browse files
MathiasWPRich-Harriseltigerchino
authored
fix: csp nonce in script-src-elem, style-src-attr and style-src-elem when using unsafe-inline (#11613)
* add nonce to script-src-elem csp directive if defined * added changeset * also handle hashes and style-src-attr and style-src-elem * changed order of variable declaration * fixed typo * updated changeset * fix bug and update test * update test * write better tests and fix bugs * Update .changeset/giant-years-drum.md * fix csp nonce in script-src-elem, style-src-attr and style-src-elem when using unsafe-inline * fix test desc * Update giant-years-drum.md * refactor * rename variable * avoid duplicate empty comment hash * add back removed test * added back another test * add back skips nonce in style-src when using unsafe-inline test * put tests in same order --------- Co-authored-by: Rich Harris <hello@rich-harris.dev> Co-authored-by: Chew Tee Ming <chew.tee.ming@nindatech.com>
1 parent c717db9 commit 6df00fc

File tree

3 files changed

+117
-67
lines changed

3 files changed

+117
-67
lines changed

‎.changeset/giant-years-drum.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@sveltejs/kit": patch
3+
---
4+
5+
fix: only add nonce to `script-src-elem`, `style-src-attr` and `style-src-elem` CSP directives when `unsafe-inline` is not present

‎packages/kit/src/runtime/server/page/csp.js

+67-63
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,24 @@ class BaseProvider {
3131
/** @type {boolean} */
3232
#script_needs_csp;
3333

34+
/** @type {boolean} */
35+
#script_src_needs_csp;
36+
37+
/** @type {boolean} */
38+
#script_src_elem_needs_csp;
39+
3440
/** @type {boolean} */
3541
#style_needs_csp;
3642

43+
/** @type {boolean} */
44+
#style_src_needs_csp;
45+
46+
/** @type {boolean} */
47+
#style_src_attr_needs_csp;
48+
49+
/** @type {boolean} */
50+
#style_src_elem_needs_csp;
51+
3752
/** @type {import('types').CspDirectives} */
3853
#directives;
3954

@@ -121,92 +136,81 @@ class BaseProvider {
121136
}
122137
}
123138

124-
this.#script_needs_csp =
125-
(!!effective_script_src &&
126-
effective_script_src.filter((value) => value !== 'unsafe-inline').length > 0) ||
127-
(!!script_src_elem &&
128-
script_src_elem.filter((value) => value !== 'unsafe-inline').length > 0);
139+
/** @param {(import('types').Csp.Source | import('types').Csp.ActionSource)[] | undefined} directive */
140+
const needs_csp = (directive) =>
141+
!!directive && !directive.some((value) => value === 'unsafe-inline');
129142

143+
this.#script_src_needs_csp = needs_csp(effective_script_src);
144+
this.#script_src_elem_needs_csp = needs_csp(script_src_elem);
145+
this.#style_src_needs_csp = needs_csp(effective_style_src);
146+
this.#style_src_attr_needs_csp = needs_csp(style_src_attr);
147+
this.#style_src_elem_needs_csp = needs_csp(style_src_elem);
148+
149+
this.#script_needs_csp = this.#script_src_needs_csp || this.#script_src_elem_needs_csp;
130150
this.#style_needs_csp =
131151
!__SVELTEKIT_DEV__ &&
132-
((!!effective_style_src &&
133-
effective_style_src.filter((value) => value !== 'unsafe-inline').length > 0) ||
134-
(!!style_src_attr &&
135-
style_src_attr.filter((value) => value !== 'unsafe-inline').length > 0) ||
136-
(!!style_src_elem &&
137-
style_src_elem.filter((value) => value !== 'unsafe-inline').length > 0));
152+
(this.#style_src_needs_csp ||
153+
this.#style_src_attr_needs_csp ||
154+
this.#style_src_elem_needs_csp);
138155

139156
this.script_needs_nonce = this.#script_needs_csp && !this.#use_hashes;
140157
this.style_needs_nonce = this.#style_needs_csp && !this.#use_hashes;
158+
141159
this.#nonce = nonce;
142160
}
143161

144162
/** @param {string} content */
145163
add_script(content) {
146-
if (this.#script_needs_csp) {
147-
const d = this.#directives;
164+
if (!this.#script_needs_csp) return;
148165

149-
if (this.#use_hashes) {
150-
const hash = sha256(content);
151-
152-
this.#script_src.push(`sha256-${hash}`);
153-
154-
if (d['script-src-elem']?.length) {
155-
this.#script_src_elem.push(`sha256-${hash}`);
156-
}
157-
} else {
158-
if (this.#script_src.length === 0) {
159-
this.#script_src.push(`nonce-${this.#nonce}`);
160-
}
161-
if (d['script-src-elem']?.length) {
162-
this.#script_src_elem.push(`nonce-${this.#nonce}`);
163-
}
164-
}
166+
/** @type {`nonce-${string}` | `sha256-${string}`} */
167+
const source = this.#use_hashes ? `sha256-${sha256(content)}` : `nonce-${this.#nonce}`;
168+
169+
if (this.#script_src_needs_csp) {
170+
this.#script_src.push(source);
171+
}
172+
173+
if (this.#script_src_elem_needs_csp) {
174+
this.#script_src_elem.push(source);
165175
}
166176
}
167177

168178
/** @param {string} content */
169179
add_style(content) {
170-
if (this.#style_needs_csp) {
171-
// this is the hash for "/* empty */"
172-
// adding it so that svelte does not break csp
173-
// see https://github.com/sveltejs/svelte/pull/7800
174-
const empty_comment_hash = '9OlNO0DNEeaVzHL4RZwCLsBHA8WBQ8toBp/4F5XV2nc=';
180+
if (!this.#style_needs_csp) return;
175181

176-
const d = this.#directives;
182+
/** @type {`nonce-${string}` | `sha256-${string}`} */
183+
const source = this.#use_hashes ? `sha256-${sha256(content)}` : `nonce-${this.#nonce}`;
177184

178-
if (this.#use_hashes) {
179-
const hash = sha256(content);
185+
if (this.#style_src_needs_csp) {
186+
this.#style_src.push(source);
187+
}
180188

181-
this.#style_src.push(`sha256-${hash}`);
189+
if (this.#style_src_needs_csp) {
190+
this.#style_src.push(source);
191+
}
182192

183-
if (d['style-src-attr']?.length) {
184-
this.#style_src_attr.push(`sha256-${hash}`);
185-
}
186-
if (d['style-src-elem']?.length) {
187-
if (
188-
hash !== empty_comment_hash &&
189-
!d['style-src-elem'].includes(`sha256-${empty_comment_hash}`)
190-
) {
191-
this.#style_src_elem.push(`sha256-${empty_comment_hash}`);
192-
}
193+
if (this.#style_src_attr_needs_csp) {
194+
this.#style_src_attr.push(source);
195+
}
193196

194-
this.#style_src_elem.push(`sha256-${hash}`);
195-
}
196-
} else {
197-
if (this.#style_src.length === 0 && !d['style-src']?.includes('unsafe-inline')) {
198-
this.#style_src.push(`nonce-${this.#nonce}`);
199-
}
200-
if (d['style-src-attr']?.length) {
201-
this.#style_src_attr.push(`nonce-${this.#nonce}`);
202-
}
203-
if (d['style-src-elem']?.length) {
204-
if (!d['style-src-elem'].includes(`sha256-${empty_comment_hash}`)) {
205-
this.#style_src_elem.push(`sha256-${empty_comment_hash}`);
206-
}
197+
if (this.#style_src_elem_needs_csp) {
198+
// this is the sha256 hash for the string "/* empty */"
199+
// adding it so that svelte does not break csp
200+
// see https://github.com/sveltejs/svelte/pull/7800
201+
const sha256_empty_comment_hash = 'sha256-9OlNO0DNEeaVzHL4RZwCLsBHA8WBQ8toBp/4F5XV2nc=';
202+
const d = this.#directives;
203+
204+
if (
205+
d['style-src-elem'] &&
206+
!d['style-src-elem'].includes(sha256_empty_comment_hash) &&
207+
!this.#style_src_elem.includes(sha256_empty_comment_hash)
208+
) {
209+
this.#style_src_elem.push(sha256_empty_comment_hash);
210+
}
207211

208-
this.#style_src_elem.push(`nonce-${this.#nonce}`);
209-
}
212+
if (source !== sha256_empty_comment_hash) {
213+
this.#style_src_elem.push(source);
210214
}
211215
}
212216
}

‎packages/kit/src/runtime/server/page/csp.spec.js

+45-4
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,20 @@ test('skips nonce with unsafe-inline', () => {
8484
{
8585
mode: 'nonce',
8686
directives: {
87-
'default-src': ['unsafe-inline']
87+
'default-src': ['unsafe-inline'],
88+
'script-src': ['unsafe-inline'],
89+
'script-src-elem': ['unsafe-inline'],
90+
'style-src': ['unsafe-inline'],
91+
'style-src-attr': ['unsafe-inline'],
92+
'style-src-elem': ['unsafe-inline']
8893
},
8994
reportOnly: {
9095
'default-src': ['unsafe-inline'],
96+
'script-src': ['unsafe-inline'],
97+
'script-src-elem': ['unsafe-inline'],
98+
'style-src': ['unsafe-inline'],
99+
'style-src-attr': ['unsafe-inline'],
100+
'style-src-elem': ['unsafe-inline'],
91101
'report-uri': ['/']
92102
}
93103
},
@@ -97,9 +107,16 @@ test('skips nonce with unsafe-inline', () => {
97107
);
98108

99109
csp.add_script('');
110+
csp.add_style('');
100111

101-
assert.equal(csp.csp_provider.get_header(), "default-src 'unsafe-inline'");
102-
assert.equal(csp.report_only_provider.get_header(), "default-src 'unsafe-inline'; report-uri /");
112+
assert.equal(
113+
csp.csp_provider.get_header(),
114+
"default-src 'unsafe-inline'; script-src 'unsafe-inline'; script-src-elem 'unsafe-inline'; style-src 'unsafe-inline'; style-src-attr 'unsafe-inline'; style-src-elem 'unsafe-inline'"
115+
);
116+
assert.equal(
117+
csp.report_only_provider.get_header(),
118+
"default-src 'unsafe-inline'; script-src 'unsafe-inline'; script-src-elem 'unsafe-inline'; style-src 'unsafe-inline'; style-src-attr 'unsafe-inline'; style-src-elem 'unsafe-inline'; report-uri /"
119+
);
103120
});
104121

105122
test('skips nonce in style-src when using unsafe-inline', () => {
@@ -151,6 +168,30 @@ test('skips hash with unsafe-inline', () => {
151168
assert.equal(csp.report_only_provider.get_header(), "default-src 'unsafe-inline'; report-uri /");
152169
});
153170

171+
test('does not add empty comment hash to style-src-elem if already defined', () => {
172+
const csp = new Csp(
173+
{
174+
mode: 'hash',
175+
directives: {
176+
'style-src-elem': ['self', 'sha256-9OlNO0DNEeaVzHL4RZwCLsBHA8WBQ8toBp/4F5XV2nc=']
177+
},
178+
reportOnly: {
179+
'report-uri': ['/']
180+
}
181+
},
182+
{
183+
prerender: false
184+
}
185+
);
186+
187+
csp.add_style('/* empty */');
188+
189+
assert.equal(
190+
csp.csp_provider.get_header(),
191+
"style-src-elem 'self' 'sha256-9OlNO0DNEeaVzHL4RZwCLsBHA8WBQ8toBp/4F5XV2nc='"
192+
);
193+
});
194+
154195
test('skips frame-ancestors, report-uri, sandbox from meta tags', () => {
155196
const csp = new Csp(
156197
{
@@ -179,7 +220,7 @@ test('skips frame-ancestors, report-uri, sandbox from meta tags', () => {
179220
);
180221
});
181222

182-
test('adds nonce to script-src-elem, style-src-attr and style-src-elem if necessary', () => {
223+
test('adds nonce style-src-attr and style-src-elem and nonce + sha to script-src-elem if necessary', () => {
183224
const csp = new Csp(
184225
{
185226
mode: 'auto',

0 commit comments

Comments
 (0)