Add latest changes from gitlab-org/security/gitlab@16-7-stable-ee

app/helpers/projects_helper.rb

@@ -800,7 +800,7 @@ def build_namespace_breadcrumb_link(project)
       group_title(project.group, nil, nil)
     else
       owner = project.namespace.owner
-      name = simple_sanitize(owner.name)
+      name = sanitize(owner.name, tags: [])
       url = user_path(owner)
 
       push_to_schema_breadcrumb(name, url)

app/models/user.rb

@@ -1431,7 +1431,7 @@ def sanitize_attrs
   def sanitize_name
     return unless self.name
 
-    self.name = self.name.gsub(%r{</?[^>]*>}, '')
+    self.name = self.name.gsub(%r{(?:</?[^>]*>|<|>)}, '-')
   end
 
   def unset_secondary_emails_matching_deleted_email!(deleted_email)

app/services/merge_requests/update_assignees_service.rb

@@ -7,7 +7,7 @@ class UpdateAssigneesService < UpdateService
     # This saves a lot of queries for irrelevant things that cannot possibly
     # change in the execution of this service.
     def execute(merge_request)
-      return merge_request unless current_user&.can?(:update_merge_request, merge_request)
+      return merge_request unless current_user&.can?(:set_merge_request_metadata, merge_request)
 
       old_assignees = merge_request.assignees.to_a
       old_ids = old_assignees.map(&:id)

app/services/merge_requests/update_reviewers_service.rb

@@ -3,7 +3,7 @@
 module MergeRequests
   class UpdateReviewersService < UpdateService
     def execute(merge_request)
-      return merge_request unless current_user&.can?(:update_merge_request, merge_request)
+      return merge_request unless current_user&.can?(:set_merge_request_metadata, merge_request)
 
       old_reviewers = merge_request.reviewers.to_a
       old_ids = old_reviewers.map(&:id)

app/views/profiles/_name.html.haml

@@ -7,3 +7,4 @@
   = form.text_field :name, class: 'gl-form-input form-control', required: true, title: s_("Profiles|Using emoji in names seems fun, but please try to set a status message instead")
   %small.form-text.text-gl-muted
     = s_("Profiles|Enter your name, so people you know can recognize you.")
+    = safe_format(_("Profiles|No \"<\" or \">\" characters, please."))

app/views/projects/tags/_tag.atom.builder

@@ -3,6 +3,9 @@
 commit = @repository.commit(tag.dereferenced_target)
 release = @releases.find { |r| r.tag == tag.name }
 tag_url = project_tag_url(@project, tag.name)
+author_email = Gitlab::SafeRequestStore.fetch([:commit_author_email, commit.author_email]) do
+  commit.author&.public_email || commit.author_email
+end
 
 if commit
   xml.entry do
@@ -12,10 +15,10 @@ if commit
     xml.summary strip_signature(tag.message)
     xml.content markdown_field(release, :description), type: 'html'
     xml.updated release.updated_at.xmlschema if release
-    xml.media   :thumbnail, width: '40', height: '40', url: image_url(avatar_icon_for_email(commit.author_email))
+    xml.media   :thumbnail, width: '40', height: '40', url: image_url(avatar_icon_for_email(author_email))
     xml.author do |author|
       xml.name  commit.author_name
-      xml.email commit.author_email
+      xml.email author_email
     end
   end
 end

lib/gitlab/dependency_linker/cargo_toml_linker.rb

@@ -32,7 +32,7 @@ def link_dependencies_at(type)
 
       def link_toml(key, value, type, &url_proc)
         if value.is_a? String
-          link_regex(/^(?<name>#{key})\s*=\s*"#{value}"/, &url_proc)
+          link_regex(Gitlab::UntrustedRegexp.new(%{^(?<name>#{key})\\s*=\\s*"#{value}"}), &url_proc)
         elsif value.is_a? Hash
           # Don't link when using a custom registry
           return if value['registry']
@@ -41,8 +41,8 @@ def link_toml(key, value, type, &url_proc)
           # See https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations
           return unless value['version']
 
-          link_regex(/^(?<name>#{key})\s*=\s*\{/, &url_proc)
-          link_regex(/^\[#{type}\.(?<name>#{key})\]/, &url_proc)
+          link_regex(Gitlab::UntrustedRegexp.new("^(?<name>#{key})\\s*=\\s*\\{"), &url_proc)
+          link_regex(Gitlab::UntrustedRegexp.new("^\\[#{type}\\.(?<name>#{key})\\]"), &url_proc)
         end
       end
 

locale/gitlab.pot

@@ -37228,6 +37228,9 @@ msgstr ""
 msgid "Profiles|Manage two-factor authentication"
 msgstr ""
 
+msgid "Profiles|No \"<\" or \">\" characters, please."
+msgstr ""
+
 msgid "Profiles|No file chosen."
 msgstr ""
 

spec/helpers/projects_helper_spec.rb

@@ -856,6 +856,16 @@ def license_name
 
       subject
     end
+
+    context 'with malicious owner name' do
+      before do
+        allow_any_instance_of(User).to receive(:name).and_return('a<a class="fixed-top" href=/api/v4')
+      end
+
+      it 'escapes the malicious owner name' do
+        expect(subject).not_to include('<a class="fixed-top" href="/api/v4"></a>')
+      end
+    end
   end
 
   describe '#project_permissions_panel_data' do

spec/lib/gitlab/dependency_linker/cargo_toml_linker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::DependencyLinker::CargoTomlLinker do
+RSpec.describe Gitlab::DependencyLinker::CargoTomlLinker, feature_category: :source_code_management do
   describe '.support?' do
     it 'supports Cargo.toml' do
       expect(described_class.support?('Cargo.toml')).to be_truthy
@@ -103,5 +103,27 @@ def link(name, url)
       expect(subject).not_to include(link('registry-ignored', 'https://crates.io/crates/registry-ignored'))
       expect(subject).not_to include(link('custom-rand', 'https://crates.io/crates/custom-rand'))
     end
+
+    context 'when file contents contain special regular expressions' do
+      let(:file_content) do
+        <<-CONTENT.strip_heredoc
+          [dependencies]
+          chrono = "0.4.7"
+          ".*((a|a)+|c)+"= { version = "0.17.5", path = ["aaaaaaaaaaaaaaaaaaa"] }
+          ".*((a|a)+|d)+"="aaaaaaaaaaaaaaaaaaa"
+
+          [dependencies.".*((a|a)+|e)+"]
+          version = "1.2.3"
+          path = "aaaaaaaaaaaaaaaaaaa"
+        CONTENT
+      end
+
+      it 'protects against malicious backtracking' do
+        expect do
+          Timeout.timeout(Gitlab::OtherMarkup::RENDER_TIMEOUT.seconds) { subject }
+        end.not_to raise_error
+        expect(subject).to include(link('chrono', 'https://crates.io/crates/chrono'))
+      end
+    end
   end
 end

spec/models/user_spec.rb

@@ -3954,28 +3954,55 @@ def login_method(login)
   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 '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>')
+    context 'for name attribute' do
+      subject { user.name }
 
-      expect(user.name).to eq('Test')
-    end
+      before do
+        user.name = input_name
+        user.sanitize_attrs
+      end
 
-    it 'sanitizes attr from js scripts' do
-      user = create(:user, name: '<script>alert("Test")</script>')
+      context 'from html tags' do
+        let(:input_name) { '<a href="//example.com">Test<a>' }
 
-      expect(user.name).to eq("alert(\"Test\")")
-    end
+        it { is_expected.to eq('-Test-') }
+      end
+
+      context 'from unclosed html tags' do
+        let(:input_name) { 'a<a class="js-evil" href=/api/v4' }
+
+        it { is_expected.to eq('a-a class="js-evil" href=/api/v4') }
+      end
+
+      context 'from closing html brackets' do
+        let(:input_name) { 'alice some> tag' }
 
-    it 'sanitizes attr from iframe scripts' do
-      user = create(:user, name: 'User"><iframe src=javascript:alert()><iframe>')
+        it { is_expected.to eq('alice some- tag') }
+      end
+
+      context 'from self-closing tags' do
+        let(:input_name) { '</link>alice' }
+
+        it { is_expected.to eq('-alice') }
+      end
+
+      context 'from js scripts' do
+        let(:input_name) { '<script>alert("Test")</script>' }
 
-      expect(user.name).to eq('User">')
+        it { is_expected.to eq('-alert("Test")-') }
+      end
+
+      context 'from iframe scripts' do
+        let(:input_name) { 'User"><iframe src=javascript:alert()><iframe>' }
+
+        it { is_expected.to eq('User"---') }
+      end
     end
   end
 

spec/requests/projects/tags_controller_spec.rb

@@ -25,6 +25,21 @@
     end
   end
 
+  describe "atom feed contents" do
+    let_it_be(:project) { create(:project, :repository, :public) }
+
+    it "returns the author's public email address rather than the commit email, when present" do
+      get(project_tags_url(project, format: :atom))
+
+      doc = Hash.from_xml(response.body)
+      commit_entry = doc["feed"]["entry"].first
+
+      expect(commit_entry["author"]).to be_a(Hash)
+      expect(commit_entry["author"]["name"]).to be_a(String)
+      expect(commit_entry["author"]["email"]).to be_a(String)
+    end
+  end
+
   describe '#show' do
     let_it_be(:project) { create(:project, :repository, :public) }
     let_it_be(:user) { create(:user) }

spec/services/merge_requests/update_assignees_service_spec.rb

@@ -128,5 +128,21 @@ def update_merge_request
           .to issue_fewer_queries_than { update_service.execute(other_mr) }
       end
     end
+
+    context 'when user has no set_merge_request_metadata permissions' do
+      before do
+        allow(user).to receive(:can?).and_call_original
+
+        allow(user)
+          .to receive(:can?)
+          .with(:set_merge_request_metadata, merge_request)
+          .and_return(false)
+      end
+
+      it 'does not update the MR assignees' do
+        expect { update_merge_request }
+          .not_to change { merge_request.reload.assignees.to_a }
+      end
+    end
   end
 end

spec/services/merge_requests/update_reviewers_service_spec.rb

@@ -174,5 +174,21 @@ def find_note(starting_with)
         )
       end
     end
+
+    context 'when user has no set_merge_request_metadata permissions' do
+      before do
+        allow(user).to receive(:can?).and_call_original
+
+        allow(user)
+          .to receive(:can?)
+          .with(:set_merge_request_metadata, merge_request)
+          .and_return(false)
+      end
+
+      it 'does not update the MR reviewers' do
+        expect { set_reviewers }
+          .not_to change { merge_request.reload.reviewers.to_a }
+      end
+    end
   end
 end