diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-30 11:43:14 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-30 11:43:21 +0000 |
commit | 16fa5cf183d9f59a66c1e258ce36cd3f09c8d3fd (patch) | |
tree | b1662c1ee4766bba9d71cf2dc06204ab281a4d11 | |
parent | 33e4d44c11427a31ada41e7a0757d35f03d62ce7 (diff) | |
download | gitlab-ce-16fa5cf183d9f59a66c1e258ce36cd3f09c8d3fd.tar.gz |
Add latest changes from gitlab-org/security/gitlab@14-0-stable-ee
-rw-r--r-- | app/models/user.rb | 11 | ||||
-rw-r--r-- | spec/features/snippets/notes_on_personal_snippets_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 21 |
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&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 |