From d5745d34c20c31a221039d8913f33064433003ea Mon Sep 17 00:00:00 2001
From: Natalie Tay <natalie.tay@discourse.org>
Date: Wed, 25 Jan 2023 19:50:33 +0800
Subject: [PATCH] SECURITY: Limit the character count of group membership
 requests (#19993)

When creating a group membership request, there is no character
limit on the 'reason' field. This can be potentially be used by
an attacker to create enormous amount of data in the database.

Co-authored-by: Ted Johansson <ted@discourse.org>
---
 .../modal/request-group-membership-form.hbs        |  2 +-
 app/models/group_request.rb                        |  4 ++++
 spec/models/group_request_spec.rb                  | 10 ++++++++++
 spec/requests/groups_controller_spec.rb            | 14 ++++++++++++++
 4 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 spec/models/group_request_spec.rb

diff --git a/app/assets/javascripts/discourse/app/templates/modal/request-group-membership-form.hbs b/app/assets/javascripts/discourse/app/templates/modal/request-group-membership-form.hbs
index dbd26377b8f..3d171b6f678 100644
--- a/app/assets/javascripts/discourse/app/templates/modal/request-group-membership-form.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal/request-group-membership-form.hbs
@@ -5,7 +5,7 @@
         {{i18n "groups.membership_request.reason"}}
       </label>
 
-      <ExpandingTextArea @value={{this.reason}} />
+      <ExpandingTextArea @value={{this.reason}} @maxlength="280" />
     </div>
   </DModalBody>
 
diff --git a/app/models/group_request.rb b/app/models/group_request.rb
index 706fcceab97..c923622aac3 100644
--- a/app/models/group_request.rb
+++ b/app/models/group_request.rb
@@ -1,8 +1,12 @@
 # frozen_string_literal: true
 
 class GroupRequest < ActiveRecord::Base
+  REASON_CHARACTER_LIMIT = 280
+
   belongs_to :group
   belongs_to :user
+
+  validates :reason, length: { maximum: REASON_CHARACTER_LIMIT }
 end
 
 # == Schema Information
diff --git a/spec/models/group_request_spec.rb b/spec/models/group_request_spec.rb
new file mode 100644
index 00000000000..3dd9a660a31
--- /dev/null
+++ b/spec/models/group_request_spec.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+RSpec.describe GroupRequest do
+  it { is_expected.to belong_to :user }
+  it { is_expected.to belong_to :group }
+
+  it do
+    is_expected.to validate_length_of(:reason).is_at_most(described_class::REASON_CHARACTER_LIMIT)
+  end
+end
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index 6b432c1b2a7..87c7ad0628e 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@@ -2198,6 +2198,20 @@ RSpec.describe GroupsController do
       expect(response.status).to eq(409)
     end
 
+    it "limits the character count of the reason" do
+      sign_in(user)
+
+      post "/groups/#{group.name}/request_membership.json",
+           params: {
+             reason: "x" * (GroupRequest::REASON_CHARACTER_LIMIT + 1),
+           }
+
+      expect(response.status).to eq(422)
+      expect(response.parsed_body["errors"]).to contain_exactly(
+        "Reason is too long (maximum is 280 characters)",
+      )
+    end
+
     it "should create the right PM" do
       owner1 = Fabricate(:user, last_seen_at: Time.zone.now)
       owner2 = Fabricate(:user, last_seen_at: Time.zone.now - 1.day)