From bd9623578f9e31fd640c3edaa09a513382a31811 Mon Sep 17 00:00:00 2001 From: Bad Date: Thu, 29 Oct 2020 10:42:17 +0100 Subject: [PATCH] Add hljs and improve sanitization --- package-lock.json | 5 ++ package.json | 3 +- src/js/events/components.js | 11 +++ src/js/events/event.js | 2 + src/js/events/message.js | 99 +++++++++++++---------- src/sass/components/highlighted-code.sass | 1 + src/sass/main.sass | 1 + 7 files changed, 77 insertions(+), 45 deletions(-) create mode 100644 src/js/events/components.js create mode 100644 src/sass/components/highlighted-code.sass diff --git a/package-lock.json b/package-lock.json index 2174afb..efaf46a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2412,6 +2412,11 @@ "integrity": "sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==", "dev": true }, + "highlight.js": { + "version": "10.3.1", + "resolved": "https://registry.npmjs.org/highlight.js/-/highlight.js-10.3.1.tgz", + "integrity": "sha512-jeW8rdPdhshYKObedYg5XGbpVgb1/DT4AHvDFXhkU7UnGSIjy9kkJ7zHG7qplhFHMitTSzh5/iClKQk3Kb2RFQ==" + }, "hmac-drbg": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/hmac-drbg/-/hmac-drbg-1.0.1.tgz", diff --git a/package.json b/package.json index ebec6de..4bdc271 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,8 @@ "author": "", "license": "AGPL-3.0-only", "dependencies": { - "dompurify": "^2.2.0" + "dompurify": "^2.2.0", + "highlight.js": "^10.3.1" }, "devDependencies": { "@babel/core": "^7.11.1", diff --git a/src/js/events/components.js b/src/js/events/components.js new file mode 100644 index 0000000..2280fd6 --- /dev/null +++ b/src/js/events/components.js @@ -0,0 +1,11 @@ +const {ElemJS} = require("../basic") +const hljs = require("highlight.js") + +class HighlightedCode extends ElemJS { + constructor(code) { + super(code) + hljs.highlightBlock(this.element) + } +} + +module.exports = {HighlightedCode} diff --git a/src/js/events/event.js b/src/js/events/event.js index 4ec6c54..765c03a 100644 --- a/src/js/events/event.js +++ b/src/js/events/event.js @@ -44,7 +44,9 @@ class MatrixEvent extends ElemJS { if (this.editedAt) { this.child(ejs("span").class("c-message__edited").text("(edited)").attribute("title", "at " + dateFormatter.format(this.editedAt))) } + return this } + static canRender(_event) { return false } diff --git a/src/js/events/message.js b/src/js/events/message.js index c50ec57..05ccfdd 100644 --- a/src/js/events/message.js +++ b/src/js/events/message.js @@ -1,21 +1,17 @@ -const {ejs} = require("../basic") +const {ejs, ElemJS} = require("../basic") +const hljs = require("highlight.js") +const {HighlightedCode} = require("./components") const DOMPurify = require("dompurify") const {resolveMxc} = require("../functions") const {MatrixEvent} = require("./event") const purifier = DOMPurify() -purifier.setConfig({ - ALLOWED_URI_REGEXP: /^mxc:\/\/[a-zA-Z0-9\.]+\/[a-zA-Z0-9]+$/, // As per the spec we only allow mxc uris - ALLOWED_TAGS: ['font', 'del', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'p', 'a', 'ul', 'ol', 'sup', 'sub', 'li', 'b', 'i', 'u', 'strong', 'em', 'strike', 'code', 'hr', 'br', 'div', 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'caption', 'pre', 'span', 'img'], +purifier.addHook("uponSanitizeAttribute", (node, hookevent, config) => { + //If purifier already rejected an attribute there is no point in checking it + if (hookevent.keepAttr === false) return; - // In case we mess up none of those attributes should read to XSS - ALLOWED_ATTR: ["data-mx-bg-color", "data-mx-color", "color", - "name", "target", "href", - "width", "height", "alt", "title", "src", "data-mx-emoticon", - "start", "class"], - //Custom config option that allows the array of attributes for a given tag - ALLOWED_ATTR_CUSTOM: { + const allowedElementAttributes = { "FONT": ["data-mx-bg-color", "data-mx-color", "color"], "SPAN": ["data-mx-bg-color", "data-mx-color", "color"], "A": ["name", "target", "href"], @@ -23,58 +19,73 @@ purifier.setConfig({ "OL": ["start"], "CODE": ["class"], } -}) -//Handle our custom tag -purifier.addHook("uponSanitizeAttribute", (node, hookevent, config) => { - //If purifier already rejected an attribute there is no point in checking it - if (hookevent.keepAttr === false) return; - - const allowed_attributes = config.ALLOWED_ATTR_CUSTOM[node.tagName] || [] + const allowed_attributes = allowedElementAttributes[node.tagName] || [] hookevent.keepAttr = allowed_attributes.indexOf(hookevent.attrName) > -1; }) //Remove bad classes from our code element purifier.addHook("uponSanitizeElement", (node, hookevent, config) => { - if (node.tagName != "CODE") return - node.classList.forEach(c => { - if (!c.startsWith("language-")) { - node.classList.remove(c) - } - }) - return node -}) - -purifier.addHook("afterSanitizeAttributes", (node, hookevent, config) => { - if (node.tagName == "IMG") { - let src = node.getAttribute("src") - if (src) src = resolveMxc(src) - - node.setAttribute("src", src) - } else if (node.tagName == "A") { + if (node.tagName == "CODE") { + node.classList.forEach(c => { + if (!c.startsWith("language-")) { + node.classList.remove(c) + } + }) + } + if (node.tagName == "A") { node.setAttribute("rel", "noopener") - } else if (node.tagName == "FONT" || node.tagName == "SPAN") { - const color = node.getAttribute("data-mx-color") - const bgColor = node.getAttribute("data-mx-bg-color") - if (color) node.style.color = color; - if (bgColor) node.style.backgroundColor = bgColor; } return node }) +function cleanHTML(html) { + const config = { + ALLOWED_URI_REGEXP: /^mxc:\/\/[a-zA-Z0-9\.]+\/[a-zA-Z0-9]+$/, // As per the spec we only allow mxc uris + ALLOWED_TAGS: ['font', 'del', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'p', 'a', 'ul', 'ol', 'sup', 'sub', 'li', 'b', 'i', 'u', 'strong', 'em', 'strike', 'code', 'hr', 'br', 'div', 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'caption', 'pre', 'span', 'img'], -function sanitize(html) { + // In case we mess up in the uponSanitizeAttribute hook + ALLOWED_ATTR: ["data-mx-bg-color", "data-mx-color", "color", + "name", "target", "href", + "width", "height", "alt", "title", "src", "data-mx-emoticon", + "start", "class"], + } return purifier.sanitize(html) } +//Here we put all the processing of the messages that isn't as likely to potentially lead to security issues +function postProcessElements(rootNode) { + const element = rootNode.element + element.querySelectorAll("code").forEach((n) => rootNode.child(new HighlightedCode(n))) + + element.querySelectorAll("img").forEach((n) => { + let src = n.getAttribute("src") + if (src) src = resolveMxc(src) + n.setAttribute("src", src) + }) + element.querySelectorAll("font, span").forEach((n) => { + const color = n.getAttribute("data-mx-color") || n.getAttribute("color") + const bgColor = n.getAttribute("data-mx-bg-color") + if (color) n.style.color = color; + if (bgColor) n.style.backgroundColor = bgColor; + }) +} + + class HTMLMessage extends MatrixEvent { render() { - super.render() + this.clearChildren() + let html = this.data.content.formatted_body const content = ejs("div") - html = sanitize(html) + + html = cleanHTML(html) content.html(html) + postProcessElements(content) + this.child(content) + + super.render() } static canRender(event) { @@ -91,8 +102,8 @@ class HTMLMessage extends MatrixEvent { class TextMessage extends MatrixEvent { render() { - super.render() - return this.text(this.data.content.body) + this.text(this.data.content.body) + return super.render() } static canRender(event) { diff --git a/src/sass/components/highlighted-code.sass b/src/sass/components/highlighted-code.sass new file mode 100644 index 0000000..1ec7290 --- /dev/null +++ b/src/sass/components/highlighted-code.sass @@ -0,0 +1 @@ +@use "../../../node_modules/highlight.js/scss/obsidian" diff --git a/src/sass/main.sass b/src/sass/main.sass index 150af73..24cd6ca 100644 --- a/src/sass/main.sass +++ b/src/sass/main.sass @@ -5,4 +5,5 @@ @use "./components/chat" @use "./components/chat-input" @use "./components/anchor" +@use "./components/highlighted-code" @use "./loading"