diff options
| -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 |