Skip to content

Commit 5f61df0

Browse files
committed
Use SAFE_FOR_XML for pasted content
We default SAFE_FOR_XML to false to allow comments in attachments. However we don't need when pasting content and it exposes us to potential XSS attacks there, so let's override the setting when pasting.
1 parent 2d72f93 commit 5f61df0

File tree

4 files changed

+25
-7
lines changed

4 files changed

+25
-7
lines changed

src/test/system/pasting_test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,21 @@ testGroup("Pasting", { template: "editor_empty" }, () => {
119119
delete window.unsanitized
120120
})
121121

122+
test("paste data-trix-attachment unsafe html div overload", async () => {
123+
window.unsanitized = []
124+
const pasteData = {
125+
"text/plain": "x",
126+
"text/html": `\
127+
<div data-trix-attachment="{&quot;contentType&quot;:&quot;text/html5&quot;,&quot;content&quot;:&quot;<div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><div><a><svg><desc><svg><image><a><desc><svg><image></image></svg></desc></a></image><style><a data-trix-caca='</style><img src=x onerror=window.unsanitized.push(1)>'></a></style></svg></desc></svg></a>&quot;}"></div>
128+
`,
129+
}
130+
131+
await pasteContent(pasteData)
132+
await delay(20)
133+
assert.deepEqual(window.unsanitized, [])
134+
delete window.unsanitized
135+
})
136+
122137
test("paste data-trix-attachment encoded mathml", async () => {
123138
window.unsanitized = []
124139
const pasteData = {

src/trix/models/composition.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export default class Composition extends BasicObject {
127127
}
128128

129129
insertHTML(html) {
130-
const document = HTMLParser.parse(html).getDocument()
130+
const document = HTMLParser.parse(html, { purifyOptions: { SAFE_FOR_XML: true } }).getDocument()
131131
const selectedRange = this.getSelectedRange()
132132

133133
this.setDocument(this.document.mergeDocumentAtRange(document, selectedRange))

src/trix/models/html_parser.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ export default class HTMLParser extends BasicObject {
6666
return parser
6767
}
6868

69-
constructor(html, { referenceElement } = {}) {
69+
constructor(html, { referenceElement, purifyOptions } = {}) {
7070
super(...arguments)
7171
this.html = html
7272
this.referenceElement = referenceElement
73+
this.purifyOptions = purifyOptions
7374
this.blocks = []
7475
this.blockElements = []
7576
this.processedElements = []
@@ -84,7 +85,7 @@ export default class HTMLParser extends BasicObject {
8485
parse() {
8586
try {
8687
this.createHiddenContainer()
87-
HTMLSanitizer.setHTML(this.containerElement, this.html)
88+
HTMLSanitizer.setHTML(this.containerElement, this.html, { purifyOptions: this.purifyOptions })
8889
const walker = walkTree(this.containerElement, { usingFilter: nodeFilter })
8990
while (walker.nextNode()) {
9091
this.processNode(walker.currentNode)

src/trix/models/html_sanitizer.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ const DEFAULT_FORBIDDEN_PROTOCOLS = "javascript:".split(" ")
1616
const DEFAULT_FORBIDDEN_ELEMENTS = "script iframe form noscript".split(" ")
1717

1818
export default class HTMLSanitizer extends BasicObject {
19-
static setHTML(element, html) {
20-
const sanitizedElement = new this(html).sanitize()
19+
static setHTML(element, html, options) {
20+
const sanitizedElement = new this(html, options).sanitize()
2121
const sanitizedHtml = sanitizedElement.getHTML ? sanitizedElement.getHTML() : sanitizedElement.outerHTML
2222
element.innerHTML = sanitizedHtml
2323
}
@@ -28,18 +28,20 @@ export default class HTMLSanitizer extends BasicObject {
2828
return sanitizer
2929
}
3030

31-
constructor(html, { allowedAttributes, forbiddenProtocols, forbiddenElements } = {}) {
31+
constructor(html, { allowedAttributes, forbiddenProtocols, forbiddenElements, purifyOptions } = {}) {
3232
super(...arguments)
3333
this.allowedAttributes = allowedAttributes || DEFAULT_ALLOWED_ATTRIBUTES
3434
this.forbiddenProtocols = forbiddenProtocols || DEFAULT_FORBIDDEN_PROTOCOLS
3535
this.forbiddenElements = forbiddenElements || DEFAULT_FORBIDDEN_ELEMENTS
36+
this.purifyOptions = purifyOptions || {}
3637
this.body = createBodyElementForHTML(html)
3738
}
3839

3940
sanitize() {
4041
this.sanitizeElements()
4142
this.normalizeListElementNesting()
42-
DOMPurify.setConfig(config.dompurify)
43+
const purifyConfig = Object.assign({}, config.dompurify, this.purifyOptions)
44+
DOMPurify.setConfig(purifyConfig)
4345
this.body = DOMPurify.sanitize(this.body)
4446

4547
return this.body

0 commit comments

Comments
 (0)