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

Drop runtime validation and lower case coercion of tag names #8563

Merged
merged 3 commits into from
Dec 13, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Only do runtime validation of tag names when createElement is not used
We introduced runtime validation of tag names because we used to generate
HTML that was supposed to be inserted into a HTML string which could've
been an XSS attack.

However, these days we use document.createElement in most cases. That
already does its internal validation in the browser which throws. We're now
double validating it. Stack still has a path where innerHTML is used and
we still need it there. However in Fiber we can remove it completely.
  • Loading branch information
sebmarkbage committed Dec 13, 2016
commit db489a4adeea8edd2aab1d47b3ce55dd28198e4b
2 changes: 2 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,8 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should properly escape text content and attributes values
* unmounts children before unsetting DOM node info
* should warn about the `onScroll` issue when unsupported (IE8)
* should throw when an invalid tag name is used server-side
* should throw when an attack vector is used server-side
* should throw when an invalid tag name is used
* should throw when an attack vector is used
* should warn about props that are no longer supported
Expand Down
17 changes: 0 additions & 17 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,21 +279,6 @@ var voidElementTags = {
...omittedCloseTags,
};

// We accept any tag to be rendered but since this gets injected into arbitrary
// HTML, we want to make sure that it's a safe tag.
// http://www.w3.org/TR/REC-xml/#NT-Name

var VALID_TAG_REGEX = /^[a-zA-Z][a-zA-Z:_\.\-\d]*$/; // Simplified subset
var validatedTagCache = {};
var hasOwnProperty = {}.hasOwnProperty;

function validateDangerousTag(tag) {
if (!hasOwnProperty.call(validatedTagCache, tag)) {
invariant(VALID_TAG_REGEX.test(tag), 'Invalid tag: %s', tag);
validatedTagCache[tag] = true;
}
}

function isCustomComponent(tagName, props) {
return tagName.indexOf('-') >= 0 || props.is != null;
}
Expand Down Expand Up @@ -488,8 +473,6 @@ var ReactDOMFiberComponent = {
rootContainerElement : Element,
parentNamespace : string | null
) : Element {
validateDangerousTag(type);

// We create tags in the namespace of their parent container, except HTML
// tags get no namespace.
var ownerDocument = rootContainerElement.ownerDocument;
Expand Down
22 changes: 18 additions & 4 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1207,23 +1207,37 @@ describe('ReactDOMComponent', () => {
});

describe('tag sanitization', () => {
it('should throw when an invalid tag name is used', () => {
it('should throw when an invalid tag name is used server-side', () => {
var hackzor = React.createElement('script tag');
expect(
() => ReactTestUtils.renderIntoDocument(hackzor)
() => ReactDOMServer.renderToString(hackzor)
).toThrowError(
'Invalid tag: script tag'
);
});

it('should throw when an attack vector is used', () => {
it('should throw when an attack vector is used server-side', () => {
var hackzor = React.createElement('div><img /><div');
expect(
() => ReactTestUtils.renderIntoDocument(hackzor)
() => ReactDOMServer.renderToString(hackzor)
).toThrowError(
'Invalid tag: div><img /><div'
);
});

it('should throw when an invalid tag name is used', () => {
var hackzor = React.createElement('script tag');
expect(
() => ReactTestUtils.renderIntoDocument(hackzor)
).toThrow();
});

it('should throw when an attack vector is used', () => {
var hackzor = React.createElement('div><img /><div');
expect(
() => ReactTestUtils.renderIntoDocument(hackzor)
).toThrow();
});
});

describe('nesting validation', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/dom/stack/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ var globalIdCounter = 1;
*/
function ReactDOMComponent(element) {
var tag = element.type;
validateDangerousTag(tag);
this._currentElement = element;
this._tag = tag.toLowerCase();
this._namespaceURI = null;
Expand Down Expand Up @@ -605,6 +604,7 @@ ReactDOMComponent.Mixin = {
this._createInitialChildren(transaction, props, context, lazyTree);
mountImage = lazyTree;
} else {
validateDangerousTag(this._tag);
var tagOpen = this._createOpenTagMarkupAndPutListeners(transaction, props);
var tagContent = this._createContentMarkup(transaction, props, context);
if (!tagContent && omittedCloseTags[this._tag]) {
Expand Down