From c8dbd3d5deda2a17e8d76998a84da6adebdf1dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= Date: Tue, 28 May 2024 14:42:12 +0100 Subject: [PATCH 1/2] Ensure we always sanitize HTML we set as innerHTML --- src/test/system/pasting_test.js | 2 +- src/test/test_helpers/fixtures/fixtures.js | 2 +- src/trix/models/html_parser.js | 11 ++--------- src/trix/models/html_sanitizer.js | 4 ++++ src/trix/views/attachment_view.js | 5 +++-- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/test/system/pasting_test.js b/src/test/system/pasting_test.js index fa9891c6d..492ac0927 100644 --- a/src/test/system/pasting_test.js +++ b/src/test/system/pasting_test.js @@ -109,7 +109,7 @@ testGroup("Pasting", { template: "editor_empty" }, () => { const pasteData = { "text/plain": "x", "text/html": `\ - copy
me + copy
me `, } diff --git a/src/test/test_helpers/fixtures/fixtures.js b/src/test/test_helpers/fixtures/fixtures.js index 9224889af..746f3da2c 100644 --- a/src/test/test_helpers/fixtures/fixtures.js +++ b/src/test/test_helpers/fixtures/fixtures.js @@ -470,7 +470,7 @@ export const fixtures = { "content attachment": (() => { const content = - "

ruby-build 20150413 is out, with definitions for 2.2.2, 2.1.6, and 2.0.0-p645 to address recent security issues: https://t.co/YEwV6NtRD8

— Sam Stephenson (@sstephenson) April 13, 2015
" + "

ruby-build 20150413 is out, with definitions for 2.2.2, 2.1.6, and 2.0.0-p645 to address recent security issues: https://t.co/YEwV6NtRD8

— Sam Stephenson (@sstephenson) April 13, 2015
" const href = "https://twitter.com/sstephenson/status/587715996783218688" const contentType = "embed/twitter" diff --git a/src/trix/models/html_parser.js b/src/trix/models/html_parser.js index de3d3a7dc..73429bcde 100644 --- a/src/trix/models/html_parser.js +++ b/src/trix/models/html_parser.js @@ -40,13 +40,7 @@ const blockForAttributes = (attributes = {}, htmlAttributes = {}) => { const parseTrixDataAttribute = (element, name) => { try { - const data = JSON.parse(element.getAttribute(`data-trix-${name}`)) - - if (data.contentType === "text/html" && data.content) { - data.content = HTMLSanitizer.sanitize(data.content).getHTML() - } - - return data + return JSON.parse(element.getAttribute(`data-trix-${name}`)) } catch (error) { return {} } @@ -90,8 +84,7 @@ export default class HTMLParser extends BasicObject { parse() { try { this.createHiddenContainer() - const html = HTMLSanitizer.sanitize(this.html).getHTML() - this.containerElement.innerHTML = html + HTMLSanitizer.setHTML(this.containerElement, this.html) const walker = walkTree(this.containerElement, { usingFilter: nodeFilter }) while (walker.nextNode()) { this.processNode(walker.currentNode) diff --git a/src/trix/models/html_sanitizer.js b/src/trix/models/html_sanitizer.js index 12893dc30..8473255a4 100644 --- a/src/trix/models/html_sanitizer.js +++ b/src/trix/models/html_sanitizer.js @@ -7,6 +7,10 @@ const DEFAULT_FORBIDDEN_PROTOCOLS = "javascript:".split(" ") const DEFAULT_FORBIDDEN_ELEMENTS = "script iframe form noscript".split(" ") export default class HTMLSanitizer extends BasicObject { + static setHTML(element, html) { + element.innerHTML = new this(html).sanitize().getHTML() + } + static sanitize(html, options) { const sanitizer = new this(html, options) sanitizer.sanitize() diff --git a/src/trix/views/attachment_view.js b/src/trix/views/attachment_view.js index 231e25360..17a9cef8c 100644 --- a/src/trix/views/attachment_view.js +++ b/src/trix/views/attachment_view.js @@ -2,6 +2,7 @@ import * as config from "trix/config" import { ZERO_WIDTH_SPACE } from "trix/constants" import { copyObject, makeElement } from "trix/core/helpers" import ObjectView from "trix/views/object_view" +import HTMLSanitizer from "trix/models/html_sanitizer" const { css } = config @@ -33,7 +34,7 @@ export default class AttachmentView extends ObjectView { } if (this.attachment.hasContent()) { - innerElement.innerHTML = this.attachment.getContent() + HTMLSanitizer.setHTML(innerElement, this.attachment.getContent()) } else { this.createContentNodes().forEach((node) => { innerElement.appendChild(node) @@ -165,6 +166,6 @@ const createCursorTarget = (name) => const htmlContainsTagName = function(html, tagName) { const div = makeElement("div") - div.innerHTML = html || "" + HTMLSanitizer.setHTML(div, html || "") return div.querySelector(tagName) } From 626a4f41d539586c2113d2f76bf029a7fb366cec Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 30 Jul 2024 16:58:33 +0200 Subject: [PATCH 2/2] Support browsers where getHTML is not supported --- src/trix/models/html_sanitizer.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/trix/models/html_sanitizer.js b/src/trix/models/html_sanitizer.js index 8473255a4..3dd0b3e68 100644 --- a/src/trix/models/html_sanitizer.js +++ b/src/trix/models/html_sanitizer.js @@ -8,7 +8,9 @@ const DEFAULT_FORBIDDEN_ELEMENTS = "script iframe form noscript".split(" ") export default class HTMLSanitizer extends BasicObject { static setHTML(element, html) { - element.innerHTML = new this(html).sanitize().getHTML() + const sanitizedElement = new this(html).sanitize() + const sanitizedHtml = sanitizedElement.getHTML ? sanitizedElement.getHTML() : sanitizedElement.outerHTML + element.innerHTML = sanitizedHtml } static sanitize(html, options) {