Commit fb9a69b4 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch 'djadmin-security-sanitize-default-branch-names' into 'master'

Sanitize default branch name in repo settings

See merge request gitlab-org/gitlab!67567
parents 65b4b26f 38401468
...@@ -573,6 +573,7 @@ class ApplicationSetting < ApplicationRecord ...@@ -573,6 +573,7 @@ class ApplicationSetting < ApplicationRecord
before_validation :ensure_uuid! before_validation :ensure_uuid!
before_validation :coerce_repository_storages_weighted, if: :repository_storages_weighted_changed? before_validation :coerce_repository_storages_weighted, if: :repository_storages_weighted_changed?
before_validation :sanitize_default_branch_name
before_save :ensure_runners_registration_token before_save :ensure_runners_registration_token
before_save :ensure_health_check_access_token before_save :ensure_health_check_access_token
...@@ -602,6 +603,14 @@ class ApplicationSetting < ApplicationRecord ...@@ -602,6 +603,14 @@ class ApplicationSetting < ApplicationRecord
!!(sourcegraph_url =~ %r{\Ahttps://(www\.)?sourcegraph\.com}) !!(sourcegraph_url =~ %r{\Ahttps://(www\.)?sourcegraph\.com})
end end
def sanitize_default_branch_name
self.default_branch_name = if default_branch_name.blank?
nil
else
Sanitize.fragment(self.default_branch_name)
end
end
def instance_review_permitted? def instance_review_permitted?
users_count = Rails.cache.fetch('limited_users_count', expires_in: 1.day) do users_count = Rails.cache.fetch('limited_users_count', expires_in: 1.day) do
::User.limit(INSTANCE_REVIEW_MIN_USERS + 1).count(:all) ::User.limit(INSTANCE_REVIEW_MIN_USERS + 1).count(:all)
......
...@@ -22,7 +22,11 @@ class NamespaceSetting < ApplicationRecord ...@@ -22,7 +22,11 @@ class NamespaceSetting < ApplicationRecord
private private
def normalize_default_branch_name def normalize_default_branch_name
self.default_branch_name = nil if default_branch_name.blank? self.default_branch_name = if default_branch_name.blank?
nil
else
Sanitize.fragment(self.default_branch_name)
end
end end
def default_branch_name_content def default_branch_name_content
......
...@@ -218,6 +218,16 @@ RSpec.describe ApplicationSetting do ...@@ -218,6 +218,16 @@ RSpec.describe ApplicationSetting do
end end
end end
describe 'default_branch_name validaitions' do
context "when javascript tags get sanitized properly" do
it "gets sanitized properly" do
setting.update!(default_branch_name: "hello<script>alert(1)</script>")
expect(setting.default_branch_name).to eq('hello')
end
end
end
describe 'spam_check_endpoint' do describe 'spam_check_endpoint' do
context 'when spam_check_endpoint is enabled' do context 'when spam_check_endpoint is enabled' do
before do before do
......
...@@ -41,6 +41,14 @@ RSpec.describe NamespaceSetting, type: :model do ...@@ -41,6 +41,14 @@ RSpec.describe NamespaceSetting, type: :model do
it_behaves_like "doesn't return an error" it_behaves_like "doesn't return an error"
end end
context "when it contains javascript tags" do
it "gets sanitized properly" do
namespace_settings.update!(default_branch_name: "hello<script>alert(1)</script>")
expect(namespace_settings.default_branch_name).to eq('hello')
end
end
end end
describe '#allow_mfa_for_group' do describe '#allow_mfa_for_group' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment