SECURITY: XSS on chat excerpts

Non-markdown tags weren't being escaped in chat excerpts. This could be
triggered by editing a chat message containing a tag (self XSS), or by
replying to a chat message with a tag (XSS).

Co-authored-by: Jan Cernik <jancernik12@gmail.com>

app/assets/javascripts/discourse/app/components/choose-topic.hbs

@@ -30,7 +30,7 @@
             />
             <TopicStatus @topic={{t}} @disableActions={{true}} />
             <span class="topic-title">
-              {{replace-emoji t.fancy_title}}
+              {{replace-emoji t.title}}
             </span>
             <span class="topic-categories">
               {{bound-category-link

app/assets/javascripts/discourse/app/helpers/replace-emoji.js

@@ -1,7 +1,9 @@
 import { emojiUnescape } from "discourse/lib/text";
-import { htmlSafe } from "@ember/template";
+import { htmlSafe, isHTMLSafe } from "@ember/template";
 import { registerUnbound } from "discourse-common/lib/helpers";
+import { escapeExpression } from "discourse/lib/utilities";
 
 registerUnbound("replace-emoji", (text, options) => {
+  text = isHTMLSafe(text) ? text.toString() : escapeExpression(text);
   return htmlSafe(emojiUnescape(text, options));
 });

app/assets/javascripts/discourse/app/templates/modal/insert-hyperlink.hbs

@@ -21,7 +21,7 @@
               data-title={{result.fancy_title}}
             >
               <TopicStatus @topic={{result}} @disableActions={{true}} />
-              {{replace-emoji result.fancy_title}}
+              {{replace-emoji result.title}}
               <div class="search-category">
                 {{#if result.category.parentCategory}}
                   {{category-link result.category.parentCategory}}

app/assets/javascripts/discourse/app/templates/review-topics.hbs

@@ -16,7 +16,7 @@
                 href={{rt.relative_url}}
                 rel="noopener noreferrer"
                 target="_blank"
-              >{{replace-emoji rt.fancy_title}}</a>
+              >{{replace-emoji rt.title}}</a>
             </div>
           </td>
           <td class="reviewable-count">

app/assets/javascripts/discourse/tests/integration/helpers/replace-emoji-test.js

@@ -0,0 +1,29 @@
+import { module, test } from "qunit";
+import { setupRenderingTest } from "discourse/tests/helpers/component-test";
+import { render } from "@ember/test-helpers";
+import { hbs } from "ember-cli-htmlbars";
+
+module("Integration | Helper | replace-emoji", function (hooks) {
+  setupRenderingTest(hooks);
+
+  test("it replaces the emoji", async function (assert) {
+    await render(hbs`<span>{{replace-emoji "some text :heart:"}}</span>`);
+
+    assert.dom(`span`).includesText("some text");
+    assert.dom(`.emoji[title="heart"]`).exists();
+  });
+
+  test("it escapes the text", async function (assert) {
+    await render(
+      hbs`<span>{{replace-emoji "<style>body: {background: red;}</style>"}}</span>`
+    );
+
+    assert.dom(`span`).hasText("<style>body: {background: red;}</style>");
+  });
+
+  test("it renders html-safe text", async function (assert) {
+    await render(hbs`<span>{{replace-emoji (html-safe "safe text")}}</span>`);
+
+    assert.dom(`span`).hasText("safe text");
+  });
+});

app/assets/javascripts/select-kit/addon/templates/components/topic-row.hbs

@@ -1,5 +1,5 @@
 <TopicStatus @topic={{this.item}} @disableActions={{true}} />
-<div class="topic-title">{{replace-emoji this.item.fancy_title}}</div>
+<div class="topic-title">{{replace-emoji this.item.title}}</div>
 <div class="topic-categories">
   {{bound-category-link
     this.item.category

plugins/chat/assets/javascripts/discourse/components/chat-channel-about-view.hbs

@@ -31,7 +31,7 @@
     </label>
     <div class="chat-form__control">
       <div class="channel-info-about-view__name">
-        {{replace-emoji this.channel.escapedTitle}}
+        {{replace-emoji this.channel.title}}
       </div>
       <div class="channel-info-about-view__slug">
         {{this.channel.slug}}

plugins/chat/assets/javascripts/discourse/components/chat-channel-card.hbs

@@ -15,7 +15,7 @@
         class="chat-channel-card__name-container"
       >
         <span class="chat-channel-card__name">
-          {{replace-emoji @channel.escapedTitle}}
+          {{replace-emoji @channel.title}}
         </span>
         {{#if @channel.chatable.read_restricted}}
           {{d-icon "lock" class="chat-channel-card__read-restricted"}}
@@ -47,7 +47,7 @@
 
     {{#if @channel.description}}
       <div class="chat-channel-card__description">
-        {{replace-emoji @channel.escapedDescription}}
+        {{replace-emoji @channel.description}}
       </div>
     {{/if}}
 

plugins/chat/assets/javascripts/discourse/components/chat-channel-title.hbs

@@ -59,7 +59,7 @@
         {{/if}}
       </span>
       <span class="chat-channel-title__name">
-        {{replace-emoji this.channel.escapedTitle}}
+        {{replace-emoji this.channel.title}}
       </span>
 
       {{#if (has-block)}}

plugins/chat/spec/system/chat_channel_spec.rb

@@ -189,6 +189,34 @@ RSpec.describe "Chat channel", type: :system, js: true do
     end
   end
 
+  context "when replying to message that has tags" do
+    fab!(:other_user) { Fabricate(:user) }
+    fab!(:message_2) do
+      Fabricate(
+        :chat_message,
+        user: other_user,
+        chat_channel: channel_1,
+        message: "<mark>not marked</mark>",
+      )
+    end
+
+    before do
+      Fabricate(:chat_message, user: other_user, chat_channel: channel_1)
+      Fabricate(:chat_message, in_reply_to: message_2, user: current_user, chat_channel: channel_1)
+      channel_1.add(other_user)
+      channel_1.add(current_user)
+      sign_in(current_user)
+    end
+
+    it "escapes the reply-to line" do
+      chat.visit_channel(channel_1)
+
+      expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq(
+        "&lt;mark&gt;not marked&lt;/mark&gt;",
+      )
+    end
+  end
+
   context "when messages are separated by a day" do
     before do
       Fabricate(:chat_message, chat_channel: channel_1, created_at: 2.days.ago)