@samitouri / QOS-React / commits / f0e8164410

sanitize javascript: urls for <object> tags (#29808)

sanitize javascript: urls for <object> tags React 19 added sanitization for `javascript:` URLs for `href` properties on various tags. This PR also adds that sanitization for `<object>` tags as well that Firefox otherwise executes.

Jan Kassens committed Jun 14, 2024 at 13:17 UTC f0e8164410049aabc680035e8fd45b657eb020ab
4 files changed +159 -1
packages/react-dom-bindings/src/client/ReactDOMComponent.js
+16 -1
@@ -406,6 +406,12 @@ function setProp(
406 break;
407 }
408 // These attributes accept URLs. These must not allow javascript: URLS.
409 + case 'data':
410 + if (tag !== 'object') {
411 + setValueForKnownAttribute(domElement, 'data', value);
412 + break;
413 + }
414 + // fallthrough
415 case 'src':
416 case 'href': {
417 if (enableFilterEmptyStringAttributesDOM) {
@@ -2453,13 +2459,22 @@ function diffHydratedGenericElement(
2459 warnForPropDifference(propKey, serverValue, value, serverDifferences);
2460 continue;
2461 }
2462 + case 'data':
2463 + if (tag !== 'object') {
2464 + extraAttributes.delete(propKey);
2465 + const serverValue = (domElement: any).getAttribute('data');
2466 + warnForPropDifference(propKey, serverValue, value, serverDifferences);
2467 + continue;
2468 + }
2469 + // fallthrough
2470 case 'src':
2471 case 'href':
2472 if (enableFilterEmptyStringAttributesDOM) {
2473 if (
2474 value === '' &&
2475 // <a href=""> is fine for "reload" links.
2462 - !(tag === 'a' && propKey === 'href')
2476 + !(tag === 'a' && propKey === 'href') &&
2477 + !(tag === 'object' && propKey === 'data')
2478 ) {
2479 if (__DEV__) {
2480 if (propKey === 'src') {
packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
+69
@@ -1601,6 +1601,73 @@ function pushStartAnchor(
1601 return children;
1602 }
1603
1604 +function pushStartObject(
1605 + target: Array<Chunk | PrecomputedChunk>,
1606 + props: Object,
1607 +): ReactNodeList {
1608 + target.push(startChunkForTag('object'));
1609 +
1610 + let children = null;
1611 + let innerHTML = null;
1612 + for (const propKey in props) {
1613 + if (hasOwnProperty.call(props, propKey)) {
1614 + const propValue = props[propKey];
1615 + if (propValue == null) {
1616 + continue;
1617 + }
1618 + switch (propKey) {
1619 + case 'children':
1620 + children = propValue;
1621 + break;
1622 + case 'dangerouslySetInnerHTML':
1623 + innerHTML = propValue;
1624 + break;
1625 + case 'data': {
1626 + if (__DEV__) {
1627 + checkAttributeStringCoercion(propValue, 'data');
1628 + }
1629 + const sanitizedValue = sanitizeURL('' + propValue);
1630 + if (enableFilterEmptyStringAttributesDOM) {
1631 + if (sanitizedValue === '') {
1632 + if (__DEV__) {
1633 + console.error(
1634 + 'An empty string ("") was passed to the %s attribute. ' +
1635 + 'To fix this, either do not render the element at all ' +
1636 + 'or pass null to %s instead of an empty string.',
1637 + propKey,
1638 + propKey,
1639 + );
1640 + }
1641 + break;
1642 + }
1643 + }
1644 + target.push(
1645 + attributeSeparator,
1646 + stringToChunk('data'),
1647 + attributeAssign,
1648 + stringToChunk(escapeTextForBrowser(sanitizedValue)),
1649 + attributeEnd,
1650 + );
1651 + break;
1652 + }
1653 + default:
1654 + pushAttribute(target, propKey, propValue);
1655 + break;
1656 + }
1657 + }
1658 + }
1659 +
1660 + target.push(endOfStartTag);
1661 + pushInnerHTML(target, innerHTML, children);
1662 + if (typeof children === 'string') {
1663 + // Special case children as a string to avoid the unnecessary comment.
1664 + // TODO: Remove this special case after the general optimization is in place.
1665 + target.push(stringToChunk(encodeHTMLTextNode(children)));
1666 + return null;
1667 + }
1668 + return children;
1669 +}
1670 +
1671 function pushStartSelect(
1672 target: Array<Chunk | PrecomputedChunk>,
1673 props: Object,
@@ -3569,6 +3636,8 @@ export function pushStartInstance(
3636 return pushStartForm(target, props, resumableState, renderState);
3637 case 'menuitem':
3638 return pushStartMenuItem(target, props);
3639 + case 'object':
3640 + return pushStartObject(target, props);
3641 case 'title':
3642 return pushTitle(
3643 target,
packages/react-dom/src/__tests__/ReactDOMServerIntegrationObject-test.js new
+55
@@ -0,0 +1,55 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @jest-environment ./scripts/jest/ReactDOMServerIntegrationEnvironment
8 + */
9 +
10 +'use strict';
11 +
12 +const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');
13 +
14 +let React;
15 +let ReactDOMClient;
16 +let ReactDOMServer;
17 +
18 +function initModules() {
19 + // Reset warning cache.
20 + jest.resetModules();
21 + React = require('react');
22 + ReactDOMClient = require('react-dom/client');
23 + ReactDOMServer = require('react-dom/server');
24 +
25 + // Make them available to the helpers.
26 + return {
27 + ReactDOMClient,
28 + ReactDOMServer,
29 + };
30 +}
31 +
32 +const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);
33 +
34 +describe('ReactDOMServerIntegrationObject', () => {
35 + beforeEach(() => {
36 + resetModules();
37 + });
38 +
39 + itRenders('an object with children', async render => {
40 + const e = await render(
41 + <object type="video/mp4" data="/example.webm" width={600} height={400}>
42 + <div>preview</div>
43 + </object>,
44 + );
45 +
46 + expect(e.outerHTML).toBe(
47 + '<object type="video/mp4" data="/example.webm" width="600" height="400"><div>preview</div></object>',
48 + );
49 + });
50 +
51 + itRenders('an object with empty data', async render => {
52 + const e = await render(<object data="" />, 1);
53 + expect(e.outerHTML).toBe('<object></object>');
54 + });
55 +});
packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js
+19
@@ -69,6 +69,25 @@ describe('ReactDOMServerIntegration - Untrusted URLs', () => {
69 expect(e.lastChild.href).toBe(EXPECTED_SAFE_URL);
70 });
71
72 + itRenders('sanitizes on various tags', async render => {
73 + const aElement = await render(<a href="javascript:notfine" />);
74 + expect(aElement.href).toBe(EXPECTED_SAFE_URL);
75 +
76 + const objectElement = await render(<object data="javascript:notfine" />);
77 + expect(objectElement.data).toBe(EXPECTED_SAFE_URL);
78 +
79 + const embedElement = await render(<embed src="javascript:notfine" />);
80 + expect(embedElement.src).toBe(EXPECTED_SAFE_URL);
81 + });
82 +
83 + itRenders('passes through data on non-object tags', async render => {
84 + const div = await render(<div data="test" />);
85 + expect(div.getAttribute('data')).toBe('test');
86 +
87 + const a = await render(<a data="javascript:fine" />);
88 + expect(a.getAttribute('data')).toBe('javascript:fine');
89 + });
90 +
91 itRenders('a javascript protocol with leading spaces', async render => {
92 const e = await render(
93 <a href={' \t \u0000\u001F\u0003javascript\n: notfine'}>p0wned</a>,