From 624c0d2cc5a926f230b0f3df3ffd233c71611d08 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Fri, 3 Apr 2015 16:05:23 -0700 Subject: [PATCH] [RFC] Trusted sources for React elements. This is a proposal for tightening React's XSS security model as referenced by #3473. React Elements are simply JS objects of a particular shape. Because React applications are frequently assembling large trees of these elements and including user data, it's possible for a malicious user to insert data that appears like a React Element and therefore render arbitrary and potentially dangerous markup. Previous versions of React used instanceof, but that limited the ability to use multiple Reacts and tightly coupled JSX to React which we wanted to avoid. This proposal replaces `{_isReactElement: true}` with `{_source: "randomstring"}`. Using React.createElement() automatically adds this _source, but JSX could generate regular object bodies with `{_source: React.getSourceID()}`. In order to use multiple Reacts, a new API `React.trustSource(sourceID)` is added. You can imagine using a different React instance in a webworker and using `React.trustSource(sourceIDFromCallingWorkersReactGetSourceID)`. To preserve back-compat, the default behavior proposed is to dangerously trust all sources, but to warn! If this proposal lands, then I imagine a future version of React will make the default behavior to not trust unknown sources, removing warnings. --- src/browser/ui/React.js | 3 + src/classic/element/ReactElement.js | 102 +++++++++- .../__tests__/ReactElementSource-test.js | 174 ++++++++++++++++++ 3 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 src/classic/element/__tests__/ReactElementSource-test.js diff --git a/src/browser/ui/React.js b/src/browser/ui/React.js index ccead4fee025c..8b44446346e90 100644 --- a/src/browser/ui/React.js +++ b/src/browser/ui/React.js @@ -76,6 +76,9 @@ var React = { unmountComponentAtNode: ReactMount.unmountComponentAtNode, isValidElement: ReactElement.isValidElement, withContext: ReactContext.withContext, + getSourceID: ReactElement.getSourceID, + trustSource: ReactElement.trustSource, + dangerouslyTrustAllSources: ReactElement.dangerouslyTrustAllSources, // Hook for JSX spread, don't use this for anything else. __spread: assign diff --git a/src/classic/element/ReactElement.js b/src/classic/element/ReactElement.js index bfc18a0dabbb9..9d58ec1e650e1 100644 --- a/src/classic/element/ReactElement.js +++ b/src/classic/element/ReactElement.js @@ -60,6 +60,35 @@ function defineWarningProperty(object, key) { */ var useMutationMembrane = false; +/** + * React instances own source ID is a randomly generated string. It does not + * have to be globally unique, just difficult to guess. + */ +var ownSourceID = + (typeof crypto === 'object' && crypto.getRandomValues ? + crypto.getRandomValues(new Uint32Array(1))[0] : + ~(Math.random() * (1 << 31)) + ).toString(36); + +/** + * If trustSource() is called, becomes an mapping of sourceIDs we trust as + * valid React Elements. + */ +var trustedSourceIDs; // ?{ [sourceID: string]: true } + +/** + * If true, dangerously trust all sources. If false, only trust explicit + * sources. + * + * If null, trust all sources but warn if not explicitly trusted. + */ +var trustAllSources = null; // ?Boolean + +/** + * Updated to true if a warning is logged so we don't spam console. + */ +var hasWarnedAboutUntrustedSource; + /** * Warn for mutations. * @@ -139,6 +168,7 @@ var ReactElement = function(type, key, ref, owner, context, props) { // We intentionally don't expose the function on the constructor property. // ReactElement should be indistinguishable from a plain object. ReactElement.prototype = { + _source: ownSourceID, _isReactElement: true }; @@ -146,6 +176,47 @@ if (__DEV__) { defineMutationMembrane(ReactElement.prototype); } + +/** + * Return this React module's source ID. + */ +ReactElement.getSourceID = function() { + return ownSourceID; +}; + +/** + * Allows this React module to trust React elements produced from another React + * module, potentially from a Server or from another Realm (iframe, webworker). + */ +ReactElement.trustSource = function(sourceID) { + if (!trustedSourceIDs) { + trustedSourceIDs = {}; + } + trustedSourceIDs[sourceID] = true; + // Calling trustSource implies using React's trusted source. + // TODO: remove in a future version when security is on by default and + // trustAllSources is no longer a ?Boolean. + if (trustAllSources === null) { + trustAllSources = false; + } +}; + +/** + * Trust React elements regardless of their source, or even if they have an + * unknown source. Using this method may be helpful during a refactor to support + * React's security model, but should be avoided as it could allow XSS attacks + * in certain conditions. + * + * For backwards compatibility, the default behavior is to trust all sources, + * but to warn in __DEV__ when rendering a React component from an unknown + * source. In a future version of React only explicitly trusted sources may + * provide React components. + */ +ReactElement.dangerouslyTrustAllSources = function(acceptPossibleXSSHoles) { + trustAllSources = + acceptPossibleXSSHoles === undefined ? true : acceptPossibleXSSHoles; +}; + ReactElement.createElement = function(type, config, children) { var propName; @@ -298,7 +369,36 @@ ReactElement.isValidElement = function(object) { // same time. This will screw with ownership and stuff. Fix it, please. // TODO: We could possibly warn here. // } - return isElement; + if (!isElement) { + return false; + } + + var sourceID = object && object._source; + + // TODO: remove in a future version when security is on by default and + // trustAllSources is no longer a ?Boolean. + if (__DEV__) { + if (trustAllSources === null && + !hasWarnedAboutUntrustedSource && + !(sourceID && sourceID === ownSourceID)) { + hasWarnedAboutUntrustedSource = true; + warning( + false, + 'React is rendering an element from an unknown or foreign source. ' + + 'This is potentially malicious and a future version of React will ' + + 'not render this element. Call ' + + 'React.dangerouslyTrustAllSources(false) to disable rendering from ' + + 'unknown and foriegn sources.' + ); + } + } + + // Determine if we trust the source of this particular React Element. + return trustAllSources !== false || + sourceID && ( + sourceID === ownSourceID || + trustedSourceIDs && trustedSourceIDs.hasOwnProperty(sourceID) + ); }; module.exports = ReactElement; diff --git a/src/classic/element/__tests__/ReactElementSource-test.js b/src/classic/element/__tests__/ReactElementSource-test.js new file mode 100644 index 0000000000000..b1c6830b8d154 --- /dev/null +++ b/src/classic/element/__tests__/ReactElementSource-test.js @@ -0,0 +1,174 @@ +/** + * Copyright 2013-2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var assign = require('Object.assign'); + +var React; +var ReactTestUtils; +var Component; + +function makeElement(type, props, source) { + return { + type: type, + key: null, + ref: null, + props: props, + _store: {props: props, originalProps: assign({}, props)}, + _source: source, + _isReactElement: true + }; +} + +describe('ReactElementSource', function() { + + beforeEach(function() { + require('mock-modules').dumpCache(); + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); + Component = React.createClass({ + render: function() { + return
{this.props.element}
; + } + }); + }); + + // TODO: this test is removed when warnings are removed in a future version. + it('should not warn when rendering an known element', function () { + spyOn(console, 'error'); + + var element =
Component
; + var component = ReactTestUtils.renderIntoDocument( + + ); + + expect(console.error.calls.length).toBe(0); + }); + + // TODO: this test is removed when warnings are removed in a future version. + it('should warn when rendering an unknown element', function () { + spyOn(console, 'error'); + + var element = makeElement('div', {className: 'unknown'}, undefined); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).toBe('unknown'); + expect(console.error.calls[0].args[0]).toBe( + 'Warning: ' + + 'React is rendering an element from an unknown or foreign source. ' + + 'This is potentially malicious and a future version of React will ' + + 'not render this element. Call ' + + 'React.dangerouslyTrustAllSources(false) to disable rendering from ' + + 'unknown and foriegn sources.' + ); + }); + + // TODO: this test is removed when warnings are removed in a future version. + it('should warn when rendering an foreign element', function () { + spyOn(console, 'error'); + + var element = makeElement('div', {className: 'foreign'}, 'randomstring'); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).toBe('foreign'); + expect(console.error.calls[0].args[0]).toBe( + 'Warning: ' + + 'React is rendering an element from an unknown or foreign source. ' + + 'This is potentially malicious and a future version of React will ' + + 'not render this element. Call ' + + 'React.dangerouslyTrustAllSources(false) to disable rendering from ' + + 'unknown and foriegn sources.' + ); + }); + + it('should render an element created by itself', function() { + spyOn(console, 'error'); + React.dangerouslyTrustAllSources(false); + + var element =
Component
; + expect(element._source).not.toBe(undefined); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).toBe('self'); + expect(console.error.calls.length).toBe(0); + }); + + it('should not render an unknown element', function() { + spyOn(console, 'error'); + React.dangerouslyTrustAllSources(false); + + var element = makeElement('div', {className: 'unknown'}, undefined); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).not.toBe('unknown'); + expect(console.error.calls[0].args[0]).toBe( + 'Warning: Any use of a keyed object should be wrapped in ' + + 'React.addons.createFragment(object) before being passed as a child.' + ); + }); + + it('should render an element created by a trusted source', function() { + spyOn(console, 'error'); + React.trustSource('randomstring'); + + var element = makeElement('div', {className: 'trusted'}, 'randomstring'); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).toBe('trusted'); + expect(console.error.calls.length).toBe(0); + }); + + it('should not render an element created by an foreign source', function() { + spyOn(console, 'error'); + React.trustSource('randomstring'); + + var element = makeElement('div', {className: 'foreign'}, 'differentrandomstring'); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).not.toBe('foreign'); + expect(console.error.calls[0].args[0]).toBe( + 'Warning: Any use of a keyed object should be wrapped in ' + + 'React.addons.createFragment(object) before being passed as a child.' + ); + }); + + it('should render unknown element when dangerously trusting', function() { + spyOn(console, 'error'); + React.dangerouslyTrustAllSources(); + + var element = makeElement('div', {className: 'unknown'}, undefined); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).toBe('unknown'); + expect(console.error.calls.length).toBe(0); + }); + + it('should render foreign element when dangerously trusting', function() { + spyOn(console, 'error'); + React.dangerouslyTrustAllSources(); + + var element = makeElement('div', {className: 'foreign'}, 'randomforeignstring'); + var component = ReactTestUtils.renderIntoDocument( + + ); + expect(React.findDOMNode(component).childNodes[0].className).toBe('foreign'); + expect(console.error.calls.length).toBe(0); + }); + +});