Skip to content

Commit 561c023

Browse files
authoredApr 19, 2024··
[Fizz] escape <script> textContent similar to bootstrapScript (#28871)
stacked on #28870 inline script children have been encoded as HTML for a while now but this can easily break script parsing so practically if you were rendering inline scripts you were using dangerouslySetInnerHTML. This is not great because now there is no escaping at all so you have to be even more careful. While care should always be taken when rendering untrusted script content driving users to use dangerous APIs is not the right approach and in this PR the escaping functionality used for bootstrapScripts and importMaps is being extended to any inline script. the approach is to escape 's' or 'S" with the appropriate unicode code point if it is inside a <script or </script sequence. This has the nice benefit of minimally escaping the text for readability while still preserving full js parsing capabilities. As articulated when we introduced this escaping for prior use cases this is only safe because we are escaping the entire script content. It would be unsafe if we were not escaping the entirety of the script because we would no longer be able to ensure there are no earlier or later <script sequences that put the parser in unexpected states.
1 parent aead514 commit 561c023

File tree

2 files changed

+88
-71
lines changed

2 files changed

+88
-71
lines changed
 

‎packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

+6-10
Original file line numberDiff line numberDiff line change
@@ -294,16 +294,16 @@ const scriptCrossOrigin = stringToPrecomputedChunk('" crossorigin="');
294294
const endAsyncScript = stringToPrecomputedChunk('" async=""></script>');
295295

296296
/**
297-
* This escaping function is designed to work with bootstrapScriptContent and importMap only.
298-
* because we know we are escaping the entire script. We can avoid for instance
297+
* This escaping function is designed to work with with inline scripts where the entire
298+
* contents are escaped. Because we know we are escaping the entire script we can avoid for instance
299299
* escaping html comment string sequences that are valid javascript as well because
300300
* if there are no sebsequent <script sequences the html parser will never enter
301301
* script data double escaped state (see: https://www.w3.org/TR/html53/syntax.html#script-data-double-escaped-state)
302302
*
303303
* While untrusted script content should be made safe before using this api it will
304304
* ensure that the script cannot be early terminated or never terminated state
305305
*/
306-
function escapeBootstrapAndImportMapScriptContent(scriptText: string) {
306+
function escapeEntireInlineScriptContent(scriptText: string) {
307307
if (__DEV__) {
308308
checkHtmlStringCoercion(scriptText);
309309
}
@@ -372,9 +372,7 @@ export function createRenderState(
372372
if (bootstrapScriptContent !== undefined) {
373373
bootstrapChunks.push(
374374
inlineScriptWithNonce,
375-
stringToChunk(
376-
escapeBootstrapAndImportMapScriptContent(bootstrapScriptContent),
377-
),
375+
stringToChunk(escapeEntireInlineScriptContent(bootstrapScriptContent)),
378376
endInlineScript,
379377
);
380378
}
@@ -411,9 +409,7 @@ export function createRenderState(
411409
const map = importMap;
412410
importMapChunks.push(importMapScriptStart);
413411
importMapChunks.push(
414-
stringToChunk(
415-
escapeBootstrapAndImportMapScriptContent(JSON.stringify(map)),
416-
),
412+
stringToChunk(escapeEntireInlineScriptContent(JSON.stringify(map))),
417413
);
418414
importMapChunks.push(importMapScriptEnd);
419415
}
@@ -3266,7 +3262,7 @@ function pushScriptImpl(
32663262

32673263
pushInnerHTML(target, innerHTML, children);
32683264
if (typeof children === 'string') {
3269-
target.push(stringToChunk(encodeHTMLTextNode(children)));
3265+
target.push(stringToChunk(escapeEntireInlineScriptContent(children)));
32703266
}
32713267
target.push(endChunkForTag('script'));
32723268
return null;

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

+82-61
Original file line numberDiff line numberDiff line change
@@ -4180,79 +4180,100 @@ describe('ReactDOMFizzServer', () => {
41804180
]);
41814181
});
41824182

4183-
describe('bootstrapScriptContent and importMap escaping', () => {
4184-
it('the "S" in "</?[Ss]cript" strings are replaced with unicode escaped lowercase s or S depending on case, preserving case sensitivity of nearby characters', async () => {
4185-
window.__test_outlet = '';
4186-
const stringWithScriptsInIt =
4187-
'prescription pre<scription pre<Scription pre</scRipTion pre</ScripTion </script><script><!-- <script> -->';
4188-
await act(() => {
4189-
const {pipe} = renderToPipeableStream(<div />, {
4190-
bootstrapScriptContent:
4191-
'window.__test_outlet = "This should have been replaced";var x = "' +
4192-
stringWithScriptsInIt +
4193-
'";\nwindow.__test_outlet = x;',
4183+
describe('inline script escaping', () => {
4184+
describe('bootstrapScriptContent', () => {
4185+
it('the "S" in "</?[Ss]cript" strings are replaced with unicode escaped lowercase s or S depending on case, preserving case sensitivity of nearby characters', async () => {
4186+
window.__test_outlet = '';
4187+
const stringWithScriptsInIt =
4188+
'prescription pre<scription pre<Scription pre</scRipTion pre</ScripTion </script><script><!-- <script> -->';
4189+
await act(() => {
4190+
const {pipe} = renderToPipeableStream(<div />, {
4191+
bootstrapScriptContent:
4192+
'window.__test_outlet = "This should have been replaced";var x = "' +
4193+
stringWithScriptsInIt +
4194+
'";\nwindow.__test_outlet = x;',
4195+
});
4196+
pipe(writable);
41944197
});
4195-
pipe(writable);
4198+
expect(window.__test_outlet).toMatch(stringWithScriptsInIt);
41964199
});
4197-
expect(window.__test_outlet).toMatch(stringWithScriptsInIt);
4198-
});
4199-
4200-
it('does not escape \\u2028, or \\u2029 characters', async () => {
4201-
// these characters are ignored in engines support https://github.com/tc39/proposal-json-superset
4202-
// in this test with JSDOM the characters are silently dropped and thus don't need to be encoded.
4203-
// if you send these characters to an older browser they could fail so it is a good idea to
4204-
// sanitize JSON input of these characters
4205-
window.__test_outlet = '';
4206-
const el = document.createElement('p');
4207-
el.textContent = '{"one":1,\u2028\u2029"two":2}';
4208-
const stringWithLSAndPSCharacters = el.textContent;
4209-
await act(() => {
4210-
const {pipe} = renderToPipeableStream(<div />, {
4211-
bootstrapScriptContent:
4212-
'let x = ' +
4213-
stringWithLSAndPSCharacters +
4214-
'; window.__test_outlet = x;',
4200+
4201+
it('does not escape \\u2028, or \\u2029 characters', async () => {
4202+
// these characters are ignored in engines support https://github.com/tc39/proposal-json-superset
4203+
// in this test with JSDOM the characters are silently dropped and thus don't need to be encoded.
4204+
// if you send these characters to an older browser they could fail so it is a good idea to
4205+
// sanitize JSON input of these characters
4206+
window.__test_outlet = '';
4207+
const el = document.createElement('p');
4208+
el.textContent = '{"one":1,\u2028\u2029"two":2}';
4209+
const stringWithLSAndPSCharacters = el.textContent;
4210+
await act(() => {
4211+
const {pipe} = renderToPipeableStream(<div />, {
4212+
bootstrapScriptContent:
4213+
'let x = ' +
4214+
stringWithLSAndPSCharacters +
4215+
'; window.__test_outlet = x;',
4216+
});
4217+
pipe(writable);
42154218
});
4216-
pipe(writable);
4219+
const outletString = JSON.stringify(window.__test_outlet);
4220+
expect(outletString).toBe(
4221+
stringWithLSAndPSCharacters.replace(/[\u2028\u2029]/g, ''),
4222+
);
4223+
});
4224+
4225+
it('does not escape <, >, or & characters', async () => {
4226+
// these characters valid javascript and may be necessary in scripts and won't be interpretted properly
4227+
// escaped outside of a string context within javascript
4228+
window.__test_outlet = null;
4229+
// this boolean expression will be cast to a number due to the bitwise &. we will look for a truthy value (1) below
4230+
const booleanLogicString = '1 < 2 & 3 > 1';
4231+
await act(() => {
4232+
const {pipe} = renderToPipeableStream(<div />, {
4233+
bootstrapScriptContent:
4234+
'let x = ' + booleanLogicString + '; window.__test_outlet = x;',
4235+
});
4236+
pipe(writable);
4237+
});
4238+
expect(window.__test_outlet).toBe(1);
42174239
});
4218-
const outletString = JSON.stringify(window.__test_outlet);
4219-
expect(outletString).toBe(
4220-
stringWithLSAndPSCharacters.replace(/[\u2028\u2029]/g, ''),
4221-
);
42224240
});
42234241

4224-
it('does not escape <, >, or & characters', async () => {
4225-
// these characters valid javascript and may be necessary in scripts and won't be interpretted properly
4226-
// escaped outside of a string context within javascript
4227-
window.__test_outlet = null;
4228-
// this boolean expression will be cast to a number due to the bitwise &. we will look for a truthy value (1) below
4229-
const booleanLogicString = '1 < 2 & 3 > 1';
4230-
await act(() => {
4231-
const {pipe} = renderToPipeableStream(<div />, {
4232-
bootstrapScriptContent:
4233-
'let x = ' + booleanLogicString + '; window.__test_outlet = x;',
4242+
describe('importMaps', () => {
4243+
it('escapes </[sS]cirpt> in importMaps', async () => {
4244+
window.__test_outlet_key = '';
4245+
window.__test_outlet_value = '';
4246+
const jsonWithScriptsInIt = {
4247+
"keypos</script><script>window.__test_outlet_key = 'pwned'</script><script>":
4248+
'value',
4249+
key: "valuepos</script><script>window.__test_outlet_value = 'pwned'</script><script>",
4250+
};
4251+
await act(() => {
4252+
const {pipe} = renderToPipeableStream(<div />, {
4253+
importMap: jsonWithScriptsInIt,
4254+
});
4255+
pipe(writable);
42344256
});
4235-
pipe(writable);
4257+
expect(window.__test_outlet_key).toBe('');
4258+
expect(window.__test_outlet_value).toBe('');
42364259
});
4237-
expect(window.__test_outlet).toBe(1);
42384260
});
42394261

4240-
it('escapes </[sS]cirpt> in importMaps', async () => {
4241-
window.__test_outlet_key = '';
4242-
window.__test_outlet_value = '';
4243-
const jsonWithScriptsInIt = {
4244-
"keypos</script><script>window.__test_outlet_key = 'pwned'</script><script>":
4245-
'value',
4246-
key: "valuepos</script><script>window.__test_outlet_value = 'pwned'</script><script>",
4247-
};
4248-
await act(() => {
4249-
const {pipe} = renderToPipeableStream(<div />, {
4250-
importMap: jsonWithScriptsInIt,
4262+
describe('inline script', () => {
4263+
it('escapes </[sS]cirpt> in inline scripts', async () => {
4264+
window.__test_outlet = '';
4265+
await act(() => {
4266+
const {pipe} = renderToPipeableStream(
4267+
<script>{`
4268+
<!-- some html comment </script><script>window.__test_outlet = 'pwned'</script>
4269+
window.__test_outlet = 'safe';
4270+
--> </script><script>window.__test_outlet = 'pwned after'</script>
4271+
`}</script>,
4272+
);
4273+
pipe(writable);
42514274
});
4252-
pipe(writable);
4275+
expect(window.__test_outlet).toBe('safe');
42534276
});
4254-
expect(window.__test_outlet_key).toBe('');
4255-
expect(window.__test_outlet_value).toBe('');
42564277
});
42574278
});
42584279

0 commit comments

Comments
 (0)
Please sign in to comment.