Warn for javascript: URLs in DOM sinks (#15047)

* Prevent javascript protocol URLs

* Just warn when disableJavaScriptURLs is false

This avoids a breaking change.

* Allow framesets

* Allow <html> to be used in integration tests

Full document renders requires server rendering so the client path
just uses the hydration path in this case to simplify writing these tests.

* Detect leading and intermediate characters and test mixed case

These are considered valid javascript urls by browser so they must be
included in the filter.

This is an exact match according to the spec but maybe we should include
a super set to be safer?

* Test updates to ensure we have coverage there too

* Fix toString invocation and Flow types

Right now we invoke toString twice when we hydrate (three times
with the flag off). Ideally we should only do it once even in this case
but the code structure doesn't really allow for that right now.

* s/itRejects/itRejectsRendering

* Dedupe warning and add the unsafe URL to the warning message

* Add test that fails if g is added to the sanitizer

This only affects the prod version since the warning is deduped anyway.

* Fix prod test
This commit is contained in:
Sebastian Markbåge 2019-03-11 16:39:49 -07:00 committed by GitHub
parent 5d0c3c6c7d
commit 103378b1ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 451 additions and 22 deletions

View File

@ -0,0 +1,271 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/
/* eslint-disable no-script-url */
'use strict';
const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');
let React;
let ReactDOM;
let ReactDOMServer;
function runTests(itRenders, itRejectsRendering, expectToReject) {
itRenders('a http link with the word javascript in it', async render => {
const e = await render(
<a href="http://javascript:0/thisisfine">Click me</a>,
);
expect(e.tagName).toBe('A');
expect(e.href).toBe('http://javascript:0/thisisfine');
});
itRejectsRendering('a javascript protocol href', async render => {
// Only the first one warns. The second warning is deduped.
const e = await render(
<div>
<a href="javascript:notfine">p0wned</a>
<a href="javascript:notfineagain">p0wned again</a>
</div>,
1,
);
expect(e.firstChild.href).toBe('javascript:notfine');
expect(e.lastChild.href).toBe('javascript:notfineagain');
});
itRejectsRendering(
'a javascript protocol with leading spaces',
async render => {
const e = await render(
<a href={' \t \u0000\u001F\u0003javascript\n: notfine'}>p0wned</a>,
1,
);
// We use an approximate comparison here because JSDOM might not parse
// \u0000 in HTML properly.
expect(e.href).toContain('notfine');
},
);
itRejectsRendering(
'a javascript protocol with intermediate new lines and mixed casing',
async render => {
const e = await render(
<a href={'\t\r\n Jav\rasCr\r\niP\t\n\rt\n:notfine'}>p0wned</a>,
1,
);
expect(e.href).toBe('javascript:notfine');
},
);
itRejectsRendering('a javascript protocol area href', async render => {
const e = await render(
<map>
<area href="javascript:notfine" />
</map>,
1,
);
expect(e.firstChild.href).toBe('javascript:notfine');
});
itRejectsRendering('a javascript protocol form action', async render => {
const e = await render(<form action="javascript:notfine">p0wned</form>, 1);
expect(e.action).toBe('javascript:notfine');
});
itRejectsRendering(
'a javascript protocol button formAction',
async render => {
const e = await render(<input formAction="javascript:notfine" />, 1);
expect(e.getAttribute('formAction')).toBe('javascript:notfine');
},
);
itRejectsRendering('a javascript protocol input formAction', async render => {
const e = await render(
<button formAction="javascript:notfine">p0wned</button>,
1,
);
expect(e.getAttribute('formAction')).toBe('javascript:notfine');
});
itRejectsRendering('a javascript protocol iframe src', async render => {
const e = await render(<iframe src="javascript:notfine" />, 1);
expect(e.src).toBe('javascript:notfine');
});
itRejectsRendering('a javascript protocol frame src', async render => {
const e = await render(
<html>
<head />
<frameset>
<frame src="javascript:notfine" />
</frameset>
</html>,
1,
);
expect(e.lastChild.firstChild.src).toBe('javascript:notfine');
});
itRejectsRendering('a javascript protocol in an SVG link', async render => {
const e = await render(
<svg>
<a href="javascript:notfine" />
</svg>,
1,
);
expect(e.firstChild.getAttribute('href')).toBe('javascript:notfine');
});
itRejectsRendering(
'a javascript protocol in an SVG link with a namespace',
async render => {
const e = await render(
<svg>
<a xlinkHref="javascript:notfine" />
</svg>,
1,
);
expect(
e.firstChild.getAttributeNS('http://www.w3.org/1999/xlink', 'href'),
).toBe('javascript:notfine');
},
);
it('rejects a javascript protocol href if it is added during an update', () => {
let container = document.createElement('div');
ReactDOM.render(<a href="thisisfine">click me</a>, container);
expectToReject(() => {
ReactDOM.render(<a href="javascript:notfine">click me</a>, container);
});
});
}
describe('ReactDOMServerIntegration - Untrusted URLs', () => {
function initModules() {
jest.resetModuleRegistry();
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
// Make them available to the helpers.
return {
ReactDOM,
ReactDOMServer,
};
}
const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);
beforeEach(() => {
resetModules();
});
runTests(itRenders, itRenders, fn =>
expect(fn).toWarnDev(
'Warning: A future version of React will block javascript: URLs as a security precaution. ' +
'Use event handlers instead if you can. If you need to generate unsafe HTML try using ' +
'dangerouslySetInnerHTML instead. React was passed "javascript:notfine".\n' +
' in a (at **)',
),
);
});
describe('ReactDOMServerIntegration - Untrusted URLs - disableJavaScriptURLs', () => {
function initModules() {
jest.resetModuleRegistry();
const ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.disableJavaScriptURLs = true;
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
// Make them available to the helpers.
return {
ReactDOM,
ReactDOMServer,
};
}
const {
resetModules,
itRenders,
itThrowsWhenRendering,
clientRenderOnBadMarkup,
clientRenderOnServerString,
} = ReactDOMServerIntegrationUtils(initModules);
const expectToReject = fn => {
let msg;
try {
fn();
} catch (x) {
msg = x.message;
}
expect(msg).toContain(
'React has blocked a javascript: URL as a security precaution.',
);
};
beforeEach(() => {
resetModules();
});
runTests(
itRenders,
(message, test) =>
itThrowsWhenRendering(message, test, 'blocked a javascript: URL'),
expectToReject,
);
itRenders('only the first invocation of toString', async render => {
let expectedToStringCalls = 1;
if (render === clientRenderOnBadMarkup) {
// It gets called once on the server and once on the client
// which happens to share the same object in our test runner.
expectedToStringCalls = 2;
}
if (render === clientRenderOnServerString && __DEV__) {
// The hydration validation calls it one extra time.
// TODO: It would be good if we only called toString once for
// consistency but the code structure makes that hard right now.
expectedToStringCalls = 2;
}
let toStringCalls = 0;
let firstIsSafe = {
toString() {
// This tries to avoid the validation by pretending to be safe
// the first times it is called and then becomes dangerous.
toStringCalls++;
if (toStringCalls <= expectedToStringCalls) {
return 'https://fb.me/';
}
return 'javascript:notfine';
},
};
const e = await render(<a href={firstIsSafe} />);
expect(toStringCalls).toBe(expectedToStringCalls);
expect(e.href).toBe('https://fb.me/');
});
it('rejects a javascript protocol href if it is added during an update twice', () => {
let container = document.createElement('div');
ReactDOM.render(<a href="thisisfine">click me</a>, container);
expectToReject(() => {
ReactDOM.render(<a href="javascript:notfine">click me</a>, container);
});
// The second update ensures that a global flag hasn't been added to the regex
// which would fail to match the second time it is called.
expectToReject(() => {
ReactDOM.render(<a href="javascript:notfine">click me</a>, container);
});
});
});

View File

@ -19,6 +19,28 @@ module.exports = function(initModules) {
({ReactDOM, ReactDOMServer} = initModules());
}
function shouldUseDocument(reactElement) {
// Used for whole document tests.
return reactElement && reactElement.type === 'html';
}
function getContainerFromMarkup(reactElement, markup) {
if (shouldUseDocument(reactElement)) {
const doc = document.implementation.createHTMLDocument('');
doc.open();
doc.write(
markup ||
'<!doctype html><html><meta charset=utf-8><title>test doc</title>',
);
doc.close();
return doc;
} else {
const container = document.createElement('div');
container.innerHTML = markup;
return container;
}
}
// Helper functions for rendering tests
// ====================================
@ -97,9 +119,7 @@ module.exports = function(initModules) {
// Does not render on client or perform client-side revival.
async function serverRender(reactElement, errorCount = 0) {
const markup = await renderIntoString(reactElement, errorCount);
const domElement = document.createElement('div');
domElement.innerHTML = markup;
return domElement.firstChild;
return getContainerFromMarkup(reactElement, markup).firstChild;
}
// this just drains a readable piped into it to a string, which can be accessed
@ -133,27 +153,28 @@ module.exports = function(initModules) {
// Does not render on client or perform client-side revival.
async function streamRender(reactElement, errorCount = 0) {
const markup = await renderIntoStream(reactElement, errorCount);
const domElement = document.createElement('div');
domElement.innerHTML = markup;
return domElement.firstChild;
return getContainerFromMarkup(reactElement, markup).firstChild;
}
const clientCleanRender = (element, errorCount = 0) => {
const div = document.createElement('div');
return renderIntoDom(element, div, false, errorCount);
if (shouldUseDocument(element)) {
// Documents can't be rendered from scratch.
return clientRenderOnServerString(element, errorCount);
}
const container = document.createElement('div');
return renderIntoDom(element, container, false, errorCount);
};
const clientRenderOnServerString = async (element, errorCount = 0) => {
const markup = await renderIntoString(element, errorCount);
resetModules();
const domElement = document.createElement('div');
domElement.innerHTML = markup;
let serverNode = domElement.firstChild;
let container = getContainerFromMarkup(element, markup);
let serverNode = container.firstChild;
const firstClientNode = await renderIntoDom(
element,
domElement,
container,
true,
errorCount,
);
@ -178,19 +199,35 @@ module.exports = function(initModules) {
const clientRenderOnBadMarkup = async (element, errorCount = 0) => {
// First we render the top of bad mark up.
const domElement = document.createElement('div');
domElement.innerHTML =
'<div id="badIdWhichWillCauseMismatch" data-reactroot="" data-reactid="1"></div>';
await renderIntoDom(element, domElement, true, errorCount + 1);
let container = getContainerFromMarkup(
element,
shouldUseDocument(element)
? '<html><body><div id="badIdWhichWillCauseMismatch" /></body></html>'
: '<div id="badIdWhichWillCauseMismatch" data-reactroot="" data-reactid="1"></div>',
);
await renderIntoDom(element, container, true, errorCount + 1);
// This gives us the resulting text content.
const hydratedTextContent = domElement.textContent;
const hydratedTextContent =
container.lastChild && container.lastChild.textContent;
// Next we render the element into a clean DOM node client side.
const cleanDomElement = document.createElement('div');
await asyncReactDOMRender(element, cleanDomElement, true);
let cleanContainer;
if (shouldUseDocument(element)) {
// We can't render into a document during a clean render,
// so instead, we'll render the children into the document element.
cleanContainer = getContainerFromMarkup(element, '<html></html>')
.documentElement;
element = element.props.children;
} else {
cleanContainer = document.createElement('div');
}
await asyncReactDOMRender(element, cleanContainer, true);
// This gives us the expected text content.
const cleanTextContent = cleanDomElement.textContent;
const cleanTextContent =
cleanContainer.lastChild && cleanContainer.lastChild.textContent;
// The only guarantee is that text content has been patched up if needed.
expect(hydratedTextContent).toBe(cleanTextContent);
@ -320,6 +357,7 @@ module.exports = function(initModules) {
asyncReactDOMRender,
serverRender,
clientCleanRender,
clientRenderOnBadMarkup,
clientRenderOnServerString,
renderIntoDom,
streamRender,

View File

@ -15,6 +15,8 @@ import {
BOOLEAN,
OVERLOADED_BOOLEAN,
} from '../shared/DOMProperty';
import sanitizeURL from '../shared/sanitizeURL';
import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags';
import type {PropertyInfo} from '../shared/DOMProperty';
@ -34,6 +36,13 @@ export function getValueForProperty(
const {propertyName} = propertyInfo;
return (node: any)[propertyName];
} else {
if (!disableJavaScriptURLs && propertyInfo.sanitizeURL) {
// If we haven't fully disabled javascript: URLs, and if
// the hydration is successful of a javascript: URL, we
// still want to warn on the client.
sanitizeURL('' + (expected: any));
}
const attributeName = propertyInfo.attributeName;
let stringValue = null;
@ -164,6 +173,9 @@ export function setValueForProperty(
// `setAttribute` with objects becomes only `[object]` in IE8/9,
// ('' + value) makes it output the correct toString()-value.
attributeValue = '' + (value: any);
if (propertyInfo.sanitizeURL) {
sanitizeURL(attributeValue);
}
}
if (attributeNamespace) {
node.setAttributeNS(attributeNamespace, attributeName, attributeValue);

View File

@ -282,7 +282,9 @@ if (__DEV__) {
);
// https://html.spec.whatwg.org/multipage/semantics.html#the-html-element
case 'html':
return tag === 'head' || tag === 'body';
return tag === 'head' || tag === 'body' || tag === 'frameset';
case 'frameset':
return tag === 'frame';
case '#document':
return tag === 'html';
}
@ -314,6 +316,7 @@ if (__DEV__) {
case 'caption':
case 'col':
case 'colgroup':
case 'frameset':
case 'frame':
case 'head':
case 'html':

View File

@ -17,6 +17,7 @@ import {
shouldIgnoreAttribute,
shouldRemoveAttribute,
} from '../shared/DOMProperty';
import sanitizeURL from '../shared/sanitizeURL';
import quoteAttributeValueForBrowser from './quoteAttributeValueForBrowser';
/**
@ -58,6 +59,10 @@ export function createMarkupForProperty(name: string, value: mixed): string {
if (type === BOOLEAN || (type === OVERLOADED_BOOLEAN && value === true)) {
return attributeName + '=""';
} else {
if (propertyInfo.sanitizeURL) {
value = '' + (value: any);
sanitizeURL(value);
}
return attributeName + '=' + quoteAttributeValueForBrowser(value);
}
} else if (isAttributeNameSafe(name)) {

View File

@ -51,6 +51,7 @@ export type PropertyInfo = {|
+mustUseProperty: boolean,
+propertyName: string,
+type: PropertyType,
+sanitizeURL: boolean,
|};
/* eslint-disable max-len */
@ -186,6 +187,7 @@ function PropertyInfoRecord(
mustUseProperty: boolean,
attributeName: string,
attributeNamespace: string | null,
sanitizeURL: boolean,
) {
this.acceptsBooleans =
type === BOOLEANISH_STRING ||
@ -196,6 +198,7 @@ function PropertyInfoRecord(
this.mustUseProperty = mustUseProperty;
this.propertyName = name;
this.type = type;
this.sanitizeURL = sanitizeURL;
}
// When adding attributes to this list, be sure to also add them to
@ -223,6 +226,7 @@ const properties = {};
false, // mustUseProperty
name, // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -240,6 +244,7 @@ const properties = {};
false, // mustUseProperty
attributeName, // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -253,6 +258,7 @@ const properties = {};
false, // mustUseProperty
name.toLowerCase(), // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -272,6 +278,7 @@ const properties = {};
false, // mustUseProperty
name, // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -308,6 +315,7 @@ const properties = {};
false, // mustUseProperty
name.toLowerCase(), // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -331,6 +339,7 @@ const properties = {};
true, // mustUseProperty
name, // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -350,6 +359,7 @@ const properties = {};
false, // mustUseProperty
name, // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -370,6 +380,7 @@ const properties = {};
false, // mustUseProperty
name, // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -381,6 +392,7 @@ const properties = {};
false, // mustUseProperty
name.toLowerCase(), // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -478,6 +490,7 @@ const capitalize = token => token[1].toUpperCase();
false, // mustUseProperty
attributeName,
null, // attributeNamespace
false, // sanitizeURL
);
});
@ -485,7 +498,6 @@ const capitalize = token => token[1].toUpperCase();
[
'xlink:actuate',
'xlink:arcrole',
'xlink:href',
'xlink:role',
'xlink:show',
'xlink:title',
@ -502,6 +514,7 @@ const capitalize = token => token[1].toUpperCase();
false, // mustUseProperty
attributeName,
'http://www.w3.org/1999/xlink',
false, // sanitizeURL
);
});
@ -522,6 +535,7 @@ const capitalize = token => token[1].toUpperCase();
false, // mustUseProperty
attributeName,
'http://www.w3.org/XML/1998/namespace',
false, // sanitizeURL
);
});
@ -535,5 +549,29 @@ const capitalize = token => token[1].toUpperCase();
false, // mustUseProperty
attributeName.toLowerCase(), // attributeName
null, // attributeNamespace
false, // sanitizeURL
);
});
// These attributes accept URLs. These must not allow javascript: URLS.
// These will also need to accept Trusted Types object in the future.
const xlinkHref = 'xlinkHref';
properties[xlinkHref] = new PropertyInfoRecord(
'xlinkHref',
STRING,
false, // mustUseProperty
'xlink:href',
'http://www.w3.org/1999/xlink',
true, // sanitizeURL
);
['src', 'href', 'action', 'formAction'].forEach(attributeName => {
properties[attributeName] = new PropertyInfoRecord(
attributeName,
STRING,
false, // mustUseProperty
attributeName.toLowerCase(), // attributeName
null, // attributeNamespace
true, // sanitizeURL
);
});

View File

@ -0,0 +1,53 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/
import invariant from 'shared/invariant';
import warning from 'shared/warning';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags';
let ReactDebugCurrentFrame = ((null: any): {getStackAddendum(): string});
if (__DEV__) {
ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
}
// A javascript: URL can contain leading C0 control or \u0020 SPACE,
// and any newline or tab are filtered out as if they're not part of the URL.
// https://url.spec.whatwg.org/#url-parsing
// Tab or newline are defined as \r\n\t:
// https://infra.spec.whatwg.org/#ascii-tab-or-newline
// A C0 control is a code point in the range \u0000 NULL to \u001F
// INFORMATION SEPARATOR ONE, inclusive:
// https://infra.spec.whatwg.org/#c0-control-or-space
/* eslint-disable max-len */
const isJavaScriptProtocol = /^[\u0000-\u001F ]*j[\r\n\t]*a[\r\n\t]*v[\r\n\t]*a[\r\n\t]*s[\r\n\t]*c[\r\n\t]*r[\r\n\t]*i[\r\n\t]*p[\r\n\t]*t[\r\n\t]*\:/i;
let didWarn = false;
function sanitizeURL(url: string) {
if (disableJavaScriptURLs) {
invariant(
!isJavaScriptProtocol.test(url),
'React has blocked a javascript: URL as a security precaution.%s',
__DEV__ ? ReactDebugCurrentFrame.getStackAddendum() : '',
);
} else if (__DEV__ && !didWarn && isJavaScriptProtocol.test(url)) {
didWarn = true;
warning(
false,
'A future version of React will block javascript: URLs as a security precaution. ' +
'Use event handlers instead if you can. If you need to generate unsafe HTML try ' +
'using dangerouslySetInnerHTML instead. React was passed %s.',
JSON.stringify(url),
);
}
}
export default sanitizeURL;

View File

@ -42,6 +42,9 @@ export function addUserTimingListener() {
throw new Error('Not implemented.');
}
// Disable javascript: URL strings in href for XSS protection.
export const disableJavaScriptURLs = false;
// React Fire: prevent the value and checked attributes from syncing
// with their related DOM properties
export const disableInputAttributeSyncing = false;

View File

@ -24,6 +24,7 @@ export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;
export const enableSchedulerDebugging = false;
export const debugRenderPhaseSideEffectsForStrictMode = true;
export const disableJavaScriptURLs = false;
export const disableInputAttributeSyncing = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const warnAboutDeprecatedLifecycles = true;

View File

@ -20,6 +20,7 @@ export const warnAboutDeprecatedLifecycles = false;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
export const enableSuspenseServerRenderer = false;
export const disableJavaScriptURLs = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;

View File

@ -20,6 +20,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
export const enableSuspenseServerRenderer = false;
export const disableJavaScriptURLs = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;

View File

@ -20,6 +20,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = false;
export const enableSchedulerTracing = false;
export const enableSuspenseServerRenderer = false;
export const disableJavaScriptURLs = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;

View File

@ -23,6 +23,7 @@ export const enableSuspenseServerRenderer = false;
export const enableStableConcurrentModeAPIs = false;
export const enableSchedulerDebugging = false;
export const warnAboutDeprecatedSetNativeProps = false;
export const disableJavaScriptURLs = false;
// Only used in www builds.
export function addUserTimingListener() {

View File

@ -16,6 +16,7 @@ export const {
debugRenderPhaseSideEffectsForStrictMode,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
warnAboutDeprecatedLifecycles,
disableJavaScriptURLs,
disableInputAttributeSyncing,
warnAboutShorthandPropertyCollision,
warnAboutDeprecatedSetNativeProps,