CVE-2023-26040: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Discourse is an open-source discussion platform. Between versions 3.1.0.beta2 and 3.1.0.beta3 of the `tests-passed` branch, editing or responding to a chat message containing malicious content could lead to a cross-site scripting attack. This issue is patched in version 3.1.0.beta3 of the `tests-passed` branch. There are no known workarounds.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2023-03-17T15:15Z
Repository: https://github.com/discourse/discourse
Language: Other
Files changed: app/assets/javascripts/discourse/app/components/choose-topic.hbs, app/assets/javascripts/discourse/app/helpers/replace-emoji.js, app/assets/javascripts/discourse/app/templates/modal/insert-hyperlink.hbs, app/assets/javascripts/discourse/app/templates/review-topics.hbs, app/assets/javascripts/discourse/tests/integration/helpers/replace-emoji-test.js, app/assets/javascripts/select-kit/addon/templates/components/topic-row.hbs, plugins/chat/assets/javascripts/discourse/components/chat-channel-about-view.hbs, plugins/chat/assets/javascripts/discourse/components/chat-channel-card.hbs, plugins/chat/assets/javascripts/discourse/components/chat-channel-title.hbs, plugins/chat/spec/system/chat_channel_spec.rb
Commit date: 2023-03-16 15:27:09 -0600
Commit message: 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>

Diff: diff --git a/app/assets/javascripts/discourse/app/components/choose-topic.hbs b/app/assets/javascripts/discourse/app/components/choose-topic.hbs
index f9bf06d223af..15e428503967 100644
--- a/app/assets/javascripts/discourse/app/components/choose-topic.hbs
+++ b/app/assets/javascripts/discourse/app/components/choose-topic.hbs
@@ -28,11 +28,11 @@
               name="choose_topic_id"
               {{on "click" (action "chooseTopic" t)}}
             />
             <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
                 t.category
                 recursive=true
diff --git a/app/assets/javascripts/discourse/app/helpers/replace-emoji.js b/app/assets/javascripts/discourse/app/helpers/replace-emoji.js
index 3109cf290036..81d78a3a1f4b 100644
--- a/app/assets/javascripts/discourse/app/helpers/replace-emoji.js
+++ b/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));
 });
diff --git a/app/assets/javascripts/discourse/app/templates/modal/insert-hyperlink.hbs b/app/assets/javascripts/discourse/app/templates/modal/insert-hyperlink.hbs
index affbf5d1b6ed..a69187ed8fbe 100644
--- a/app/assets/javascripts/discourse/app/templates/modal/insert-hyperlink.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal/insert-hyperlink.hbs
@@ -19,11 +19,11 @@
               href={{result.url}}
               onclick={{action "linkClick"}}
               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}}
                 {{/if}}
                 {{category-link result.category hideParent=true}}
diff --git a/app/assets/javascripts/discourse/app/templates/review-topics.hbs b/app/assets/javascripts/discourse/app/templates/review-topics.hbs
index 286e391e0a71..5bed189751f9 100644
--- a/app/assets/javascripts/discourse/app/templates/review-topics.hbs
+++ b/app/assets/javascripts/discourse/app/templates/review-topics.hbs
@@ -14,11 +14,11 @@
               <TopicStatus @topic={{rt}} />
               <a
                 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">
             {{rt.stats.count}}
           </td>
diff --git a/app/assets/javascripts/discourse/tests/integration/helpers/replace-emoji-test.js b/app/assets/javascripts/discourse/tests/integration/helpers/replace-emoji-test.js
new file mode 100644
index 000000000000..2ce8e73be3c7
--- /dev/null
+++ b/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");
+  });
+});
diff --git a/app/assets/javascripts/select-kit/addon/templates/components/topic-row.hbs b/app/assets/javascripts/select-kit/addon/templates/components/topic-row.hbs
index d9b2074909b4..fc832e4c6403 100644
--- a/app/assets/javascripts/select-kit/addon/templates/components/topic-row.hbs
+++ b/app/assets/javascripts/select-kit/addon/templates/components/topic-row.hbs
@@ -1,7 +1,7 @@
 <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
     recursive=true
     hideParent=true
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-about-view.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-channel-about-view.hbs
index 77f999c9b697..29b3fa73c0ab 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-about-view.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-about-view.hbs
@@ -29,11 +29,11 @@
         </div>
       {{/if}}
     </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}}
       </div>
     </div>
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-card.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-channel-card.hbs
index a7c4b39bb4f4..9b4d077b090e 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-card.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-card.hbs
@@ -13,11 +13,11 @@
         @route="chat.channel"
         @models={{@channel.routeModels}}
         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"}}
         {{/if}}
       </LinkTo>
@@ -45,11 +45,11 @@
       </div>
     </div>
 
     {{#if @channel.description}}
       <div class="chat-channel-card__description">
-        {{replace-emoji @channel.escapedDescription}}
+        {{replace-emoji @channel.description}}
       </div>
     {{/if}}
 
     <div class="chat-channel-card__cta">
       {{#if @channel.isFollowing}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-title.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-channel-title.hbs
index 5fb7fde9883d..b25ceb099fd9 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-title.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-title.hbs
@@ -57,11 +57,11 @@
         {{#if this.channel.chatable.read_restricted}}
           {{d-icon "lock" class="chat-channel-title__restricted-category-icon"}}
         {{/if}}
       </span>
       <span class="chat-channel-title__name">
-        {{replace-emoji this.channel.escapedTitle}}
+        {{replace-emoji this.channel.title}}
       </span>
 
       {{#if (has-block)}}
         {{yield}}
       {{/if}}
diff --git a/plugins/chat/spec/system/chat_channel_spec.rb b/plugins/chat/spec/system/chat_channel_spec.rb
index 455a5c17acec..b934db3ad81a 100644
--- a/plugins/chat/spec/system/chat_channel_spec.rb
+++ b/plugins/chat/spec/system/chat_channel_spec.rb
@@ -187,10 +187,38 @@ RSpec.describe "Chat channel", type: :system, js: true do
 
       expect(page).to have_selector(".chat-reply__excerpt")
     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)
 
       channel_1.add(current_user)

Vulnerable code:               {{replace-emoji t.fancy_title}}
import { htmlSafe } from "@ember/template";
              {{replace-emoji result.fancy_title}}
              >{{replace-emoji rt.fancy_title}}</a>
<div class="topic-title">{{replace-emoji this.item.fancy_title}}</div>
        {{replace-emoji this.channel.escapedTitle}}
          {{replace-emoji @channel.escapedTitle}}
        {{replace-emoji @channel.escapedDescription}}
        {{replace-emoji this.channel.escapedTitle}}
Fixed code:               {{replace-emoji t.title}}
import { htmlSafe, isHTMLSafe } from "@ember/template";
import { escapeExpression } from "discourse/lib/utilities";
  text = isHTMLSafe(text) ? text.toString() : escapeExpression(text);
              {{replace-emoji result.title}}
              >{{replace-emoji rt.title}}</a>
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");
  });
});
<div class="topic-title">{{replace-emoji this.item.title}}</div>
        {{replace-emoji this.channel.title}}
          {{replace-emoji @channel.title}}
        {{replace-emoji @channel.description}}
        {{replace-emoji this.channel.title}}
  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