From 3e0cc4a5d9ef44ad902f6985d046ebb32f0a14ee Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Fri, 20 Jan 2023 15:00:46 +0800 Subject: [PATCH] SECURITY: Limit the character count of group membership requests 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. --- .../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 dbd26377b8..3d171b6f67 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"}} - + diff --git a/app/models/group_request.rb b/app/models/group_request.rb index 706fcceab9..c923622aac 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 0000000000..3dd9a660a3 --- /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 0e108963ae..5045d59bbc 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -2040,6 +2040,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)