Skip to content
Permalink
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Drenmi authored and nbianca committed Jan 25, 2023
1 parent 3dcd0bc commit 3e0cc4a
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 @@ -2040,6 +2040,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 3e0cc4a

Please sign in to comment.