CVE-2022-41921: Allocation of Resources Without Limits or Throttling (CWE-770)
value: Discourse is an open-source discussion platform. Prior to version 2.9.0.beta13, users can post chat messages of an unlimited length, which can cause a denial of service for other users when posting huge amounts of text. Users should upgrade to version 2.9.0.beta13, where a limit has been introduced. No known workarounds are available.
CWE description: The product allocates a reusable resource or group of resources on behalf of an actor without imposing any restrictions on the size or number of resources that can be allocated, in violation of the intended security policy for that actor.
CVSS v3 base score: 4.3
Published: 2022-11-28T15:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb, plugins/chat/app/models/chat_message.rb, plugins/chat/config/locales/server.en.yml, plugins/chat/config/settings.yml, plugins/chat/db/post_migrate/20221117052348_truncate_chat_messages_over_max_length.rb, plugins/chat/spec/components/chat_message_creator_spec.rb, plugins/chat/spec/components/chat_message_updater_spec.rb, plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb
Commit date: 2022-11-28 10:48:30 +1000
Commit message: SECURITY: Limit chat message char length (#19207)

Only allow maximum of 6000 characters for chat messages when they
are created or edited. A hidden setting can control this limit,
6000 is the default.

There is also a migration here to truncate any existing messages to
6000 characters if the message is already over that and if the
chat_messages table exists. We also set cooked_version to NULL
for those messages so we can identify them for rebake.
Diff: diff --git a/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb b/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb
index 1cf4963621ef..f528e59513aa 100644
--- a/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb
+++ b/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb
@@ -1,9 +1,8 @@
 # frozen_string_literal: true
 
 class Chat::IncomingChatWebhooksController < ApplicationController
-  WEBHOOK_MAX_MESSAGE_LENGTH = 2000
   WEBHOOK_MESSAGES_PER_MINUTE_LIMIT = 10
 
   skip_before_action :verify_authenticity_token, :redirect_to_login_if_required
 
   before_action :validate_payload
@@ -78,13 +77,13 @@ class Chat::IncomingChatWebhooksController < ApplicationController
     ).performed!
     webhook
   end
 
   def validate_message_length(message)
-    return if message.length <= WEBHOOK_MAX_MESSAGE_LENGTH
+    return if message.length <= SiteSetting.chat_maximum_message_length
     raise Discourse::InvalidParameters.new(
-            "Body cannot be over #{WEBHOOK_MAX_MESSAGE_LENGTH} characters",
+            "Body cannot be over #{SiteSetting.chat_maximum_message_length} characters",
           )
   end
 
   def validate_payload
     params.require([:key])
diff --git a/plugins/chat/app/models/chat_message.rb b/plugins/chat/app/models/chat_message.rb
index 2a73b0f1791e..4c1f70832878 100644
--- a/plugins/chat/app/models/chat_message.rb
+++ b/plugins/chat/app/models/chat_message.rb
@@ -46,10 +46,20 @@ class ChatMessage < ActiveRecord::Base
           "chat.errors.minimum_length_not_met",
           minimum: SiteSetting.chat_minimum_message_length,
         ),
       )
     end
+
+    if message_too_long?
+      self.errors.add(
+        :base,
+        I18n.t(
+          "chat.errors.message_too_long",
+          maximum: SiteSetting.chat_maximum_message_length,
+        ),
+      )
+    end
   end
 
   def attach_uploads(uploads)
     return if uploads.blank?
 
@@ -203,10 +213,14 @@ class ChatMessage < ActiveRecord::Base
 
   def message_too_short?
     message.length < SiteSetting.chat_minimum_message_length
   end
 
+  def message_too_long?
+    message.length > SiteSetting.chat_maximum_message_length
+  end
+
   def ensure_last_editor_id
     self.last_editor_id ||= self.user_id
   end
 end
 
diff --git a/plugins/chat/config/locales/server.en.yml b/plugins/chat/config/locales/server.en.yml
index 66971e7d5a32..0a9838ce72fa 100644
--- a/plugins/chat/config/locales/server.en.yml
+++ b/plugins/chat/config/locales/server.en.yml
@@ -46,10 +46,11 @@ en:
       auto_silence_from_flags: "Chat message flagged with score high enough to silence user."
       channel_cannot_be_archived: "The channel cannot be archived at this time, it must be either closed or open to archive."
       duplicate_message: "You posted an identical message too recently."
       delete_channel_failed: "Delete channel failed, please try again."
       minimum_length_not_met: "Message is too short, must have a minimum of %{minimum} characters."
+      message_too_long: "Message is too long, messages must be a maximum of %{maximum} characters."
       max_reactions_limit_reached: "New reactions are not allowed on this message."
       message_move_invalid_channel: "The source and destination channel must be public channels."
       message_move_no_messages_found: "No messages were found with the provided message IDs."
       cant_update_direct_message_channel: "Direct message channel properties like name and description can’t be updated."
       not_accepting_dms: "Sorry, %{username} is not accepting messages at the moment."
diff --git a/plugins/chat/config/settings.yml b/plugins/chat/config/settings.yml
index bcf2177bc2b2..be2d86cf9f60 100644
--- a/plugins/chat/config/settings.yml
+++ b/plugins/chat/config/settings.yml
@@ -73,10 +73,17 @@ chat:
     type: integer
     default: 1
     min: 1
     max: 50
     client: true
+  chat_maximum_message_length:
+    type: integer
+    default: 6000
+    client: true
+    hidden: true
+    min: 100
+    max: 12000
   chat_allow_uploads:
     default: true
     client: true
     validator: "ChatAllowUploadsValidator"
   max_chat_auto_joined_users:
diff --git a/plugins/chat/db/post_migrate/20221117052348_truncate_chat_messages_over_max_length.rb b/plugins/chat/db/post_migrate/20221117052348_truncate_chat_messages_over_max_length.rb
new file mode 100644
index 000000000000..60a0d80961ee
--- /dev/null
+++ b/plugins/chat/db/post_migrate/20221117052348_truncate_chat_messages_over_max_length.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class TruncateChatMessagesOverMaxLength < ActiveRecord::Migration[7.0]
+  def up
+    if table_exists?(:chat_messages)
+      # 6000 is the default of the chat_maximum_message_length
+      # site setting, its safe to do this because this will be
+      # run the first time the setting is introduced.
+      execute <<~SQL
+        UPDATE chat_messages
+        SET message = LEFT(message, 6000), cooked_version = NULL
+        WHERE LENGTH(message) > 6000
+      SQL
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/plugins/chat/spec/components/chat_message_creator_spec.rb b/plugins/chat/spec/components/chat_message_creator_spec.rb
index bebdd3029666..0520ce31d8b3 100644
--- a/plugins/chat/spec/components/chat_message_creator_spec.rb
+++ b/plugins/chat/spec/components/chat_message_creator_spec.rb
@@ -70,10 +70,27 @@ describe Chat::ChatMessageCreator do
           { minimum: SiteSetting.chat_minimum_message_length },
         ),
       )
     end
 
+    it "errors when length is greater than `chat_maximum_message_length`" do
+      SiteSetting.chat_maximum_message_length = 100
+      creator =
+        Chat::ChatMessageCreator.create(
+          chat_channel: public_chat_channel,
+          user: user1,
+          content: "a really long and in depth message that is just too detailed" * 100,
+        )
+      expect(creator.failed?).to eq(true)
+      expect(creator.error.message).to match(
+        I18n.t(
+          "chat.errors.message_too_long",
+          { maximum: SiteSetting.chat_maximum_message_length },
+        ),
+      )
+    end
+
     it "allows message creation when length is less than `chat_minimum_message_length` when upload is present" do
       upload = Fabricate(:upload, user: user1)
       SiteSetting.chat_minimum_message_length = 10
       expect {
         Chat::ChatMessageCreator.create(
diff --git a/plugins/chat/spec/components/chat_message_updater_spec.rb b/plugins/chat/spec/components/chat_message_updater_spec.rb
index d31e4bae8e15..d53bf0d71428 100644
--- a/plugins/chat/spec/components/chat_message_updater_spec.rb
+++ b/plugins/chat/spec/components/chat_message_updater_spec.rb
@@ -66,19 +66,42 @@ describe Chat::ChatMessageUpdater do
       ),
     )
     expect(chat_message.reload.message).to eq(og_message)
   end
 
+  it "errors when length is greater than `chat_maximum_message_length`" do
+    SiteSetting.chat_maximum_message_length = 100
+    og_message = "This won't be changed!"
+    chat_message = create_chat_message(user1, og_message, public_chat_channel)
+    new_message = "2 long" * 100
+
+    updater =
+      Chat::ChatMessageUpdater.update(
+        guardian: guardian,
+        chat_message: chat_message,
+        new_content: new_message,
+      )
+    expect(updater.failed?).to eq(true)
+    expect(updater.error.message).to match(
+      I18n.t(
+        "chat.errors.message_too_long",
+        { maximum: SiteSetting.chat_maximum_message_length },
+      ),
+    )
+    expect(chat_message.reload.message).to eq(og_message)
+  end
+
   it "errors if a user other than the message user is trying to edit the message" do
     og_message = "This won't be changed!"
     chat_message = create_chat_message(user1, og_message, public_chat_channel)
     new_message = "2 short"
-    updater = Chat::ChatMessageUpdater.update(
-      guardian: Guardian.new(Fabricate(:user)),
-      chat_message: chat_message,
-      new_content: new_message,
-    )
+    updater =
+      Chat::ChatMessageUpdater.update(
+        guardian: Guardian.new(Fabricate(:user)),
+        chat_message: chat_message,
+        new_content: new_message,
+      )
     expect(updater.failed?).to eq(true)
     expect(updater.error).to match(Discourse::InvalidAccess)
   end
 
   it "it updates a messages content" do
@@ -93,17 +116,18 @@ describe Chat::ChatMessageUpdater do
     expect(chat_message.reload.message).to eq(new_message)
   end
 
   it "publishes a DiscourseEvent for updated messages" do
     chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
-    events = DiscourseEvent.track_events {
-      Chat::ChatMessageUpdater.update(
-        guardian: guardian,
-        chat_message: chat_message,
-        new_content: "Change to this!",
-      )
-    }
+    events =
+      DiscourseEvent.track_events do
+        Chat::ChatMessageUpdater.update(
+          guardian: guardian,
+          chat_message: chat_message,
+          new_content: "Change to this!",
+        )
+      end
     expect(events.map { _1[:event_name] }).to include(:chat_message_edited)
   end
 
   it "creates mention notifications for unmentioned users" do
     chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
diff --git a/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb b/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb
index 448230c2b836..a8a54334718a 100644
--- a/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb
+++ b/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb
@@ -17,14 +17,14 @@ RSpec.describe Chat::IncomingChatWebhooksController do
     it "errors when no body is present" do
       post "/chat/hooks/#{webhook.key}.json"
       expect(response.status).to eq(400)
     end
 
-    it "errors when the body is over WEBHOOK_MAX_MESSAGE_LENGTH characters" do
+    it "errors when the body is over chat_maximum_message_length characters" do
       post "/chat/hooks/#{webhook.key}.json",
            params: {
-             text: "$" * (Chat::IncomingChatWebhooksController::WEBHOOK_MAX_MESSAGE_LENGTH + 1),
+             text: "$" * (SiteSetting.chat_maximum_message_length + 1),
            }
       expect(response.status).to eq(400)
     end
 
     it "creates a new chat message" do

Vulnerable code:   WEBHOOK_MAX_MESSAGE_LENGTH = 2000
    return if message.length <= WEBHOOK_MAX_MESSAGE_LENGTH
            "Body cannot be over #{WEBHOOK_MAX_MESSAGE_LENGTH} characters",
    updater = Chat::ChatMessageUpdater.update(
      guardian: Guardian.new(Fabricate(:user)),
      chat_message: chat_message,
      new_content: new_message,
    )
    events = DiscourseEvent.track_events {
      Chat::ChatMessageUpdater.update(
        guardian: guardian,
        chat_message: chat_message,
        new_content: "Change to this!",
      )
    }
    it "errors when the body is over WEBHOOK_MAX_MESSAGE_LENGTH characters" do
             text: "$" * (Chat::IncomingChatWebhooksController::WEBHOOK_MAX_MESSAGE_LENGTH + 1),
Fixed code:     return if message.length <= SiteSetting.chat_maximum_message_length
            "Body cannot be over #{SiteSetting.chat_maximum_message_length} characters",

    if message_too_long?
      self.errors.add(
        :base,
        I18n.t(
          "chat.errors.message_too_long",
          maximum: SiteSetting.chat_maximum_message_length,
        ),
      )
    end
  def message_too_long?
    message.length > SiteSetting.chat_maximum_message_length
  end

      message_too_long: "Message is too long, messages must be a maximum of %{maximum} characters."
  chat_maximum_message_length:
    type: integer
    default: 6000
    client: true
    hidden: true
    min: 100
    max: 12000
# frozen_string_literal: true

class TruncateChatMessagesOverMaxLength < ActiveRecord::Migration[7.0]
  def up
    if table_exists?(:chat_messages)
      # 6000 is the default of the chat_maximum_message_length
      # site setting, its safe to do this because this will be
      # run the first time the setting is introduced.
      execute <<~SQL
        UPDATE chat_messages
        SET message = LEFT(message, 6000), cooked_version = NULL
        WHERE LENGTH(message) > 6000
      SQL
    end
  end

  def down
    raise ActiveRecord::IrreversibleMigration
  end
end
    it "errors when length is greater than `chat_maximum_message_length`" do
      SiteSetting.chat_maximum_message_length = 100
      creator =
        Chat::ChatMessageCreator.create(
          chat_channel: public_chat_channel,
          user: user1,
          content: "a really long and in depth message that is just too detailed" * 100,
        )
      expect(creator.failed?).to eq(true)
      expect(creator.error.message).to match(
        I18n.t(
          "chat.errors.message_too_long",
          { maximum: SiteSetting.chat_maximum_message_length },
        ),
      )
    end

  it "errors when length is greater than `chat_maximum_message_length`" do
    SiteSetting.chat_maximum_message_length = 100
    og_message = "This won't be changed!"
    chat_message = create_chat_message(user1, og_message, public_chat_channel)
    new_message = "2 long" * 100

    updater =
      Chat::ChatMessageUpdater.update(
        guardian: guardian,
        chat_message: chat_message,
        new_content: new_message,
      )
    expect(updater.failed?).to eq(true)
    expect(updater.error.message).to match(
      I18n.t(
        "chat.errors.message_too_long",
        { maximum: SiteSetting.chat_maximum_message_length },
      ),
    )
    expect(chat_message.reload.message).to eq(og_message)
  end

    updater =
      Chat::ChatMessageUpdater.update(
        guardian: Guardian.new(Fabricate(:user)),
        chat_message: chat_message,
        new_content: new_message,
      )
    events =
      DiscourseEvent.track_events do
        Chat::ChatMessageUpdater.update(
          guardian: guardian,
          chat_message: chat_message,
          new_content: "Change to this!",
        )
      end
    it "errors when the body is over chat_maximum_message_length characters" do
             text: "$" * (SiteSetting.chat_maximum_message_length + 1),