SECURITY: Limit user profile field length (#18302)

Adds limits to location and website fields at model and DB level
to match the bio_raw field limits. A limit cannot be added at the
DB level for bio_raw because it is a postgres text field.

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This commit is contained in:
Martin Brennan 2022-09-21 12:07:06 +10:00 committed by GitHub
parent b98cd73ace
commit e69f7d2fd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 4 deletions

View File

@ -13,9 +13,9 @@ class UserProfile < ActiveRecord::Base
has_many :user_profile_views, dependent: :destroy has_many :user_profile_views, dependent: :destroy
validates :bio_raw, length: { maximum: 3000 }, watched_words: true validates :bio_raw, length: { maximum: 3000 }, watched_words: true
validates :website, url: true, allow_blank: true, if: :validate_website? validates :website, url: true, length: { maximum: 3000 }, allow_blank: true, if: :validate_website?
validates :location, length: { maximum: 3000 }, watched_words: true
validates :user, presence: true validates :user, presence: true
validates :location, watched_words: true
validate :website_domain_validator, if: :validate_website? validate :website_domain_validator, if: :validate_website?
@ -188,8 +188,8 @@ end
# Table name: user_profiles # Table name: user_profiles
# #
# user_id :integer not null, primary key # user_id :integer not null, primary key
# location :string # location :string(3000)
# website :string # website :string(3000)
# bio_raw :text # bio_raw :text
# bio_cooked :text # bio_cooked :text
# dismissed_banner_key :integer # dismissed_banner_key :integer

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class EnforceUserProfileMaxLimits < ActiveRecord::Migration[7.0]
def change
execute "UPDATE user_profiles SET location = LEFT(location, 3000) WHERE location IS NOT NULL"
execute "UPDATE user_profiles SET website = LEFT(website, 3000) WHERE website IS NOT NULL"
change_column :user_profiles, :location, :string, limit: 3000
change_column :user_profiles, :website, :string, limit: 3000
end
end

View File

@ -42,6 +42,15 @@ RSpec.describe UserProfile do
end end
end end
context "when it is > 3000 characters" do
before { profile.location = "a" * 3500 }
it "is not valid" do
expect(profile.valid?).to eq(false)
expect(profile.errors.full_messages).to include(/Location is too long \(maximum is 3000 characters\)/)
end
end
context "when it does not contain watched words" do context "when it does not contain watched words" do
it { is_expected.to be_valid } it { is_expected.to be_valid }
end end
@ -63,6 +72,15 @@ RSpec.describe UserProfile do
end end
end end
context "when it is > 3000 characters" do
before { profile.bio_raw = "a" * 3500 }
it "is not valid" do
expect(profile.valid?).to eq(false)
expect(profile.errors.full_messages).to include(/About Me is too long \(maximum is 3000 characters\)/)
end
end
context "when it does not contain watched words" do context "when it does not contain watched words" do
it { is_expected.to be_valid } it { is_expected.to be_valid }
end end
@ -129,6 +147,11 @@ RSpec.describe UserProfile do
user_profile.website = 'user - https://forum.example.com/user' user_profile.website = 'user - https://forum.example.com/user'
expect { user_profile.save! }.to raise_error(ActiveRecord::RecordInvalid) expect { user_profile.save! }.to raise_error(ActiveRecord::RecordInvalid)
end end
it "does not allow > 3000 characters" do
user_profile.website = "a" * 3500
expect(user_profile).to_not be_valid
end
end end
describe 'after save' do describe 'after save' do