Fix purify and highlight

- purify: apply target=_blank to links
- purify: remove ALLOWED_URI_REGEXP - this breaks external links in
  anchor elements
- purify: return a DOM fragment instead of a string
- postprocess: only highlight pre
- postprocess: remove nested code inside pre
- better style messages with css

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) {
+	constructor(element) {
-		super(code)
+		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))
 	}
 }

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] || []
+	const allowedAttributes = allowedElementAttributes[node.tagName] || []
-	hookevent.keepAttr = allowed_attributes.indexOf(hookevent.attrName) > -1;
+	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") {
+	if (node.tagName === "A") {
-		node.setAttribute("rel", "noopener")
+		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: [
-		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'],
+			"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",
+		ALLOWED_ATTR: [
-			"name", "target", "href",
+			"color", "name", "target", "href", "width", "height", "alt", "title",
-			"width", "height", "alt", "title", "src", "data-mx-emoticon",
+			"src", "start", "class", "noreferrer", "noopener",
-			"start", "class"],
+			// 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 (color) n.style.color = color
-		if (bgColor) n.style.backgroundColor = bgColor;
+		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)
+		const fragment = cleanHTML(html)
-		content.html(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() {

src/sass/colors.sass

@@ -5,3 +5,4 @@ $mild: #393c42
 $milder: #42454a
 $divider: #4b4e54
 $muted: #999
+$link: #57bffd

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