Skip to content
Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
nattsw and Drenmi committed Jan 25, 2023
1 parent f91ac52 commit d5745d3
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 1 deletion.
Expand Up @@ -5,7 +5,7 @@
{{i18n "groups.membership_request.reason"}}
</label>

<ExpandingTextArea @value={{this.reason}} />
<ExpandingTextArea @value={{this.reason}} @maxlength="280" />
</div>
</DModalBody>

Expand Down
4 changes: 4 additions & 0 deletions 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
Expand Down
10 changes: 10 additions & 0 deletions 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
14 changes: 14 additions & 0 deletions spec/requests/groups_controller_spec.rb
Expand Up @@ -2198,6 +2198,20 @@ def expect_type_to_return_right_groups(type, expected_group_ids)
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)
Expand Down

0 comments on commit d5745d3

Please sign in to comment.