Add hljs and improve sanitization
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is failing Details

This commit is contained in:
Bad 2020-10-29 10:42:17 +01:00
parent c144d75c99
commit bd9623578f
7 changed files with 77 additions and 45 deletions

5
package-lock.json generated
View File

@ -2412,6 +2412,11 @@
"integrity": "sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==", "integrity": "sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==",
"dev": true "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": { "hmac-drbg": {
"version": "1.0.1", "version": "1.0.1",
"resolved": "https://registry.npmjs.org/hmac-drbg/-/hmac-drbg-1.0.1.tgz", "resolved": "https://registry.npmjs.org/hmac-drbg/-/hmac-drbg-1.0.1.tgz",

View File

@ -12,7 +12,8 @@
"author": "", "author": "",
"license": "AGPL-3.0-only", "license": "AGPL-3.0-only",
"dependencies": { "dependencies": {
"dompurify": "^2.2.0" "dompurify": "^2.2.0",
"highlight.js": "^10.3.1"
}, },
"devDependencies": { "devDependencies": {
"@babel/core": "^7.11.1", "@babel/core": "^7.11.1",

View File

@ -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}

View File

@ -44,7 +44,9 @@ class MatrixEvent extends ElemJS {
if (this.editedAt) { if (this.editedAt) {
this.child(ejs("span").class("c-message__edited").text("(edited)").attribute("title", "at " + dateFormatter.format(this.editedAt))) this.child(ejs("span").class("c-message__edited").text("(edited)").attribute("title", "at " + dateFormatter.format(this.editedAt)))
} }
return this
} }
static canRender(_event) { static canRender(_event) {
return false return false
} }

View File

@ -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 DOMPurify = require("dompurify")
const {resolveMxc} = require("../functions") const {resolveMxc} = require("../functions")
const {MatrixEvent} = require("./event") const {MatrixEvent} = require("./event")
const purifier = DOMPurify() const purifier = DOMPurify()
purifier.setConfig({ purifier.addHook("uponSanitizeAttribute", (node, hookevent, config) => {
ALLOWED_URI_REGEXP: /^mxc:\/\/[a-zA-Z0-9\.]+\/[a-zA-Z0-9]+$/, // As per the spec we only allow mxc uris //If purifier already rejected an attribute there is no point in checking it
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'], if (hookevent.keepAttr === false) return;
// In case we mess up none of those attributes should read to XSS const allowedElementAttributes = {
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: {
"FONT": ["data-mx-bg-color", "data-mx-color", "color"], "FONT": ["data-mx-bg-color", "data-mx-color", "color"],
"SPAN": ["data-mx-bg-color", "data-mx-color", "color"], "SPAN": ["data-mx-bg-color", "data-mx-color", "color"],
"A": ["name", "target", "href"], "A": ["name", "target", "href"],
@ -23,58 +19,73 @@ purifier.setConfig({
"OL": ["start"], "OL": ["start"],
"CODE": ["class"], "CODE": ["class"],
} }
})
//Handle our custom tag const allowed_attributes = allowedElementAttributes[node.tagName] || []
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] || []
hookevent.keepAttr = allowed_attributes.indexOf(hookevent.attrName) > -1; hookevent.keepAttr = allowed_attributes.indexOf(hookevent.attrName) > -1;
}) })
//Remove bad classes from our code element //Remove bad classes from our code element
purifier.addHook("uponSanitizeElement", (node, hookevent, config) => { purifier.addHook("uponSanitizeElement", (node, hookevent, config) => {
if (node.tagName != "CODE") return if (node.tagName == "CODE") {
node.classList.forEach(c => { node.classList.forEach(c => {
if (!c.startsWith("language-")) { if (!c.startsWith("language-")) {
node.classList.remove(c) node.classList.remove(c)
} }
}) })
return node }
}) if (node.tagName == "A") {
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") {
node.setAttribute("rel", "noopener") 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 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) 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 { class HTMLMessage extends MatrixEvent {
render() { render() {
super.render() this.clearChildren()
let html = this.data.content.formatted_body let html = this.data.content.formatted_body
const content = ejs("div") const content = ejs("div")
html = sanitize(html)
html = cleanHTML(html)
content.html(html) content.html(html)
postProcessElements(content)
this.child(content) this.child(content)
super.render()
} }
static canRender(event) { static canRender(event) {
@ -91,8 +102,8 @@ class HTMLMessage extends MatrixEvent {
class TextMessage extends MatrixEvent { class TextMessage extends MatrixEvent {
render() { render() {
super.render() this.text(this.data.content.body)
return this.text(this.data.content.body) return super.render()
} }
static canRender(event) { static canRender(event) {

View File

@ -0,0 +1 @@
@use "../../../node_modules/highlight.js/scss/obsidian"

View File

@ -5,4 +5,5 @@
@use "./components/chat" @use "./components/chat"
@use "./components/chat-input" @use "./components/chat-input"
@use "./components/anchor" @use "./components/anchor"
@use "./components/highlighted-code"
@use "./loading" @use "./loading"