Skip to content

Commit

Permalink
fix Remove integrity attributes from resources (close DevExpress#235
Browse files Browse the repository at this point in the history 
) (DevExpress#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
  • Loading branch information
@Farfurix @AndreyBelym
Farfurix authored and AndreyBelym committed Feb 28, 2019
1 parent 9b7978c commit d2b1e03
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 14 deletions.
3 changes: 2 additions & 1 deletion Gulpfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
14 changes: 14 additions & 0 deletions src/client/sandbox/native-methods.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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;
Expand Down
23 changes: 23 additions & 0 deletions src/client/sandbox/node/element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}

Expand Down
6 changes: 6 additions & 0 deletions src/client/sandbox/node/window.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 17 additions & 3 deletions src/processing/dom/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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*/
Expand All @@ -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;
}
Expand Down Expand Up @@ -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 }) {
Expand Down
48 changes: 48 additions & 0 deletions test/client/fixtures/sandbox/node/attributes-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: <html>...</html>" src', function () {
return createTestIframe({ src: 'javascript:"<script>var d = {}; d.src = 1; window.test = true;<' + '/script>"' })
.then(function (iframe) {
Expand Down
23 changes: 15 additions & 8 deletions test/client/fixtures/sandbox/node/dom-processor-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,22 +431,29 @@ module('should create a proxy url for the img src attribute if the image has the

module('regression');

test('remove the "integrity" attribute from the link and script tags (GH-235)', function () {
var script = nativeMethods.createElement.call(document, 'script');
var link = nativeMethods.createElement.call(document, 'link');

nativeMethods.setAttribute.call(script, 'integrity', 'sha384-Li9vy3DqF8tnTXuiaAJuML3ky+er10rcgNR/VqsVpcw+ThHmYcwiB1pbOxEbzJr7');
nativeMethods.setAttribute.call(link, 'integrity', 'sha384-Li9vy3DqF8tnTXuiaAJuML3ky+er10rcgNR/VqsVpcw+ThHmYcwiB1pbOxEbzJr7');
test('process the "integrity" attribute in the link and script tags (GH-235)', function () {
var script = nativeMethods.createElement.call(document, 'script');
var link = nativeMethods.createElement.call(document, 'link');
var integrityValue = 'sha384-oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8wC';

var urlReplacer = function (url) {
return url;
};

function checkIntegrityAttr (el) {
ok(el.hasAttribute('integrity'));
strictEqual(el.getAttribute('integrity'), integrityValue);
strictEqual(nativeMethods.getAttribute.call(el, 'integrity'), null);
}

nativeMethods.setAttribute.call(script, 'integrity', integrityValue);
nativeMethods.setAttribute.call(link, 'integrity', integrityValue);

domProcessor.processElement(script, urlReplacer);
domProcessor.processElement(link, urlReplacer);

ok(!script.hasAttribute('integrity'));
ok(!link.hasAttribute('integrity'));
checkIntegrityAttr(script);
checkIntegrityAttr(link);
});

test('link with target="_parent" in iframe (T216999)', function () {
Expand Down
4 changes: 2 additions & 2 deletions test/server/data/page/expected.html
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"><html manifest="http://127.0.0.1:1836/sessionId/http://base.url/some.url" manifest-hammerhead-stored-value="/some.url"><head><meta class="charset-hammerhead-shadow-ui" charset="utf-8"><script class="self-removing-script-hammerhead-shadow-ui">(function () {window.localStorage.setItem("hammerhead|storage-wrapper|sessionId|127.0.0.1:2000", "[[\"key1\"],[\" ' \\\" \\\\ \\n \\t \\b \\f \"]]");window.sessionStorage.setItem("hammerhead|storage-wrapper|sessionId|127.0.0.1:2000", "[[\"key2\"],[\"value\"]]");var currentScript = document.currentScript;if (!currentScript) {var scripts;try {var hammerhead = window["%hammerhead%"] || parent["%hammerhead%"];scripts = hammerhead ? hammerhead.nativeMethods.documentScriptsGetter.call(document) : document.scripts;} catch (e) {scripts = document.scripts;}currentScript = scripts[scripts.length - 1];}currentScript.parentNode.removeChild(currentScript);})();</script><link rel="stylesheet" type="text/css" class="ui-stylesheet-hammerhead-shadow-ui" href="http://127.0.0.1:1836/styles1.css"><link rel="stylesheet" type="text/css" class="ui-stylesheet-hammerhead-shadow-ui" href="http://127.0.0.1:1836/styles2.css"><script type="text/javascript" class="script-hammerhead-shadow-ui" charset="UTF-8" src="http://127.0.0.1:1836/hammerhead.js"></script><script type="text/javascript" class="script-hammerhead-shadow-ui" charset="UTF-8" src="http://127.0.0.1:1836/script1.js"></script><script type="text/javascript" class="script-hammerhead-shadow-ui" charset="UTF-8" src="http://127.0.0.1:1836/script2.js"></script><script type="text/javascript" class="script-hammerhead-shadow-ui" charset="UTF-8" src="http://127.0.0.1:1836/task.js"></script>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=7">
<link id="stylesheet" rel="stylesheet" type="text/css" href="http://127.0.0.1:1836/sessionId/http://stylesheet.url" crossorigin="anonymous" href-hammerhead-stored-value="http://stylesheet.url">
<script type="text/javascript" src="http://127.0.0.1:1836/sessionId!s!utf-8/http://link.url" crossorigin="anonymous" src-hammerhead-stored-value="http://link.url"></script>
<link id="stylesheet" rel="stylesheet" type="text/css" href="http://127.0.0.1:1836/sessionId/http://stylesheet.url" crossorigin="anonymous" href-hammerhead-stored-value="http://stylesheet.url" integrity-hammerhead-stored-value="sha384-Li9vy3DqF8tnTXuiaAJuML3ky+er10rcgNR/VqsVpcw+ThHmYcwiB1pbOxEbzJr7">
<script type="text/javascript" src="http://127.0.0.1:1836/sessionId!s!utf-8/http://link.url" crossorigin="anonymous" src-hammerhead-stored-value="http://link.url" integrity-hammerhead-stored-value="sha384-Li9vy3DqF8tnTXuiaAJuML3ky+er10rcgNR/VqsVpcw+ThHmYcwiB1pbOxEbzJr7"></script>
<script type="text/javascript" charset="utf-16be" src="http://127.0.0.1:1836/sessionId!s!utf-16be/http://link.url" src-hammerhead-stored-value="http://link.url"></script>
<meta http-equiv="Refresh" content="0;URL=http://127.0.0.1:1836/sessionId/http://link.url/">
<meta>
Expand Down

0 comments on commit d2b1e03

Please sign in to comment.