Merge branch 'security-hiperlink-injection-prevention' into 'master'

Add sanitizing for name field

See merge request gitlab-org/security/gitlab!1460

app/models/user.rb

@@ -1262,12 +1262,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

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

spec/models/user_spec.rb

@@ -2904,7 +2904,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')
@@ -2913,6 +2913,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