Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sanitize javascript: urls for <object> tags #29808

Merged
merged 1 commit into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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