FIX: Escape Font Awesome icons (#12421)

This is not a security issue because regular users are not allowed to insert FA icons anywhere in the app. Admins can insert icons via custom badges, but they do have the ability to create themes with JS.

app/assets/javascripts/discourse-common/addon/lib/escape.js

@@ -0,0 +1,32 @@
+const ESCAPE_REPLACEMENTS = {
+  "&": "&",
+  "<": "&lt;",
+  ">": "&gt;",
+  '"': "&quot;",
+  "'": "&#x27;",
+  "`": "&#x60;",
+};
+const BAD_CHARS = /[&<>"'`]/g;
+const POSSIBLE_CHARS = /[&<>"'`]/;
+
+function escapeChar(chr) {
+  return ESCAPE_REPLACEMENTS[chr];
+}
+
+export default function escape(string) {
+  if (string === null) {
+    return "";
+  } else if (!string) {
+    return string + "";
+  }
+
+  // Force a string conversion as this will be done by the append regardless and
+  // the regex test will do this transparently behind the scenes, causing issues if
+  // an object's to string has escaped characters in it.
+  string = "" + string;
+
+  if (!POSSIBLE_CHARS.test(string)) {
+    return string;
+  }
+  return string.replace(BAD_CHARS, escapeChar);
+}

app/assets/javascripts/discourse-common/addon/lib/icon-library.js

@@ -2,6 +2,7 @@ import I18n from "I18n";
 import attributeHook from "discourse-common/lib/attribute-hook";
 import { h } from "virtual-dom";
 import { isDevelopment } from "discourse-common/config/environment";
+import escape from "discourse-common/lib/escape";
 
 const SVG_NAMESPACE = "http://www.w3.org/2000/svg";
 let _renderers = [];
@@ -140,25 +141,24 @@ registerIconRenderer({
   name: "font-awesome",
 
   string(icon, params) {
-    const id = handleIconId(icon);
+    const id = escape(handleIconId(icon));
-    let html = `<svg class='${iconClasses(icon, params)} svg-string'`;
+    let html = `<svg class='${escape(iconClasses(icon, params))} svg-string'`;
 
     if (params.label) {
       html += " aria-hidden='true'";
     }
     html += ` xmlns="${SVG_NAMESPACE}"><use xlink:href="#${id}" /></svg>`;
     if (params.label) {
-      html += `<span class='sr-only'>${params.label}</span>`;
+      html += `<span class='sr-only'>${escape(params.label)}</span>`;
     }
     if (params.title) {
-      html = `<span class="svg-icon-title" title='${I18n.t(
+      html = `<span class="svg-icon-title" title='${escape(
-        params.title
+        I18n.t(params.title)
-      ).replace(/'/g, "&#39;")}'>${html}</span>`;
+      )}'>${html}</span>`;
     }
     if (params.translatedtitle) {
-      html = `<span class="svg-icon-title" title='${params.translatedtitle.replace(
+      html = `<span class="svg-icon-title" title='${escape(
-        /'/g,
+        params.translatedtitle
-        "&#39;"
       )}'>${html}</span>`;
     }
     return html;
@@ -176,7 +176,10 @@ registerIconRenderer({
       },
       [
         h("use", {
-          "xlink:href": attributeHook("http://www.w3.org/1999/xlink", `#${id}`),
+          "xlink:href": attributeHook(
+            "http://www.w3.org/1999/xlink",
+            `#${escape(id)}`
+          ),
           namespace: SVG_NAMESPACE,
         }),
       ]

app/assets/javascripts/discourse/tests/unit/lib/icon-library-test.js

@@ -24,4 +24,16 @@ module("Unit | Utility | icon-library", function () {
     const iconC = convertIconClass("  fab fa-facebook  ");
     assert.ok(iconHTML(iconC).indexOf("  ") === -1, "trims whitespace");
   });
+
+  test("escape icon names, classes and titles", function (assert) {
+    const html = iconHTML("'<img src='x'>", {
+      translatedtitle: "'<script src='y'>",
+      label: "<iframe src='z'>",
+      class: "'<link href='w'>",
+    });
+    assert.ok(html.includes("&#x27;&lt;img src=&#x27;x&#x27;&gt;"));
+    assert.ok(html.includes("&#x27;&lt;script src=&#x27;y&#x27;&gt;"));
+    assert.ok(html.includes("&lt;iframe src=&#x27;z&#x27;&gt;"));
+    assert.ok(html.includes("&#x27;&lt;link href=&#x27;w&#x27;&gt;"));
+  });
 });

app/assets/javascripts/pretty-text/addon/sanitizer.js

@@ -1,4 +1,5 @@
 import xss from "xss";
+import escape from "discourse-common/lib/escape";
 
 function attr(name, value) {
   if (value) {
@@ -8,38 +9,7 @@ function attr(name, value) {
   return name;
 }
 
-const ESCAPE_REPLACEMENTS = {
+export { escape };
-  "&": "&",
-  "<": "&lt;",
-  ">": "&gt;",
-  '"': "&quot;",
-  "'": "&#x27;",
-  "`": "&#x60;",
-};
-const BAD_CHARS = /[&<>"'`]/g;
-const POSSIBLE_CHARS = /[&<>"'`]/;
-
-function escapeChar(chr) {
-  return ESCAPE_REPLACEMENTS[chr];
-}
-
-export function escape(string) {
-  if (string === null) {
-    return "";
-  } else if (!string) {
-    return string + "";
-  }
-
-  // Force a string conversion as this will be done by the append regardless and
-  // the regex test will do this transparently behind the scenes, causing issues if
-  // an object's to string has escaped characters in it.
-  string = "" + string;
-
-  if (!POSSIBLE_CHARS.test(string)) {
-    return string;
-  }
-  return string.replace(BAD_CHARS, escapeChar);
-}
 
 export function hrefAllowed(href, extraHrefMatchers) {
   // escape single quotes

lib/pretty_text.rb

@@ -91,6 +91,7 @@ module PrettyText
     apply_es6_file(ctx, root_path, "discourse-common/addon/lib/get-url")
     apply_es6_file(ctx, root_path, "discourse-common/addon/lib/object")
     apply_es6_file(ctx, root_path, "discourse-common/addon/lib/deprecated")
+    apply_es6_file(ctx, root_path, "discourse-common/addon/lib/escape")
     apply_es6_file(ctx, root_path, "discourse/app/lib/to-markdown")
     apply_es6_file(ctx, root_path, "discourse/app/lib/utilities")