summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-06-30 11:43:14 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-06-30 11:43:21 +0000
commit16fa5cf183d9f59a66c1e258ce36cd3f09c8d3fd (patch)
treeb1662c1ee4766bba9d71cf2dc06204ab281a4d11
parent33e4d44c11427a31ada41e7a0757d35f03d62ce7 (diff)
downloadgitlab-ce-16fa5cf183d9f59a66c1e258ce36cd3f09c8d3fd.tar.gz
Add latest changes from gitlab-org/security/gitlab@14-0-stable-ee
-rw-r--r--app/models/user.rb11
-rw-r--r--spec/features/snippets/notes_on_personal_snippets_spec.rb12
-rw-r--r--spec/models/user_spec.rb21
3 files changed, 31 insertions, 13 deletions
diff --git a/app/models/user.rb b/app/models/user.rb
index 5fbd6271589..3879eb51371 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1257,12 +1257,23 @@ class User < ApplicationRecord
end
def sanitize_attrs
+ sanitize_links
+ sanitize_name
+ end
+
+ def sanitize_links
%i[skype linkedin twitter].each do |attr|
value = self[attr]
self[attr] = Sanitize.clean(value) if value.present?
end
end
+ def sanitize_name
+ return unless self.name
+
+ self.name = self.name.gsub(%r{</?[^>]*>}, '')
+ end
+
def set_notification_email
if notification_email.blank? || all_emails.exclude?(notification_email)
self.notification_email = email
diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb
index 47dad9bd88e..e03f71c5352 100644
--- a/spec/features/snippets/notes_on_personal_snippets_spec.rb
+++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb
@@ -65,18 +65,6 @@ RSpec.describe 'Comments on personal snippets', :js do
expect(page).to have_content(user_name)
end
end
-
- context 'when the author name contains HTML' do
- let(:user_name) { '<h1><a href="https://bad.link/malicious.exe" class="evil">Fake Content<img class="fake-icon" src="image.png"></a></h1>' }
-
- it 'renders the name as plain text' do
- visit snippet_path(snippet)
-
- content = find("#note_#{snippet_notes[0].id} .note-header-author-name").text
-
- expect(content).to eq user_name
- end
- end
end
context 'when submitting a note' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index e5c86e69ffc..dc78ec2be21 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2882,7 +2882,7 @@ RSpec.describe User do
end
describe '#sanitize_attrs' do
- let(:user) { build(:user, name: 'test & user', skype: 'test&user') }
+ let(:user) { build(:user, name: 'test <& user', skype: 'test&user') }
it 'encodes HTML entities in the Skype attribute' do
expect { user.sanitize_attrs }.to change { user.skype }.to('test&amp;user')
@@ -2891,6 +2891,25 @@ RSpec.describe User do
it 'does not encode HTML entities in the name attribute' do
expect { user.sanitize_attrs }.not_to change { user.name }
end
+
+ it 'sanitizes attr from html tags' do
+ user = create(:user, name: '<a href="//example.com">Test<a>', twitter: '<a href="//evil.com">https://twitter.com<a>')
+
+ expect(user.name).to eq('Test')
+ expect(user.twitter).to eq('https://twitter.com')
+ end
+
+ it 'sanitizes attr from js scripts' do
+ user = create(:user, name: '<script>alert("Test")</script>')
+
+ expect(user.name).to eq("alert(\"Test\")")
+ end
+
+ it 'sanitizes attr from iframe scripts' do
+ user = create(:user, name: 'User"><iframe src=javascript:alert()><iframe>')
+
+ expect(user.name).to eq('User">')
+ end
end
describe '#starred?' do