From d56b2e85aacb9744649562c5b8a8cba22f186c6c Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Wed, 17 Mar 2021 16:11:40 +0300
Subject: [PATCH] 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.
---
 .../discourse-common/addon/lib/escape.js      | 32 +++++++++++++++++
 .../addon/lib/icon-library.js                 | 23 +++++++------
 .../tests/unit/lib/icon-library-test.js       | 12 +++++++
 .../pretty-text/addon/sanitizer.js            | 34 ++-----------------
 lib/pretty_text.rb                            |  1 +
 5 files changed, 60 insertions(+), 42 deletions(-)
 create mode 100644 app/assets/javascripts/discourse-common/addon/lib/escape.js

diff --git a/app/assets/javascripts/discourse-common/addon/lib/escape.js b/app/assets/javascripts/discourse-common/addon/lib/escape.js
new file mode 100644
index 00000000000..6b7da6a67f0
--- /dev/null
+++ b/app/assets/javascripts/discourse-common/addon/lib/escape.js
@@ -0,0 +1,32 @@
+const ESCAPE_REPLACEMENTS = {
+  "&": "&amp;",
+  "<": "&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);
+}
diff --git a/app/assets/javascripts/discourse-common/addon/lib/icon-library.js b/app/assets/javascripts/discourse-common/addon/lib/icon-library.js
index 5f26ea7708a..8a01d633f02 100644
--- a/app/assets/javascripts/discourse-common/addon/lib/icon-library.js
+++ b/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);
-    let html = `<svg class='${iconClasses(icon, params)} svg-string'`;
+    const id = escape(handleIconId(icon));
+    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(
-        params.title
-      ).replace(/'/g, "&#39;")}'>${html}</span>`;
+      html = `<span class="svg-icon-title" title='${escape(
+        I18n.t(params.title)
+      )}'>${html}</span>`;
     }
     if (params.translatedtitle) {
-      html = `<span class="svg-icon-title" title='${params.translatedtitle.replace(
-        /'/g,
-        "&#39;"
+      html = `<span class="svg-icon-title" title='${escape(
+        params.translatedtitle
       )}'>${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,
         }),
       ]
diff --git a/app/assets/javascripts/discourse/tests/unit/lib/icon-library-test.js b/app/assets/javascripts/discourse/tests/unit/lib/icon-library-test.js
index 3089ebec8fa..de03fa9c142 100644
--- a/app/assets/javascripts/discourse/tests/unit/lib/icon-library-test.js
+++ b/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;"));
+  });
 });
diff --git a/app/assets/javascripts/pretty-text/addon/sanitizer.js b/app/assets/javascripts/pretty-text/addon/sanitizer.js
index f29c37b80f5..66bdcbf4ce2 100644
--- a/app/assets/javascripts/pretty-text/addon/sanitizer.js
+++ b/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 = {
-  "&": "&amp;",
-  "<": "&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 { escape };
 
 export function hrefAllowed(href, extraHrefMatchers) {
   // escape single quotes
diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb
index 4434190e6c7..173b0fe2eaa 100644
--- a/lib/pretty_text.rb
+++ b/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")