diff --git a/src/js/events/components.js b/src/js/events/components.js index bb0cc63..79e3a8a 100644 --- a/src/js/events/components.js +++ b/src/js/events/components.js @@ -2,8 +2,16 @@ const {ElemJS} = require("../basic") const {lazyLoad} = require("../lazy-load-module") class HighlightedCode extends ElemJS { - constructor(code) { - super(code) + constructor(element) { + super(element) + if (this.element.tagName === "PRE" && this.element.children.length === 1 && this.element.children[0].tagName === "CODE") { + // we shouldn't nest code inside a pre. put the text in the pre directly. + const code = this.element.children[0] + this.clearChildren() + for (const child of code.childNodes) { + this.element.appendChild(child) + } + } lazyLoad("./static/hljs.js").then(hljs => hljs.highlightBlock(this.element)) } } diff --git a/src/js/events/message.js b/src/js/events/message.js index b1ea8d9..f0760af 100644 --- a/src/js/events/message.js +++ b/src/js/events/message.js @@ -12,61 +12,80 @@ purifier.addHook("uponSanitizeAttribute", (node, hookevent, config) => { const allowedElementAttributes = { "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"], "A": ["name", "target", "href"], "IMG": ["width", "height", "alt", "title", "src", "data-mx-emoticon"], "OL": ["start"], "CODE": ["class"], } - const allowed_attributes = allowedElementAttributes[node.tagName] || [] - hookevent.keepAttr = allowed_attributes.indexOf(hookevent.attrName) > -1; + const allowedAttributes = allowedElementAttributes[node.tagName] || [] + hookevent.keepAttr = allowedAttributes.indexOf(hookevent.attrName) > -1 }) purifier.addHook("uponSanitizeElement", (node, hookevent, config) => { // Remove bad classes from our code element - if (node.tagName == "CODE") { + if (node.tagName === "CODE") { node.classList.forEach(c => { if (!c.startsWith("language-")) { node.classList.remove(c) } }) } - if (node.tagName == "A") { - node.setAttribute("rel", "noopener") + if (node.tagName === "A") { + node.setAttribute("rel", "noopener") // prevent the opening page from accessing carbon + node.setAttribute("target", "_blank") // open in a new tab instead of replacing carbon } 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'], + 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", + // matrix tags + "mx-reply" + ], // 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"], + ALLOWED_ATTR: [ + "color", "name", "target", "href", "width", "height", "alt", "title", + "src", "start", "class", "noreferrer", "noopener", + // matrix attrs + "data-mx-emoticon", "data-mx-bg-color", "data-mx-color" + ], + + // Return a DOM fragment instead of a string, avoids potential future mutation XSS + // should also be faster than the browser parsing HTML twice + // https://research.securitum.com/mutation-xss-via-mathml-mutation-dompurify-2-0-17-bypass/ + RETURN_DOM_FRAGMENT: true, + RETURN_DOM_IMPORT: true } - return purifier.sanitize(html) + return purifier.sanitize(html, config) } // 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) => { + element.querySelectorAll("pre").forEach(n => { + 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) => { + + 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; + if (color) n.style.color = color + if (bgColor) n.style.backgroundColor = bgColor }) } @@ -78,8 +97,8 @@ class HTMLMessage extends MatrixEvent { let html = this.data.content.formatted_body const content = ejs("div") - html = cleanHTML(html) - content.html(html) + const fragment = cleanHTML(html) + content.element.appendChild(fragment) postProcessElements(content) this.child(content) @@ -89,24 +108,23 @@ class HTMLMessage extends MatrixEvent { static canRender(event) { const content = event.content - return event.type == "m.room.message" && content.msgtype == "m.text" && content.format == "org.matrix.custom.html" && content.formatted_body - + return event.type === "m.room.message" && content.msgtype === "m.text" && content.format === "org.matrix.custom.html" && content.formatted_body } canGroup() { return true } - } class TextMessage extends MatrixEvent { render() { + this.clearChildren() this.text(this.data.content.body) - return super.render() + super.render() } static canRender(event) { - return event.type == "m.room.message" + return event.type === "m.room.message" } canGroup() { diff --git a/src/sass/colors.sass b/src/sass/colors.sass index 278fc02..e40189c 100644 --- a/src/sass/colors.sass +++ b/src/sass/colors.sass @@ -5,3 +5,4 @@ $mild: #393c42 $milder: #42454a $divider: #4b4e54 $muted: #999 +$link: #57bffd diff --git a/src/sass/components/messages.sass b/src/sass/components/messages.sass index 2a7665a..30bece0 100644 --- a/src/sass/components/messages.sass +++ b/src/sass/components/messages.sass @@ -1,6 +1,6 @@ @use "../colors" as c -.c-event-groups * +.c-event-groups > * overflow-anchor: none .c-message-group, .c-message-event @@ -67,6 +67,33 @@ &:hover background-color: c.$darker + // message formatting rules + + code, pre + border-radius: 4px + font-size: 0.9em + + pre + background-color: c.$darkest + padding: 8px + border: 1px solid c.$divider + + code + background-color: c.$darker + padding: 2px 4px + + a + color: c.$link + + p, pre + margin: 16px 0px + + &:first-child + margin-top: 0px + + &:last-child + margin-bottom: 0px + .c-message-event padding-top: 10px padding-left: 6px