Skip to content

Commit

Permalink
Ensure we always sanitize HTML we set as innerHTML
Browse files Browse the repository at this point in the history
  • Loading branch information
afcapel committed May 28, 2024
1 parent 0c79bcb commit 8f7608e
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/test/system/pasting_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ testGroup("Pasting", { template: "editor_empty" }, () => {
const pasteData = {
"text/plain": "x",
"text/html": `\
copy<div data-trix-attachment="{&quot;contentType&quot;:&quot;text/html&quot;,&quot;content&quot;:&quot;&lt;img src=1 onerror=window.unsanitized.push(1)&gt;HELLO123&quot;}"></div>me
copy<div data-trix-attachment="{&quot;contentType&quot;:&quot;text/anything&quot;,&quot;content&quot;:&quot;&lt;img src=1 onerror=window.unsanitized.push(1)&gt;HELLO123&quot;}"></div>me
`,
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/test_helpers/fixtures/fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ export const fixtures = {

"content attachment": (() => {
const content =
"<blockquote class=\"twitter-tweet\" data-cards=\"hidden\"><p>ruby-build 20150413 is out, with definitions for 2.2.2, 2.1.6, and 2.0.0-p645 to address recent security issues: <a href=\"https://t.co/YEwV6NtRD8\">https://t.co/YEwV6NtRD8</a></p>&mdash; Sam Stephenson (@sstephenson) <a href=\"https://twitter.com/sstephenson/status/587715996783218688\">April 13, 2015</a></blockquote>"
"<blockquote class=\"twitter-tweet\"><p>ruby-build 20150413 is out, with definitions for 2.2.2, 2.1.6, and 2.0.0-p645 to address recent security issues: <a href=\"https://t.co/YEwV6NtRD8\">https://t.co/YEwV6NtRD8</a></p>&mdash; Sam Stephenson (@sstephenson) <a href=\"https://twitter.com/sstephenson/status/587715996783218688\">April 13, 2015</a></blockquote>"
const href = "https://twitter.com/sstephenson/status/587715996783218688"
const contentType = "embed/twitter"

Expand Down
11 changes: 2 additions & 9 deletions src/trix/models/html_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
}
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/trix/models/html_sanitizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 3 additions & 2 deletions src/trix/views/attachment_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

0 comments on commit 8f7608e

Please sign in to comment.