Add latest changes from gitlab-org/security/gitlab@15-11-stable-ee

15 files changed, 183 insertions, 19 deletions

diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index 0fac2cb5fc5..57075a44d0f 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -116,7 +116,7 @@ module AvatarsHelper
   private
 
   def avatar_icon_by_user_email_or_gravatar(email, size, scale, only_path:)
-    user = User.find_by_any_email(email)
+    user = User.with_public_email(email).first
 
     if user
       avatar_icon_for_user(user, size, scale, only_path: only_path)
diff --git a/doc/integration/saml.md b/doc/integration/saml.md
index ef32311f85d..45bb85ce47d 100644
--- a/doc/integration/saml.md
+++ b/doc/integration/saml.md
@@ -3119,7 +3119,7 @@ such as the following:
 | Encrypt SAML assertion | Optional | Uses TLS between your identity provider, the user's browser, and GitLab. |
 | Sign SAML assertion | Optional | Validates the integrity of a SAML assertion. When active, signs the whole response. |
 | Check SAML request signature | Optional | Checks the signature on the SAML response. |
-| Default RelayState | Optional | Specifies the URL users should end up on after successfully signing in through SAML at your IdP. |
+| Default RelayState | Optional | Specifies the sub-paths of the base URL that users should end up on after successfully signing in through SAML at your IdP. |
 | NameID format | Persistent | See [NameID format details](../user/group/saml_sso/index.md#manage-user-saml-identity). |
 | Additional URLs | Optional | May include the issuer, identifier, or assertion consumer service URL in other fields on some providers. |
 
diff --git a/lib/banzai/filter/asset_proxy_filter.rb b/lib/banzai/filter/asset_proxy_filter.rb
index 4c14ee7299b..6371a8f23af 100644
--- a/lib/banzai/filter/asset_proxy_filter.rb
+++ b/lib/banzai/filter/asset_proxy_filter.rb
@@ -6,11 +6,35 @@ module Banzai
     # as well as hiding the customer's IP address when requesting images.
     # Copies the original img `src` to `data-canonical-src` then replaces the
     # `src` with a new url to the proxy server.
-    class AssetProxyFilter < HTML::Pipeline::CamoFilter
+    #
+    # Based on https://github.com/gjtorikian/html-pipeline/blob/v2.14.3/lib/html/pipeline/camo_filter.rb
+    class AssetProxyFilter < HTML::Pipeline::Filter
       def initialize(text, context = nil, result = nil)
         super
       end
 
+      def call
+        return doc unless asset_proxy_enabled?
+
+        doc.search('img').each do |element|
+          original_src = element['src']
+          next unless original_src
+
+          begin
+            uri = URI.parse(original_src)
+          rescue StandardError
+            next
+          end
+
+          next if uri.host.nil? && !original_src.start_with?('///')
+          next if asset_host_allowed?(uri.host)
+
+          element['src'] = asset_proxy_url(original_src)
+          element['data-canonical-src'] = original_src
+        end
+        doc
+      end
+
       def validate
         needs(:asset_proxy, :asset_proxy_secret_key) if asset_proxy_enabled?
       end
@@ -63,6 +87,24 @@ module Banzai
           application_settings.try(:asset_proxy_whitelist).presence ||
           [Gitlab.config.gitlab.host]
       end
+
+      private
+
+      def asset_proxy_enabled?
+        !context[:disable_asset_proxy]
+      end
+
+      def asset_proxy_url(url)
+        "#{context[:asset_proxy]}/#{asset_url_hash(url)}/#{hexencode(url)}"
+      end
+
+      def asset_url_hash(url)
+        OpenSSL::HMAC.hexdigest('sha1', context[:asset_proxy_secret_key], url)
+      end
+
+      def hexencode(str)
+        str.unpack1('H*')
+      end
     end
   end
 end
diff --git a/lib/gitlab/checks/branch_check.rb b/lib/gitlab/checks/branch_check.rb
index e8f13a92ee7..fa7c4972c91 100644
--- a/lib/gitlab/checks/branch_check.rb
+++ b/lib/gitlab/checks/branch_check.rb
@@ -42,7 +42,7 @@ module Gitlab
       def prohibited_branch_checks
         return if deletion?
 
-        if branch_name =~ /\A\h{40}\z/
+        if branch_name =~ %r{\A\h{40}(/|\z)}
           raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_hex_branch_name]
         end
       end
diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb
index 6eb97a99264..b7fdadbd036 100644
--- a/spec/helpers/avatars_helper_spec.rb
+++ b/spec/helpers/avatars_helper_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe AvatarsHelper do
+RSpec.describe AvatarsHelper, feature_category: :source_code_management do
   include UploadHelpers
 
   let_it_be(:user) { create(:user) }
@@ -88,7 +88,7 @@ RSpec.describe AvatarsHelper do
   describe '#avatar_icon_for' do
     let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') }
     let(:email) { 'foo@example.com' }
-    let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) }
+    let!(:another_user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path), email: email) }
 
     it 'prefers the user to retrieve the avatar_url' do
       expect(helper.avatar_icon_for(user, email).to_s)
@@ -102,7 +102,7 @@ RSpec.describe AvatarsHelper do
   end
 
   describe '#avatar_icon_for_email', :clean_gitlab_redis_cache do
-    let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
+    let(:user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path)) }
 
     subject { helper.avatar_icon_for_email(user.email).to_s }
 
@@ -114,6 +114,14 @@ RSpec.describe AvatarsHelper do
           end
         end
 
+        context 'when a private email is used' do
+          it 'calls gravatar_icon' do
+            expect(helper).to receive(:gravatar_icon).with(user.commit_email, 20, 2)
+
+            helper.avatar_icon_for_email(user.commit_email, 20, 2)
+          end
+        end
+
         context 'when no user exists for the email' do
           it 'calls gravatar_icon' do
             expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
@@ -136,7 +144,7 @@ RSpec.describe AvatarsHelper do
     it_behaves_like "returns avatar for email"
 
     it "caches the request" do
-      expect(User).to receive(:find_by_any_email).once.and_call_original
+      expect(User).to receive(:with_public_email).once.and_call_original
 
       expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url)
       expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url)
diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
index 004c70c28f1..dc6ac52a8c2 100644
--- a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
+++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
@@ -80,6 +80,15 @@ RSpec.describe Banzai::Filter::AssetProxyFilter, feature_category: :team_plannin
       expect(doc.at_css('img')['data-canonical-src']).to eq src
     end
 
+    it 'replaces invalid URLs' do
+      src     = '///example.com/test.png'
+      new_src = 'https://assets.example.com/3368d2c7b9bed775bdd1e811f36a4b80a0dcd8ab/2f2f2f6578616d706c652e636f6d2f746573742e706e67'
+      doc     = filter(image(src), @context)
+
+      expect(doc.at_css('img')['src']).to eq new_src
+      expect(doc.at_css('img')['data-canonical-src']).to eq src
+    end
+
     it 'skips internal images' do
       src      = "#{Gitlab.config.gitlab.url}/test.png"
       doc      = filter(image(src), @context)
diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
index 3ebe0798972..896f3beb7c2 100644
--- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
+++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
@@ -218,7 +218,7 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c
       # any path-only link will automatically be prefixed
       # with the path of its repository.
       # See: "build_relative_path" in "lib/banzai/filter/relative_link_filter.rb"
-      let(:user_with_avatar) { create(:user, :with_avatar, username: 'foobar') }
+      let(:user_with_avatar) { create(:user, :public_email, :with_avatar, username: 'foobar') }
 
       it 'returns a full path for avatar urls' do
         _, message_html = build_commit_message(
diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb
index d6280d3c28c..7f535e86d69 100644
--- a/spec/lib/gitlab/checks/branch_check_spec.rb
+++ b/spec/lib/gitlab/checks/branch_check_spec.rb
@@ -26,8 +26,14 @@ RSpec.describe Gitlab::Checks::BranchCheck do
         expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.")
       end
 
+      it "prohibits 40-character hexadecimal branch names as the start of a path" do
+        allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e/test")
+
+        expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.")
+      end
+
       it "doesn't prohibit a nested hexadecimal in a branch name" do
-        allow(subject).to receive(:branch_name).and_return("fix-267208abfe40e546f5e847444276f7d43a39503e")
+        allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e-fix")
 
         expect { subject.validate! }.not_to raise_error
       end
diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb
index de10653d87e..a2ab59f56ab 100644
--- a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb
+++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb
@@ -23,8 +23,7 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do
       # we have an existing N+1, one for each project for which user is not a member
       # in this spec, project_3, project_4, project_5
       # https://gitlab.com/gitlab-org/gitlab/-/issues/362890
-      ee_only_policy_check_queries = Gitlab.ee? ? 1 : 0
-      expect { query }.to make_queries(projects.size + 3 + ee_only_policy_check_queries)
+      expect { query }.to make_queries(projects.size + 3)
     end
   end
 
diff --git a/workhorse/internal/headers/content_headers.go b/workhorse/internal/headers/content_headers.go
index 854cc8abddd..54c7c1bdd95 100644
--- a/workhorse/internal/headers/content_headers.go
+++ b/workhorse/internal/headers/content_headers.go
@@ -1,6 +1,7 @@
 package headers
 
 import (
+	"mime"
 	"net/http"
 	"regexp"
 
@@ -13,8 +14,9 @@ var (
 	imageTypeRegex   = regexp.MustCompile(`^image/*`)
 	svgMimeTypeRegex = regexp.MustCompile(`^image/svg\+xml$`)
 
-	textTypeRegex = regexp.MustCompile(`^text/*`)
-
+	textTypeRegex  = regexp.MustCompile(`^text/*`)
+	xmlTypeRegex   = regexp.MustCompile(`^text/xml`)
+	xhtmlTypeRegex = regexp.MustCompile(`^text/html`)
 	videoTypeRegex = regexp.MustCompile(`^video/*`)
 
 	pdfTypeRegex = regexp.MustCompile(`application\/pdf`)
@@ -26,6 +28,8 @@ var (
 // Mime types that can't be inlined. Usually subtypes of main types
 var forbiddenInlineTypes = []*regexp.Regexp{svgMimeTypeRegex}
 
+var htmlRenderingTypes = []*regexp.Regexp{xmlTypeRegex, xhtmlTypeRegex}
+
 // Mime types that can be inlined. We can add global types like "image/" or
 // specific types like "text/plain". If there is a specific type inside a global
 // allowed type that can't be inlined we must add it to the forbiddenInlineTypes var.
@@ -38,12 +42,28 @@ const (
 	textPlainContentType      = "text/plain; charset=utf-8"
 	attachmentDispositionText = "attachment"
 	inlineDispositionText     = "inline"
+	dummyFilename             = "blob"
 )
 
 func SafeContentHeaders(data []byte, contentDisposition string) (string, string) {
-	contentType := safeContentType(data)
+	detectedContentType := detectContentType(data)
+
+	contentType := safeContentType(detectedContentType)
 	contentDisposition = safeContentDisposition(contentType, contentDisposition)
 
+	// Some browsers will render XML inline unless a filename directive is provided with a non-xml file extension
+	// This overrides the filename directive in the case of XML data
+	for _, element := range htmlRenderingTypes {
+		if isType(detectedContentType, element) {
+			disposition, directives, err := mime.ParseMediaType(contentDisposition)
+			if err == nil {
+				directives["filename"] = dummyFilename
+				contentDisposition = mime.FormatMediaType(disposition, directives)
+				break
+			}
+		}
+	}
+
 	// Set attachments to application/octet-stream since browsers can do
 	// a better job distinguishing certain types (for example: ZIP files
 	// vs. Microsoft .docx files). However, browsers may safely render SVGs even
@@ -56,15 +76,17 @@ func SafeContentHeaders(data []byte, contentDisposition string) (string, string)
 	return contentType, contentDisposition
 }
 
-func safeContentType(data []byte) string {
+func detectContentType(data []byte) string {
 	// Special case for svg because DetectContentType detects it as text
 	if svg.Is(data) {
 		return svgContentType
 	}
 
 	// Override any existing Content-Type header from other ResponseWriters
-	contentType := http.DetectContentType(data)
+	return http.DetectContentType(data)
+}
 
+func safeContentType(contentType string) string {
 	// http.DetectContentType does not support JavaScript and would only
 	// return text/plain. But for cautionary measures, just in case they start supporting
 	// it down the road and start returning application/javascript, we want to handle it now
diff --git a/workhorse/internal/headers/content_headers_test.go b/workhorse/internal/headers/content_headers_test.go
new file mode 100644
index 00000000000..7cfce335d88
--- /dev/null
+++ b/workhorse/internal/headers/content_headers_test.go
@@ -0,0 +1,56 @@
+package headers
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func fileContents(fileName string) []byte {
+	fileContents, _ := os.ReadFile(fileName)
+	return fileContents
+}
+
+func TestHeaders(t *testing.T) {
+	tests := []struct {
+		desc                       string
+		fileContents               []byte
+		expectedContentType        string
+		expectedContentDisposition string
+	}{
+		{
+			desc:                       "XML file",
+			fileContents:               fileContents("../../testdata/test.xml"),
+			expectedContentType:        "text/plain; charset=utf-8",
+			expectedContentDisposition: "inline; filename=blob",
+		},
+		{
+			desc:                       "XHTML file",
+			fileContents:               fileContents("../../testdata/index.xhtml"),
+			expectedContentType:        "text/plain; charset=utf-8",
+			expectedContentDisposition: "inline; filename=blob",
+		},
+		{
+			desc:                       "svg+xml file",
+			fileContents:               fileContents("../../testdata/xml.svg"),
+			expectedContentType:        "image/svg+xml",
+			expectedContentDisposition: "attachment",
+		},
+		{
+			desc:                       "text file",
+			fileContents:               []byte(`a text file`),
+			expectedContentType:        "text/plain; charset=utf-8",
+			expectedContentDisposition: "inline",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			contentType, newContentDisposition := SafeContentHeaders(test.fileContents, "")
+
+			require.Equal(t, test.expectedContentType, contentType)
+			require.Equal(t, test.expectedContentDisposition, newContentDisposition)
+		})
+	}
+}
diff --git a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go
index b04263de6b9..e863935be6f 100644
--- a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go
+++ b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go
@@ -51,13 +51,13 @@ func TestSetProperContentTypeAndDisposition(t *testing.T) {
 		{
 			desc:               "HTML type",
 			contentType:        "text/plain; charset=utf-8",
-			contentDisposition: "inline",
+			contentDisposition: "inline; filename=blob",
 			body:               "<html><body>Hello world!</body></html>",
 		},
 		{
 			desc:               "Javascript within HTML type",
 			contentType:        "text/plain; charset=utf-8",
-			contentDisposition: "inline",
+			contentDisposition: "inline; filename=blob",
 			body:               "<script>alert(\"foo\")</script>",
 		},
 		{
diff --git a/workhorse/testdata/index.xhtml b/workhorse/testdata/index.xhtml
new file mode 100644
index 00000000000..1dd50a70e69
--- /dev/null
+++ b/workhorse/testdata/index.xhtml
@@ -0,0 +1,9 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
+"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+  <title>Title of document</title>
+</head>
+<body>
+</body>
+</html>
diff --git a/workhorse/testdata/test.xml b/workhorse/testdata/test.xml
new file mode 100644
index 00000000000..54b94e62355
--- /dev/null
+++ b/workhorse/testdata/test.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<html>
+	<head></head>
+	<body>
+	</body>
+</html>
\ No newline at end of file
diff --git a/workhorse/testdata/xml.svg b/workhorse/testdata/xml.svg
new file mode 100644
index 00000000000..c41c4c44b49
--- /dev/null
+++ b/workhorse/testdata/xml.svg
@@ -0,0 +1,7 @@
+<?xml version="1.0" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+
+<svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
+  <polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/>
+  <div>hello this is html</div>
+</svg>