From 5b19684fc3eddb44a790f31804707de9234147c7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 18 Jul 2018 01:03:07 +0100 Subject: [PATCH] Sanitize unknown attribute names for SSR --- .../src/__tests__/ReactDOMComponent-test.js | 165 ++++++++++++++++-- .../src/server/DOMMarkupOperations.js | 3 +- 2 files changed, 152 insertions(+), 16 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index 441545fc0a154..642c5834c881b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -554,39 +554,174 @@ describe('ReactDOMComponent', () => { expect(stubStyle.color).toEqual('green'); }); - it('should reject attribute key injection attack on markup', () => { + it('should reject attribute key injection attack on markup for regular DOM (SSR)', () => { expect(() => { for (let i = 0; i < 3; i++) { - const container = document.createElement('div'); - const element = React.createElement( + const element1 = React.createElement( + 'div', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ); + const element2 = React.createElement( + 'div', + {'>': 'selected'}, + null, + ); + let result1 = ReactDOMServer.renderToString(element1); + let result2 = ReactDOMServer.renderToString(element2); + expect(result1.toLowerCase()).not.toContain('onclick'); + expect(result2.toLowerCase()).not.toContain('script'); + } + }).toWarnDev([ + 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`', + 'Warning: Invalid attribute name: `>`', + ]); + }); + + it('should reject attribute key injection attack on markup for custom elements (SSR)', () => { + expect(() => { + for (let i = 0; i < 3; i++) { + const element1 = React.createElement( 'x-foo-component', {'blah" onclick="beevil" noise="hi': 'selected'}, null, ); - ReactDOM.render(element, container); + const element2 = React.createElement( + 'x-foo-component', + {'>': 'selected'}, + null, + ); + let result1 = ReactDOMServer.renderToString(element1); + let result2 = ReactDOMServer.renderToString(element2); + expect(result1.toLowerCase()).not.toContain('onclick'); + expect(result2.toLowerCase()).not.toContain('script'); } - }).toWarnDev( + }).toWarnDev([ 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`', - ); + 'Warning: Invalid attribute name: `>`', + ]); }); - it('should reject attribute key injection attack on update', () => { + it('should reject attribute key injection attack on mount for regular DOM', () => { expect(() => { for (let i = 0; i < 3; i++) { const container = document.createElement('div'); - const beforeUpdate = React.createElement('x-foo-component', {}, null); + ReactDOM.render( + React.createElement( + 'div', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + container, + ); + expect(container.firstChild.attributes.length).toBe(0); + ReactDOM.unmountComponentAtNode(container); + ReactDOM.render( + React.createElement( + 'div', + {'>': 'selected'}, + null, + ), + container, + ); + expect(container.firstChild.attributes.length).toBe(0); + } + }).toWarnDev([ + 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`', + 'Warning: Invalid attribute name: `>`', + ]); + }); + + it('should reject attribute key injection attack on mount for custom elements', () => { + expect(() => { + for (let i = 0; i < 3; i++) { + const container = document.createElement('div'); + ReactDOM.render( + React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + container, + ); + expect(container.firstChild.attributes.length).toBe(0); + ReactDOM.unmountComponentAtNode(container); + ReactDOM.render( + React.createElement( + 'x-foo-component', + {'>': 'selected'}, + null, + ), + container, + ); + expect(container.firstChild.attributes.length).toBe(0); + } + }).toWarnDev([ + 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`', + 'Warning: Invalid attribute name: `>`', + ]); + }); + + it('should reject attribute key injection attack on update for regular DOM', () => { + expect(() => { + for (let i = 0; i < 3; i++) { + const container = document.createElement('div'); + const beforeUpdate = React.createElement('div', {}, null); ReactDOM.render(beforeUpdate, container); + ReactDOM.render( + React.createElement( + 'div', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + container, + ); + expect(container.firstChild.attributes.length).toBe(0); + ReactDOM.render( + React.createElement( + 'div', + {'>': 'selected'}, + null, + ), + container, + ); + expect(container.firstChild.attributes.length).toBe(0); + } + }).toWarnDev([ + 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`', + 'Warning: Invalid attribute name: `>`', + ]); + }); - const afterUpdate = React.createElement( - 'x-foo-component', - {'blah" onclick="beevil" noise="hi': 'selected'}, - null, + it('should reject attribute key injection attack on update for custom elements', () => { + expect(() => { + for (let i = 0; i < 3; i++) { + const container = document.createElement('div'); + const beforeUpdate = React.createElement('x-foo-component', {}, null); + ReactDOM.render(beforeUpdate, container); + ReactDOM.render( + React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + container, + ); + expect(container.firstChild.attributes.length).toBe(0); + ReactDOM.render( + React.createElement( + 'x-foo-component', + {'>': 'selected'}, + null, + ), + container, ); - ReactDOM.render(afterUpdate, container); + expect(container.firstChild.attributes.length).toBe(0); } - }).toWarnDev( + }).toWarnDev([ 'Warning: Invalid attribute name: `blah" onclick="beevil" noise="hi`', - ); + 'Warning: Invalid attribute name: `>`', + ]); }); it('should update arbitrary attributes for tags containing dashes', () => { diff --git a/packages/react-dom/src/server/DOMMarkupOperations.js b/packages/react-dom/src/server/DOMMarkupOperations.js index 37eb5de9d4dd1..67d89a1c8ad36 100644 --- a/packages/react-dom/src/server/DOMMarkupOperations.js +++ b/packages/react-dom/src/server/DOMMarkupOperations.js @@ -60,9 +60,10 @@ export function createMarkupForProperty(name: string, value: mixed): string { } else { return attributeName + '=' + quoteAttributeValueForBrowser(value); } - } else { + } else if (isAttributeNameSafe(name)) { return name + '=' + quoteAttributeValueForBrowser(value); } + return ''; } /**