From d2b1e03f8cb1d918db5bfc92e3b3dcc7c7aa6573 Mon Sep 17 00:00:00 2001 From: Vladimir Airikh Date: Fri, 4 May 2018 16:19:24 +0300 Subject: [PATCH] fix `Remove `integrity` attributes from resources` (close #235) (#1592) * fix `Remove `integrity` attributes from resources` * add semicolon * fix Edge and IE11 cases * fix dom-processor-test * requested changes * fix comments * requested changes * requested changes * requested changes * set edge version for client tests * requested changes: rename hasCorrectIntegrityAttr to hasIntegrityAttr in attribute-test --- Gulpfile.js | 3 +- src/client/sandbox/native-methods.js | 14 ++++++ src/client/sandbox/node/element.js | 23 +++++++++ src/client/sandbox/node/window.js | 6 +++ src/processing/dom/index.js | 20 ++++++-- .../fixtures/sandbox/node/attributes-test.js | 48 +++++++++++++++++++ .../sandbox/node/dom-processor-test.js | 23 +++++---- test/server/data/page/expected.html | 4 +- 8 files changed, 127 insertions(+), 14 deletions(-) diff --git a/Gulpfile.js b/Gulpfile.js index 763ca3470b..272a618c2e 100644 --- a/Gulpfile.js +++ b/Gulpfile.js @@ -37,7 +37,8 @@ const CLIENT_TESTS_SETTINGS = { const CLIENT_TESTS_BROWSERS = [ { platform: 'Windows 10', - browserName: 'MicrosoftEdge' + browserName: 'MicrosoftEdge', + version: '16.16299' }, { platform: 'Windows 10', diff --git a/src/client/sandbox/native-methods.js b/src/client/sandbox/native-methods.js index dc77df3608..57a78336d8 100644 --- a/src/client/sandbox/native-methods.js +++ b/src/client/sandbox/native-methods.js @@ -233,6 +233,7 @@ class NativeMethods { const textAreaValueDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLTextAreaElement.prototype, 'value'); const imageSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLImageElement.prototype, 'src'); const scriptSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLScriptElement.prototype, 'src'); + const scriptIntegrityDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLScriptElement.prototype, 'integrity'); const embedSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLEmbedElement.prototype, 'src'); const sourceSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLSourceElement.prototype, 'src'); const mediaSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLMediaElement.prototype, 'src'); @@ -241,6 +242,7 @@ class NativeMethods { const iframeSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLIFrameElement.prototype, 'src'); const anchorHrefDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLAnchorElement.prototype, 'href'); const linkHrefDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLLinkElement.prototype, 'href'); + const linkIntegrityDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLLinkElement.prototype, 'integrity'); const areaHrefDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLAreaElement.prototype, 'href'); const baseHrefDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLBaseElement.prototype, 'href'); const anchorHostDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLAnchorElement.prototype, 'host'); @@ -310,6 +312,12 @@ class NativeMethods { this.elementInnerHTMLSetter = elementInnerHTMLDescriptor.set; this.elementOuterHTMLSetter = elementOuterHTMLDescriptor.set; + // NOTE: Some browsers (for example, Edge, Internet Explorer 11, Safari) don't support the 'integrity' property. + if (scriptIntegrityDescriptor && linkIntegrityDescriptor) { + this.scriptIntegritySetter = scriptIntegrityDescriptor.set; + this.linkIntegritySetter = linkIntegrityDescriptor.set; + } + // NOTE: Event properties is located in window prototype only in IE11 this.isEventPropsLocatedInProto = win.Window.prototype.hasOwnProperty('onerror'); @@ -389,6 +397,12 @@ class NativeMethods { const anchorOriginDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLAnchorElement.prototype, 'origin'); + // NOTE: Some browsers (for example, Edge, Internet Explorer 11, Safari) don't support the 'integrity' property. + if (scriptIntegrityDescriptor && linkIntegrityDescriptor) { + this.scriptIntegrityGetter = scriptIntegrityDescriptor.get; + this.linkIntegrityGetter = linkIntegrityDescriptor.get; + } + // NOTE: IE and Edge don't support origin property if (anchorOriginDescriptor) this.anchorOriginGetter = anchorOriginDescriptor.get; diff --git a/src/client/sandbox/node/element.js b/src/client/sandbox/node/element.js index 59f2de9cf5..29ce437c21 100644 --- a/src/client/sandbox/node/element.js +++ b/src/client/sandbox/node/element.js @@ -85,6 +85,7 @@ export default class ElementSandbox extends SandboxBase { const loweredAttr = attr.toLowerCase(); const ns = isNs ? args[0] : null; const getAttrMeth = isNs ? nativeMethods.getAttributeNS : nativeMethods.getAttribute; + const tagName = domUtils.getTagName(el); // OPTIMIZATION: The hasAttribute method is very slow. if (domProcessor.isUrlAttr(el, loweredAttr, ns) || @@ -98,6 +99,15 @@ export default class ElementSandbox extends SandboxBase { else if (el.hasAttribute(storedAttrName)) args[isNs ? 1 : 0] = storedAttrName; } + // NOTE: We simply remove the 'integrity' attribute because its value will not be relevant after the script + // content changes (http://www.w3.org/TR/SRI/). If this causes problems in the future, we will need to generate + // the correct SHA for the changed script. (GH-235) + else if (!isNs && loweredAttr === 'integrity' && DomProcessor.isTagWithIntegrityAttr(tagName)) { + const storedIntegrityAttr = DomProcessor.getStoredAttrName(attr); + + if (nativeMethods.hasAttribute.call(el, storedIntegrityAttr)) + args[0] = storedIntegrityAttr; + } return getAttrMeth.apply(el, args); } @@ -222,6 +232,11 @@ export default class ElementSandbox extends SandboxBase { setAttrMeth.apply(el, isNs ? [ns, storedStyleAttr, value] : [storedStyleAttr, value]); args[valueIndex] = styleProcessor.process(value, urlUtils.getProxyUrl); } + else if (!isNs && loweredAttr === 'integrity' && DomProcessor.isTagWithIntegrityAttr(tagName)) { + const storedIntegrityAttr = DomProcessor.getStoredAttrName(attr); + + return setAttrMeth.apply(el, [storedIntegrityAttr, value]); + } const result = setAttrMeth.apply(el, args); @@ -241,6 +256,14 @@ export default class ElementSandbox extends SandboxBase { DomProcessor.isAddedAutocompleteAttr(args[attributeNameArgIndex], storedAutocompleteAttrValue)) return false; + // NOTE: We simply remove the 'integrity' attribute because its value will not be relevant after the script + // content changes (http://www.w3.org/TR/SRI/). If this causes problems in the future, we will need to generate + // the correct SHA for the changed script. + // _hasAttributeCore returns true for 'integrity' attribute if the stored attribute is exists. (GH-235) + if (!isNs && args[attributeNameArgIndex] === 'integrity' && + DomProcessor.isTagWithIntegrityAttr(domUtils.getTagName(el))) + args[attributeNameArgIndex] = DomProcessor.getStoredAttrName('integrity'); + return hasAttrMeth.apply(el, args); } diff --git a/src/client/sandbox/node/window.js b/src/client/sandbox/node/window.js index 9315a54ebd..a90c77ec43 100644 --- a/src/client/sandbox/node/window.js +++ b/src/client/sandbox/node/window.js @@ -767,6 +767,12 @@ export default class WindowSandbox extends SandboxBase { this._overrideAttrDescriptors('autocomplete', [window.HTMLInputElement]); + // NOTE: Some browsers (for example, Edge, Internet Explorer 11, Safari) don't support the 'integrity' property. + if (nativeMethods.scriptIntegrityGetter && nativeMethods.linkIntegrityGetter) { + this._overrideAttrDescriptors('integrity', [window.HTMLScriptElement]); + this._overrideAttrDescriptors('integrity', [window.HTMLLinkElement]); + } + this._overrideUrlPropDescriptor('port', nativeMethods.anchorPortGetter, nativeMethods.anchorPortSetter); this._overrideUrlPropDescriptor('host', nativeMethods.anchorHostGetter, nativeMethods.anchorHostSetter); this._overrideUrlPropDescriptor('hostname', nativeMethods.anchorHostnameGetter, nativeMethods.anchorHostnameSetter); diff --git a/src/processing/dom/index.js b/src/processing/dom/index.js index 1ec7f6ce6a..d57ba0063f 100644 --- a/src/processing/dom/index.js +++ b/src/processing/dom/index.js @@ -24,7 +24,8 @@ const SVG_XLINK_HREF_TAGS = [ 'mpath', 'pattern', 'script', 'textpath', 'use', 'tref' ]; -const TARGET_ATTR_TAGS = ['a', 'form', 'area', 'base']; +const TARGET_ATTR_TAGS = ['a', 'form', 'area', 'base']; +const INTEGRITY_ATTR_TAGS = ['script', 'link']; /*eslint-disable hammerhead/proto-methods*/ const IFRAME_FLAG_TAGS = TARGET_ATTR_TAGS.filter(tagName => tagName !== 'base').concat('button'); /*eslint-enable hammerhead/proto-methods*/ @@ -51,6 +52,10 @@ export default class DomProcessor { return TARGET_ATTR_TAGS.indexOf(tagName) !== -1; } + static isTagWithIntegrityAttr (tagName) { + return INTEGRITY_ATTR_TAGS.indexOf(tagName) !== -1; + } + static isIframeFlagTag (tagName) { return tagName && IFRAME_FLAG_TAGS.indexOf(tagName) !== -1; } @@ -290,9 +295,18 @@ export default class DomProcessor { // NOTE: We simply remove the 'integrity' attribute because its value will not be relevant after the script // content changes (http://www.w3.org/TR/SRI/). If this causes problems in the future, we will need to generate - // the correct SHA for the changed script. (GH-235) + // the correct SHA for the changed script. + // In addition, we create stored 'integrity' attribute with the current 'integrity' attribute value. (GH-235) _processIntegrityAttr (el) { - this.adapter.removeAttr(el, 'integrity'); + const storedIntegrityAttr = DomProcessor.getStoredAttrName('integrity'); + const processed = this.adapter.hasAttr(el, storedIntegrityAttr) && !this.adapter.hasAttr(el, 'integrity'); + const attrValue = this.adapter.getAttr(el, processed ? storedIntegrityAttr : 'integrity'); + + if (attrValue) + this.adapter.setAttr(el, storedIntegrityAttr, attrValue); + + if (!processed) + this.adapter.removeAttr(el, 'integrity'); } _processJsAttr (el, attrName, { isJsProtocol, isEventAttr }) { diff --git a/test/client/fixtures/sandbox/node/attributes-test.js b/test/client/fixtures/sandbox/node/attributes-test.js index d92a0b5f52..abb10f4aea 100644 --- a/test/client/fixtures/sandbox/node/attributes-test.js +++ b/test/client/fixtures/sandbox/node/attributes-test.js @@ -181,6 +181,54 @@ test('script src', function () { settings.get().sessionId = storedSessionId; }); +test('"integrity" attribute (GH-235)', function () { + var script = nativeMethods.createElement.call(document, 'script'); + var link = nativeMethods.createElement.call(document, 'link'); + var integrityValue = 'sha384-oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8wC'; + + function checkIntegrityAttr (el, hasIntegrityAttr) { + ok(hasIntegrityAttr ? el.hasAttribute('integrity') : !el.hasAttribute('integrity')); + strictEqual(el.getAttribute('integrity'), hasIntegrityAttr ? integrityValue : null); + strictEqual(nativeMethods.getAttribute.call(el, 'integrity'), null); + } + + script.setAttribute('integrity', integrityValue); + link.setAttribute('integrity', integrityValue); + + checkIntegrityAttr(script, true); + checkIntegrityAttr(link, true); + + + // Check integrity attribute when only NS integrity attribute sets (incorrect integrity attribute) + script = nativeMethods.createElement.call(document, 'script'); + link = nativeMethods.createElement.call(document, 'link'); + + script.setAttributeNS('http://www.example.com/ns/specialspace', 'specialspace:integrity', integrityValue); + script.setAttributeNS('http://www.example.com/ns/specialspace', 'specialspace:integrity', integrityValue); + + checkIntegrityAttr(script, false); + checkIntegrityAttr(link, false); +}); + +if (nativeMethods.scriptIntegrityGetter && nativeMethods.linkIntegrityGetter) { + test('"integrity" property', function () { + var integrityValue = 'sha384-oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8wC'; + var script = nativeMethods.createElement.call(document, 'script'); + var link = nativeMethods.createElement.call(document, 'link'); + + function checkIntegrityAttr (el) { + strictEqual(el.integrity, integrityValue); + strictEqual(nativeMethods.getAttribute.call(el, 'integrity'), null); + } + + script.integrity = integrityValue; + link.integrity = integrityValue; + + checkIntegrityAttr(script); + checkIntegrityAttr(link); + }); +} + test('iframe with "javascript: ..." src', function () { return createTestIframe({ src: 'javascript:" - - + +