Skip to content

Commit

Permalink
sanitize javascript: urls for <object> tags
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kassens committed Jun 12, 2024
1 parent f3e09d6 commit 475b518
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 1 deletion.
17 changes: 16 additions & 1 deletion packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,12 @@ function setProp(
break;
}
// These attributes accept URLs. These must not allow javascript: URLS.
case 'data':
if (tag !== 'object') {
setValueForKnownAttribute(domElement, 'data', value);
break;
}
// fallthrough
case 'src':
case 'href': {
if (enableFilterEmptyStringAttributesDOM) {
Expand Down Expand Up @@ -2453,13 +2459,22 @@ function diffHydratedGenericElement(
warnForPropDifference(propKey, serverValue, value, serverDifferences);
continue;
}
case 'data':
if (tag !== 'object') {
extraAttributes.delete(propKey);
const serverValue = (domElement: any).getAttribute('data');
warnForPropDifference(propKey, serverValue, value, serverDifferences);
continue;
}
// fallthrough
case 'src':
case 'href':
if (enableFilterEmptyStringAttributesDOM) {
if (
value === '' &&
// <a href=""> is fine for "reload" links.
!(tag === 'a' && propKey === 'href')
!(tag === 'a' && propKey === 'href') &&
!(tag === 'object' && propKey === 'data')
) {
if (__DEV__) {
if (propKey === 'src') {
Expand Down
69 changes: 69 additions & 0 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,73 @@ function pushStartAnchor(
return children;
}

function pushStartObject(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
): ReactNodeList {
target.push(startChunkForTag('object'));

let children = null;
let innerHTML = null;
for (const propKey in props) {
if (hasOwnProperty.call(props, propKey)) {
const propValue = props[propKey];
if (propValue == null) {
continue;
}
switch (propKey) {
case 'children':
children = propValue;
break;
case 'dangerouslySetInnerHTML':
innerHTML = propValue;
break;
case 'data': {
if (__DEV__) {
checkAttributeStringCoercion(propValue, 'data');
}
const sanitizedValue = sanitizeURL('' + propValue);
if (enableFilterEmptyStringAttributesDOM) {
if (sanitizedValue === '') {
if (__DEV__) {
console.error(
'An empty string ("") was passed to the %s attribute. ' +
'To fix this, either do not render the element at all ' +
'or pass null to %s instead of an empty string.',
propKey,
propKey,
);
}
break;
}
}
target.push(
attributeSeparator,
stringToChunk('data'),
attributeAssign,
stringToChunk(escapeTextForBrowser(sanitizedValue)),
attributeEnd,
);
break;
}
default:
pushAttribute(target, propKey, propValue);
break;
}
}
}

target.push(endOfStartTag);
pushInnerHTML(target, innerHTML, children);
if (typeof children === 'string') {
// Special case children as a string to avoid the unnecessary comment.
// TODO: Remove this special case after the general optimization is in place.
target.push(stringToChunk(encodeHTMLTextNode(children)));
return null;
}
return children;
}

function pushStartSelect(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
Expand Down Expand Up @@ -3569,6 +3636,8 @@ export function pushStartInstance(
return pushStartForm(target, props, resumableState, renderState);
case 'menuitem':
return pushStartMenuItem(target, props);
case 'object':
return pushStartObject(target, props);
case 'title':
return pushTitle(
target,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @jest-environment ./scripts/jest/ReactDOMServerIntegrationEnvironment
*/

'use strict';

const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');

let React;
let ReactDOMClient;
let ReactDOMServer;

function initModules() {
// Reset warning cache.
jest.resetModules();
React = require('react');
ReactDOMClient = require('react-dom/client');
ReactDOMServer = require('react-dom/server');

// Make them available to the helpers.
return {
ReactDOMClient,
ReactDOMServer,
};
}

const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);

describe('ReactDOMServerIntegrationObject', () => {
beforeEach(() => {
resetModules();
});

itRenders('an object with children', async render => {
const e = await render(
<object type="video/mp4" data="/example.webm" width={600} height={400}>
<div>preview</div>
</object>,
);

expect(e.outerHTML).toBe(
'<object type="video/mp4" data="/example.webm" width="600" height="400"><div>preview</div></object>',
);
});

itRenders('an object with empty data', async render => {
const e = await render(<object data="" />, 1);
expect(e.outerHTML).toBe('<object></object>');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,25 @@ describe('ReactDOMServerIntegration - Untrusted URLs', () => {
expect(e.lastChild.href).toBe(EXPECTED_SAFE_URL);
});

itRenders('sanitizes on various tags', async render => {
const aElement = await render(<a href="javascript:notfine" />);
expect(aElement.href).toBe(EXPECTED_SAFE_URL);

const objectElement = await render(<object data="javascript:notfine" />);
expect(objectElement.data).toBe(EXPECTED_SAFE_URL);

const embedElement = await render(<embed src="javascript:notfine" />);
expect(embedElement.src).toBe(EXPECTED_SAFE_URL);
});

itRenders('passes through data on non-object tags', async render => {
const div = await render(<div data="test" />);
expect(div.getAttribute('data')).toBe('test');

const a = await render(<a data="javascript:fine" />);
expect(a.getAttribute('data')).toBe('javascript:fine');
});

itRenders('a javascript protocol with leading spaces', async render => {
const e = await render(
<a href={' \t \u0000\u001F\u0003javascript\n: notfine'}>p0wned</a>,
Expand Down

0 comments on commit 475b518

Please sign in to comment.