diff --git a/core/modules/filter/filter.filter_html.admin.es6.js b/core/modules/filter/filter.filter_html.admin.es6.js index 7ddca90bba..3c85166e8b 100644 --- a/core/modules/filter/filter.filter_html.admin.es6.js +++ b/core/modules/filter/filter.filter_html.admin.es6.js @@ -3,7 +3,7 @@ * Attaches behavior for updating filter_html's settings automatically. */ -(function ($, Drupal, _, document) { +(function ($, Drupal, CKEDITOR, _, document) { 'use strict'; @@ -230,45 +230,32 @@ * tag name. */ _parseSetting: function (setting) { - var node; var tag; var rule; - var attributes; - var attribute; - var allowedTags = setting.match(/(<[^>]+>)/g); - var sandbox = document.createElement('div'); var rules = {}; - for (var t = 0; t < allowedTags.length; t++) { - // Let the browser do the parsing work for us. - sandbox.innerHTML = allowedTags[t]; - node = sandbox.firstChild; - tag = node.tagName.toLowerCase(); + var parser = new CKEDITOR.htmlParser(); + parser.onTagOpen = function (tagName, attributes, selfClosing) { + tag = tagName.toLowerCase(); // Build the Drupal.FilterHtmlRule object. rule = new Drupal.FilterHTMLRule(); // We create one rule per allowed tag, so always one tag. rule.restrictedTags.tags = [tag]; // Add the attribute restrictions. - attributes = node.attributes; - for (var i = 0; i < attributes.length; i++) { - attribute = attributes.item(i); - var attributeName = attribute.nodeName; - // @todo Drupal.FilterHtmlRule does not allow for generic attribute - // value restrictions, only for the "class" and "style" attribute's - // values. The filter_html filter always disallows the "style" - // attribute, so we only need to support "class" attribute value - // restrictions. Fix once https://www.drupal.org/node/2567801 lands. - if (attributeName === 'class') { - var attributeValue = attribute.textContent; + Object.keys(attributes).forEach(function (nodeName) { + if (nodeName === 'class') { + var attributeValue = attributes[nodeName]; rule.restrictedTags.allowed.classes = attributeValue.split(' '); } else { - rule.restrictedTags.allowed.attributes.push(attributeName); + rule.restrictedTags.allowed.attributes.push(nodeName); } - } + }); rules[tag] = rule; - } + }; + parser.parse(setting); + return rules; }, @@ -325,4 +312,4 @@ return html; }; -})(jQuery, Drupal, _, document); +})(jQuery, Drupal, CKEDITOR, _, document); diff --git a/core/modules/filter/filter.filter_html.admin.js b/core/modules/filter/filter.filter_html.admin.js index cd1235a3f1..a9cd7d0301 100644 --- a/core/modules/filter/filter.filter_html.admin.js +++ b/core/modules/filter/filter.filter_html.admin.js @@ -6,7 +6,7 @@ * @preserve **/ -(function ($, Drupal, _, document) { +(function ($, Drupal, CKEDITOR, _, document) { 'use strict'; @@ -138,38 +138,31 @@ }, _parseSetting: function _parseSetting(setting) { - var node; var tag; var rule; - var attributes; - var attribute; - var allowedTags = setting.match(/(<[^>]+>)/g); - var sandbox = document.createElement('div'); var rules = {}; - for (var t = 0; t < allowedTags.length; t++) { - sandbox.innerHTML = allowedTags[t]; - node = sandbox.firstChild; - tag = node.tagName.toLowerCase(); + + var parser = new CKEDITOR.htmlParser(); + parser.onTagOpen = function (tagName, attributes, selfClosing) { + tag = tagName.toLowerCase(); rule = new Drupal.FilterHTMLRule(); rule.restrictedTags.tags = [tag]; - attributes = node.attributes; - for (var i = 0; i < attributes.length; i++) { - attribute = attributes.item(i); - var attributeName = attribute.nodeName; - - if (attributeName === 'class') { - var attributeValue = attribute.textContent; + Object.keys(attributes).forEach(function (nodeName) { + if (nodeName === 'class') { + var attributeValue = attributes[nodeName]; rule.restrictedTags.allowed.classes = attributeValue.split(' '); } else { - rule.restrictedTags.allowed.attributes.push(attributeName); + rule.restrictedTags.allowed.attributes.push(nodeName); } - } + }); rules[tag] = rule; - } + }; + parser.parse(setting); + return rules; }, @@ -203,4 +196,4 @@ html += '

'; return html; }; -})(jQuery, Drupal, _, document); \ No newline at end of file +})(jQuery, Drupal, CKEDITOR, _, document); \ No newline at end of file diff --git a/core/modules/filter/filter.libraries.yml b/core/modules/filter/filter.libraries.yml index 91aea0be5b..3ce684e85c 100644 --- a/core/modules/filter/filter.libraries.yml +++ b/core/modules/filter/filter.libraries.yml @@ -19,6 +19,7 @@ drupal.filter.filter_html.admin: - core/jquery - core/jquery.once - core/underscore + - core/ckeditor drupal.filter: version: VERSION diff --git a/core/modules/filter/tests/src/FunctionalJavascript/FilterIntegrationTest.php b/core/modules/filter/tests/src/FunctionalJavascript/FilterIntegrationTest.php new file mode 100644 index 0000000000..2665de97f1 --- /dev/null +++ b/core/modules/filter/tests/src/FunctionalJavascript/FilterIntegrationTest.php @@ -0,0 +1,104 @@ + 'basic_html', + 'name' => 'Basic HTML', + 'filters' => [ + 'filter_html' => [ + 'status' => 1, + 'settings' => [ + 'allowed_html' => '


', + ], + ], + ], + ]); + $basic_html_format->save(); + + + $this->adminUser = $this->drupalCreateUser([ + 'administer filters', + $basic_html_format->getPermissionName(), + ]); + + $this->drupalLogin($this->adminUser); + } + + /** + * Tests if we can exploit JS injection when setting allowed HTML tags. + */ + public function testJSInjection() { + $this->drupalGet('admin/config/content/formats/manage/basic_html'); + $page = $this->getSession()->getPage(); + + // Test that javascript is run when adding an img tag with 'onerror'. + $injected_tag = ' '; + $page->fillField('edit-filters-filter-html-settings-allowed-html', $injected_tag); + + // JavaScript blur event does not work properly on the test engine. + // So we reload the page and checking the result. + $page->pressButton('Save configuration'); + $this->drupalGet('admin/config/content/formats/manage/basic_html'); + $this->assertText('hacked'); + } + + /** + * Tests if HTML parser produces error. + * @see: https://www.drupal.org/node/2865842 + */ + public function testParserError() { + $this->drupalGet('admin/config/content/formats/manage/basic_html'); + $page = $this->getSession()->getPage(); + + $tags = [ + 'a', 'abbr', 'acronym', 'address', 'area', 'b', 'base', 'bdo', + 'big', 'blockquote', 'body', 'br', 'button', 'caption', 'cite', + 'code', 'col', 'colgroup', 'dd', 'del', 'div', 'dfn', 'dl', + 'dt', 'em', 'fieldset', 'form', 'frame', 'frameset', 'h1', 'h2', + 'h3', 'h4', 'h5', 'h6', 'head', 'hr', 'html', 'i', 'iframe', + 'img', 'input', 'ins', 'kbd', 'label', 'legend', 'li', 'link', + 'map', 'meta', 'noframes', 'noscript', 'object', 'ol', 'optgroup', + 'option', 'p', 'param', 'pre', 'q', 'samp', 'script', 'select', + 'small', 'span', 'strong', 'style', 'sub', 'sup', 'table', + 'tbody', 'td', 'textarea', 'tfoot', 'th', 'thead', 'title', + 'tr', 'tt', 'ul', 'var', + // HTML5 + 'article', 'aside', 'audio', 'bdi', 'canvas', 'command', + 'datalist', 'details', 'dialog', 'embed', 'figure', 'figcaption', + 'footer', 'header', 'hgroup', 'keygen', 'mark', 'meter', 'nav', + 'output', 'progress', 'ruby', 'rt', 'rp', 'track', 'section', + 'source', 'summary', 'time', 'video', 'wbr' + ]; + + $tags = array_map(function ($tag) { + return '<' . $tag . '>'; + }, $tags); + + $tags = implode(' ', $tags); + + // FunctionalJavascript test won't catch JavaScript error. + // @todo: Convert to JavaScript UnitTest. + $this->getSession()->executeScript('Drupal.behaviors.filterFilterHtmlUpdating._parseSetting("' . $tags . '")'); + } + +}