CVE-2023-45806: Inefficient Regular Expression Complexity (CWE-1333)
value: Discourse is an open source platform for community discussion. Prior to version 3.1.3 of the `stable` branch and version 3.2.0.beta3 of the `beta` and `tests-passed` branches, if a user has been quoted and uses a `|` in their full name, they might be able to trigger a bug that generates a lot of duplicate content in all the posts they've been quoted by updating their full name again. Version 3.1.3 of the `stable` branch and version 3.2.0.beta3 of the `beta` and `tests-passed` branches contain a patch for this issue. No known workaround exists, although one can stop the "bleeding" by ensuring users only use alphanumeric characters in their full name field.
CWE description: The product uses a regular expression with an inefficient, possibly exponential worst-case computational complexity that consumes excessive CPU cycles.
CVSS v3 base score: 5.4
Published: 2023-11-10T15:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: lib/quote_rewriter.rb, spec/jobs/change_display_name_spec.rb
Commit date: 2023-11-09 13:39:06 +1100
Commit message: SECURITY: escape display names

Ensure we escape the display names before passing it to the regexp used to update
quotes whenever a user change their display name.

Diff: diff --git a/lib/quote_rewriter.rb b/lib/quote_rewriter.rb
index a316b81ca2de..5a45d580d6ba 100644
--- a/lib/quote_rewriter.rb
+++ b/lib/quote_rewriter.rb
@@ -4,21 +4,24 @@ class QuoteRewriter
   def initialize(user_id)
     @user_id = user_id
   end
 
   def rewrite_raw_username(raw, old_username, new_username)
+    escaped_old_username = Regexp.escape(old_username)
     pattern =
       Regexp.union(
-        /(?<pre>\[quote\s*=\s*["'']?.*username:)#{old_username}(?<post>\,?[^\]]*\])/i,
-        /(?<pre>\[quote\s*=\s*["'']?)#{old_username}(?<post>\,?[^\]]*\])/i,
+        /(?<pre>\[quote\s*=\s*["'']?.*username:)#{escaped_old_username}(?<post>\,?[^\]]*\])/i,
+        /(?<pre>\[quote\s*=\s*["'']?)#{escaped_old_username}(?<post>\,?[^\]]*\])/i,
       )
 
     raw.gsub(pattern, "\\k<pre>#{new_username}\\k<post>")
   end
 
   def rewrite_cooked_username(cooked, old_username, new_username, avatar_img)
-    pattern = /(?<=\s)#{PrettyText::Helpers.format_username(old_username)}(?=:)/i
+    formatted_old_username = PrettyText::Helpers.format_username(old_username)
+    escaped_old_username = Regexp.escape(formatted_old_username)
+    pattern = /(?<=\s)#{escaped_old_username}(?=:)/i
 
     cooked
       .css("aside.quote")
       .each do |aside|
         next unless div = aside.at_css("div.title")
@@ -40,17 +43,21 @@ class QuoteRewriter
         end
       end
   end
 
   def rewrite_raw_display_name(raw, old_display_name, new_display_name)
-    pattern = /(?<pre>\[quote\s*=\s*["'']?)#{old_display_name}(?<post>\,[^\]]*username[^\]]*\])/i
+    escaped_old_display_name = Regexp.escape(old_display_name)
+    pattern =
+      /(?<pre>\[quote\s*=\s*["'']?)#{escaped_old_display_name}(?<post>\,[^\]]*username[^\]]*\])/i
 
     raw.gsub(pattern, "\\k<pre>#{new_display_name}\\k<post>")
   end
 
   def rewrite_cooked_display_name(cooked, old_display_name, new_display_name)
-    pattern = /(?<=\s)#{PrettyText::Helpers.format_username(old_display_name)}(?=:)/i
+    formatted_old_display_name = PrettyText::Helpers.format_username(old_display_name)
+    escaped_old_display_name = Regexp.escape(formatted_old_display_name)
+    pattern = /(?<=\s)#{escaped_old_display_name}(?=:)/i
 
     cooked
       .css("aside.quote")
       .each do |aside|
         next unless div = aside.at_css("div.title")
diff --git a/spec/jobs/change_display_name_spec.rb b/spec/jobs/change_display_name_spec.rb
index e18db2cf3466..bb87afee4a01 100644
--- a/spec/jobs/change_display_name_spec.rb
+++ b/spec/jobs/change_display_name_spec.rb
@@ -1,49 +1,53 @@
 # frozen_string_literal: true
 
 RSpec.describe Jobs::ChangeDisplayName do
   before { stub_image_size }
 
-  let(:user) { Fabricate(:user, username: "codinghorror", name: "Jeff") }
+  let(:username) { "codinghorror" }
+  let(:old_display_name) { "|| Jeff ||" }
+  let(:new_display_name) { "|| Mr. Atwood ||" }
+
+  let(:user) { Fabricate(:user, username: username, name: old_display_name) }
   let(:topic) { Fabricate(:topic, user: user) }
   let!(:post) { create_post(post_attributes.merge(topic_id: topic.id)) }
 
   let!(:quoted_post) { create_post(user: user, topic: topic, post_number: 1, raw: "quoted post") }
   let(:avatar_url) { user.avatar_template_url.gsub("{size}", "48") }
 
   let(:post_attributes) { { raw: <<~RAW } }
-    [quote="Jeff, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
+    [quote="#{old_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
     quoted post
     [/quote]
   RAW
 
   let(:revised_post_attributes) { { raw: <<~RAW } }
-    [quote="Jeff, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
+    [quote="#{old_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
     quoted post
     [/quote]
     Forgot something.
   RAW
 
-  let(:args) { { user_id: user.id, old_name: "Jeff", new_name: "Mr. Atwood" } }
+  let(:args) { { user_id: user.id, old_name: old_display_name, new_name: new_display_name } }
 
   describe "#execute" do
     context "when the renamed user has been quoted" do
       it "rewrites the raw quote display name" do
         expect { described_class.new.execute(args) }.to change { post.reload.raw }.to(<<~RAW.strip)
-          [quote="Mr. Atwood, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
+          [quote="#{new_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
           quoted post
           [/quote]
         RAW
       end
 
       it "rewrites the cooked quote display name" do
         expect { described_class.new.execute(args) }.to change { post.reload.cooked }.to(
           match_html(<<~HTML.strip),
-          <aside class="quote no-group" data-username="codinghorror" data-post="1" data-topic="#{quoted_post.topic.id}">
+          <aside class="quote no-group" data-username="#{username}" data-post="1" data-topic="#{quoted_post.topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
-          <img loading="lazy" alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> Mr. Atwood:</div>
+          <img loading="lazy" alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> #{new_display_name}:</div>
           <blockquote>
           <p>quoted post</p>
           </blockquote>
           </aside>
         HTML
@@ -56,11 +60,11 @@ RSpec.describe Jobs::ChangeDisplayName do
 
       it "rewrites the quote in revisions" do
         expect { described_class.new.execute(args) }.to change {
           post.reload.revisions[0].modifications["raw"][0]
         }.to(<<~RAW.strip)
-          [quote="Mr. Atwood, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
+          [quote="#{new_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
           quoted post
           [/quote]
           RAW
       end
     end

Vulnerable code:         /(?<pre>\[quote\s*=\s*["'']?.*username:)#{old_username}(?<post>\,?[^\]]*\])/i,
        /(?<pre>\[quote\s*=\s*["'']?)#{old_username}(?<post>\,?[^\]]*\])/i,
    pattern = /(?<=\s)#{PrettyText::Helpers.format_username(old_username)}(?=:)/i
    pattern = /(?<pre>\[quote\s*=\s*["'']?)#{old_display_name}(?<post>\,[^\]]*username[^\]]*\])/i
    pattern = /(?<=\s)#{PrettyText::Helpers.format_username(old_display_name)}(?=:)/i
  let(:user) { Fabricate(:user, username: "codinghorror", name: "Jeff") }
    [quote="Jeff, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
    [quote="Jeff, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
  let(:args) { { user_id: user.id, old_name: "Jeff", new_name: "Mr. Atwood" } }
          [quote="Mr. Atwood, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
          <aside class="quote no-group" data-username="codinghorror" data-post="1" data-topic="#{quoted_post.topic.id}">
          <img loading="lazy" alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> Mr. Atwood:</div>
          [quote="Mr. Atwood, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"]
Fixed code:     escaped_old_username = Regexp.escape(old_username)
        /(?<pre>\[quote\s*=\s*["'']?.*username:)#{escaped_old_username}(?<post>\,?[^\]]*\])/i,
        /(?<pre>\[quote\s*=\s*["'']?)#{escaped_old_username}(?<post>\,?[^\]]*\])/i,
    formatted_old_username = PrettyText::Helpers.format_username(old_username)
    escaped_old_username = Regexp.escape(formatted_old_username)
    pattern = /(?<=\s)#{escaped_old_username}(?=:)/i
    escaped_old_display_name = Regexp.escape(old_display_name)
    pattern =
      /(?<pre>\[quote\s*=\s*["'']?)#{escaped_old_display_name}(?<post>\,[^\]]*username[^\]]*\])/i
    formatted_old_display_name = PrettyText::Helpers.format_username(old_display_name)
    escaped_old_display_name = Regexp.escape(formatted_old_display_name)
    pattern = /(?<=\s)#{escaped_old_display_name}(?=:)/i
  let(:username) { "codinghorror" }
  let(:old_display_name) { "|| Jeff ||" }
  let(:new_display_name) { "|| Mr. Atwood ||" }

  let(:user) { Fabricate(:user, username: username, name: old_display_name) }
    [quote="#{old_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
    [quote="#{old_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
  let(:args) { { user_id: user.id, old_name: old_display_name, new_name: new_display_name } }
          [quote="#{new_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
          <aside class="quote no-group" data-username="#{username}" data-post="1" data-topic="#{quoted_post.topic.id}">
          <img loading="lazy" alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> #{new_display_name}:</div>
          [quote="#{new_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]