Add latest changes from gitlab-org/security/gitlab@14-8-stable-ee

16 files changed, 204 insertions, 19 deletions

diff --git a/app/assets/javascripts/snippets/components/show.vue b/app/assets/javascripts/snippets/components/show.vue
index 35d88d5ec8e..ee8b00c1f5d 100644
--- a/app/assets/javascripts/snippets/components/show.vue
+++ b/app/assets/javascripts/snippets/components/show.vue
@@ -1,5 +1,5 @@
 <script>
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlAlert, GlLoadingIcon } from '@gitlab/ui';
 import eventHub from '~/blob/components/eventhub';
 import {
   SNIPPET_MARK_VIEW_APP_START,
@@ -23,6 +23,7 @@ export default {
     EmbedDropdown,
     SnippetHeader,
     SnippetTitle,
+    GlAlert,
     GlLoadingIcon,
     SnippetBlob,
     CloneDropdownButton,
@@ -35,6 +36,9 @@ export default {
     canBeCloned() {
       return Boolean(this.snippet.sshUrlToRepo || this.snippet.httpUrlToRepo);
     },
+    hasUnretrievableBlobs() {
+      return this.snippet.hasUnretrievableBlobs;
+    },
   },
   beforeCreate() {
     performanceMarkAndMeasure({ mark: SNIPPET_MARK_VIEW_APP_START });
@@ -66,6 +70,13 @@ export default {
           data-qa-selector="clone_button"
         />
       </div>
+      <gl-alert v-if="hasUnretrievableBlobs" variant="danger" class="gl-mb-3" :dismissible="false">
+        {{
+          __(
+            'WARNING: This snippet contains hidden files which might be used to mask malicious behavior. Exercise caution if cloning and executing code from this snippet.',
+          )
+        }}
+      </gl-alert>
       <snippet-blob
         v-for="blob in blobs"
         :key="blob.path"
diff --git a/app/assets/javascripts/snippets/mixins/snippets.js b/app/assets/javascripts/snippets/mixins/snippets.js
index b72befef56b..0b3cca4e53a 100644
--- a/app/assets/javascripts/snippets/mixins/snippets.js
+++ b/app/assets/javascripts/snippets/mixins/snippets.js
@@ -17,6 +17,7 @@ export const getSnippetMixin = {
 
         // Set `snippet.blobs` since some child components are coupled to this.
         if (!isEmpty(res)) {
+          res.hasUnretrievableBlobs = res.blobs?.hasUnretrievableBlobs || false;
           // It's possible for us to not get any blobs in a response.
           // In this case, we should default to current blobs.
           res.blobs = res.blobs ? res.blobs.nodes : blobsDefault;
diff --git a/app/graphql/queries/snippet/snippet.query.graphql b/app/graphql/queries/snippet/snippet.query.graphql
index 24b268ec853..5c0c7ebaa1b 100644
--- a/app/graphql/queries/snippet/snippet.query.graphql
+++ b/app/graphql/queries/snippet/snippet.query.graphql
@@ -15,6 +15,7 @@ query GetSnippetQuery($ids: [SnippetID!]) {
       sshUrlToRepo
       blobs {
         __typename
+        hasUnretrievableBlobs
         nodes {
           __typename
           binary
diff --git a/app/graphql/queries/snippet/snippet_blob_content.query.graphql b/app/graphql/queries/snippet/snippet_blob_content.query.graphql
index 005f42ff726..4459a5e4316 100644
--- a/app/graphql/queries/snippet/snippet_blob_content.query.graphql
+++ b/app/graphql/queries/snippet/snippet_blob_content.query.graphql
@@ -12,6 +12,7 @@ query SnippetBlobContent($ids: [ID!], $rich: Boolean!, $paths: [String!]) {
           richData @include(if: $rich)
           plainData @skip(if: $rich)
         }
+        hasUnretrievableBlobs
       }
     }
   }
diff --git a/app/graphql/resolvers/snippets/blobs_resolver.rb b/app/graphql/resolvers/snippets/blobs_resolver.rb
index cbbc65d7263..29716ce1394 100644
--- a/app/graphql/resolvers/snippets/blobs_resolver.rb
+++ b/app/graphql/resolvers/snippets/blobs_resolver.rb
@@ -19,18 +19,18 @@ module Resolvers
       def resolve(paths: [])
         return [snippet.blob] if snippet.empty_repo?
 
-        if paths.empty?
-          snippet.blobs
-        else
-          snippet.repository.blobs_at(transformed_blob_paths(paths))
-        end
-      end
-
-      private
-
-      def transformed_blob_paths(paths)
-        ref = snippet.default_branch
-        paths.map { |path| [ref, path] }
+        paths = snippet.all_files if paths.empty?
+        blobs = snippet.blobs(paths)
+
+        # TODO: Some blobs, e.g. those with non-utf8 filenames, are returned as nil from the
+        # repository. We need to provide a flag to notify the user of this until we come up with a
+        # way to retrieve and display these blobs. We will be exploring a more holistic solution for
+        # this general problem of making all blobs retrievable as part
+        # of https://gitlab.com/gitlab-org/gitlab/-/issues/323082, at which point this attribute may
+        # be removed.
+        context[:unretrievable_blobs?] = blobs.size < paths.size
+
+        blobs
       end
     end
   end
diff --git a/app/graphql/types/snippets/blob_connection_type.rb b/app/graphql/types/snippets/blob_connection_type.rb
new file mode 100644
index 00000000000..15d26af7374
--- /dev/null
+++ b/app/graphql/types/snippets/blob_connection_type.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module Types
+  module Snippets
+    # rubocop: disable Graphql/AuthorizeTypes
+    class BlobConnectionType < GraphQL::Types::Relay::BaseConnection
+      field :has_unretrievable_blobs, GraphQL::Types::Boolean, null: false,
+            description: 'Indicates if the snippet has unretrievable blobs.',
+            resolver_method: :unretrievable_blobs?
+
+      def unretrievable_blobs?
+        !!context[:unretrievable_blobs?]
+      end
+    end
+  end
+end
diff --git a/app/graphql/types/snippets/blob_type.rb b/app/graphql/types/snippets/blob_type.rb
index 2b9b76a6194..80702c71f63 100644
--- a/app/graphql/types/snippets/blob_type.rb
+++ b/app/graphql/types/snippets/blob_type.rb
@@ -8,6 +8,8 @@ module Types
       description 'Represents the snippet blob'
       present_using SnippetBlobPresenter
 
+      connection_type_class(Types::Snippets::BlobConnectionType)
+
       field :rich_data, GraphQL::Types::String,
             description: 'Blob highlighted data.',
             null: true
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 6a8123b3c08..b04fca64c87 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -237,15 +237,19 @@ class Snippet < ApplicationRecord
     end
   end
 
+  def all_files
+    list_files(default_branch)
+  end
+
   def blob
     @blob ||= Blob.decorate(SnippetBlob.new(self), self)
   end
 
-  def blobs
+  def blobs(paths = [])
     return [] unless repository_exists?
 
-    files = list_files(default_branch)
-    items = files.map { |file| [default_branch, file] }
+    paths = all_files if paths.empty?
+    items = paths.map { |path| [default_branch, path] }
 
     repository.blobs_at(items).compact
   end
diff --git a/config/initializers/action_mailer_hooks.rb b/config/initializers/action_mailer_hooks.rb
index 46d5e387d9d..fb09ed34bf6 100644
--- a/config/initializers/action_mailer_hooks.rb
+++ b/config/initializers/action_mailer_hooks.rb
@@ -8,6 +8,7 @@ end
 ActionMailer::Base.register_interceptors(
   ::Gitlab::Email::Hook::AdditionalHeadersInterceptor,
   ::Gitlab::Email::Hook::EmailTemplateInterceptor,
+  ::Gitlab::Email::Hook::ValidateAddressesInterceptor,
   ::Gitlab::Email::Hook::DeliveryMetricsObserver
 )
 
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 18c99b7d151..49c0361123e 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -7841,6 +7841,7 @@ The connection type for [`SnippetBlob`](#snippetblob).
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | <a id="snippetblobconnectionedges"></a>`edges` | [`[SnippetBlobEdge]`](#snippetblobedge) | A list of edges. |
+| <a id="snippetblobconnectionhasunretrievableblobs"></a>`hasUnretrievableBlobs` | [`Boolean!`](#boolean) | Indicates if the snippet has unretrievable blobs. |
 | <a id="snippetblobconnectionnodes"></a>`nodes` | [`[SnippetBlob]`](#snippetblob) | A list of nodes. |
 | <a id="snippetblobconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. |
 
diff --git a/lib/gitlab/email/hook/validate_addresses_interceptor.rb b/lib/gitlab/email/hook/validate_addresses_interceptor.rb
new file mode 100644
index 00000000000..e63f047e63d
--- /dev/null
+++ b/lib/gitlab/email/hook/validate_addresses_interceptor.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Email
+    module Hook
+      # Check for unsafe characters in the envelope-from and -to addresses.
+      # These are passed directly as arguments to sendmail and are liable to shell injection attacks:
+      # https://github.com/mikel/mail/blob/2.7.1/lib/mail/network/delivery_methods/sendmail.rb#L53-L58
+      class ValidateAddressesInterceptor
+        UNSAFE_CHARACTERS = /(\\|[^[:print:]])/.freeze
+
+        def self.delivering_email(message)
+          addresses = Array(message.smtp_envelope_from) + Array(message.smtp_envelope_to)
+
+          addresses.each do |address|
+            next unless address.match?(UNSAFE_CHARACTERS)
+
+            Gitlab::AuthLogger.info(
+              message: 'Skipping email with unsafe characters in address',
+              address: address,
+              subject: message.subject
+            )
+
+            message.perform_deliveries = false
+
+            break
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 37f8a3fd7b4..6cdc5ba18e2 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -40768,6 +40768,9 @@ msgstr ""
 msgid "WARNING:"
 msgstr ""
 
+msgid "WARNING: This snippet contains hidden files which might be used to mask malicious behavior. Exercise caution if cloning and executing code from this snippet."
+msgstr ""
+
 msgid "Wait for the file to load to copy its contents"
 msgstr ""
 
diff --git a/spec/frontend/snippets/components/show_spec.js b/spec/frontend/snippets/components/show_spec.js
index af61f4ea54f..c73bf8f80a2 100644
--- a/spec/frontend/snippets/components/show_spec.js
+++ b/spec/frontend/snippets/components/show_spec.js
@@ -1,4 +1,4 @@
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlLoadingIcon, GlAlert } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import { Blob, BinaryBlob } from 'jest/blob/components/mock_data';
 import EmbedDropdown from '~/snippets/components/embed_dropdown.vue';
@@ -106,6 +106,23 @@ describe('Snippet view app', () => {
     });
   });
 
+  describe('hasUnretrievableBlobs alert rendering', () => {
+    it.each`
+      hasUnretrievableBlobs | condition       | isRendered
+      ${false}              | ${'not render'} | ${false}
+      ${true}               | ${'render'}     | ${true}
+    `('does $condition gl-alert by default', ({ hasUnretrievableBlobs, isRendered }) => {
+      createComponent({
+        data: {
+          snippet: {
+            hasUnretrievableBlobs,
+          },
+        },
+      });
+      expect(wrapper.findComponent(GlAlert).exists()).toBe(isRendered);
+    });
+  });
+
   describe('Clone button rendering', () => {
     it.each`
       httpUrlToRepo   | sshUrlToRepo   | shouldRender    | isRendered
diff --git a/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb b/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb
index ebe286769cf..b5db7ac5748 100644
--- a/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb
+++ b/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb
@@ -13,11 +13,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
     let_it_be(:current_user) { create(:user) }
     let_it_be(:snippet) { create(:personal_snippet, :private, :repository, author: current_user) }
 
+    let(:query_context) { {} }
+
     context 'when user is not authorized' do
       let(:other_user) { create(:user) }
 
       it 'redacts the field' do
         expect(resolve_blobs(snippet, user: other_user)).to be_nil
+        expect(query_context[:unretrievable_blobs?]).to eq(false)
       end
     end
 
@@ -28,6 +31,7 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
         expect(result).to match_array(snippet.list_files.map do |file|
           have_attributes(path: file)
         end)
+        expect(query_context[:unretrievable_blobs?]).to eq(false)
       end
     end
 
@@ -37,12 +41,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
           path = 'CHANGELOG'
 
           expect(resolve_blobs(snippet, paths: [path])).to contain_exactly(have_attributes(path: path))
+          expect(query_context[:unretrievable_blobs?]).to eq(false)
         end
       end
 
       context 'the argument does not match anything' do
         it 'returns an empty result' do
           expect(resolve_blobs(snippet, paths: ['does not exist'])).to be_empty
+          expect(query_context[:unretrievable_blobs?]).to eq(true)
         end
       end
 
@@ -53,12 +59,15 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do
           expect(resolve_blobs(snippet, paths: paths)).to match_array(paths.map do |file|
             have_attributes(path: file)
           end)
+          expect(query_context[:unretrievable_blobs?]).to eq(false)
         end
       end
     end
   end
 
-  def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths })
-    resolve(described_class, args: args, ctx: { current_user: user }, obj: snippet)
+  def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths }, has_unretrievable_blobs: false)
+    query_context[:current_user] = user
+    query_context[:unretrievable_blobs?] = has_unretrievable_blobs
+    resolve(described_class, args: args, ctx: query_context, obj: snippet)
   end
 end
diff --git a/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb b/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb
new file mode 100644
index 00000000000..a3f0158db40
--- /dev/null
+++ b/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Email::Hook::ValidateAddressesInterceptor do
+  describe 'UNSAFE_CHARACTERS' do
+    subject { described_class::UNSAFE_CHARACTERS }
+
+    it { is_expected.to match('\\') }
+    it { is_expected.to match("\x00") }
+    it { is_expected.to match("\x01") }
+    it { is_expected.not_to match('') }
+    it { is_expected.not_to match('user@example.com') }
+    it { is_expected.not_to match('foo-123+bar_456@example.com') }
+  end
+
+  describe '.delivering_email' do
+    let(:mail) do
+      ActionMailer::Base.mail(to: 'test@mail.com', from: 'info@mail.com', subject: 'title', body: 'hello')
+    end
+
+    let(:unsafe_email) { "evil+\x01$HOME@example.com" }
+
+    it 'sends emails to normal addresses' do
+      expect(Gitlab::AuthLogger).not_to receive(:info)
+      expect { mail.deliver_now }.to change(ActionMailer::Base.deliveries, :count)
+    end
+
+    [:from, :to, :cc, :bcc].each do |header|
+      it "does not send emails if the #{header.inspect} header contains unsafe characters" do
+        mail[header] = unsafe_email
+
+        expect(Gitlab::AuthLogger).to receive(:info).with(
+          message: 'Skipping email with unsafe characters in address',
+          address: unsafe_email,
+          subject: mail.subject
+        )
+
+        expect { mail.deliver_now }.not_to change(ActionMailer::Base.deliveries, :count)
+      end
+    end
+
+    [:reply_to].each do |header|
+      it "sends emails if the #{header.inspect} header contains unsafe characters" do
+        mail[header] = unsafe_email
+
+        expect(Gitlab::AuthLogger).not_to receive(:info)
+        expect { mail.deliver_now }.to change(ActionMailer::Base.deliveries, :count)
+      end
+    end
+  end
+end
diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb
index 5d4a78bb15f..92e4bc7d1a9 100644
--- a/spec/models/snippet_spec.rb
+++ b/spec/models/snippet_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Snippet do
+  include FakeBlobHelpers
+
   describe 'modules' do
     subject { described_class }
 
@@ -526,6 +528,21 @@ RSpec.describe Snippet do
     end
   end
 
+  describe '#all_files' do
+    let(:snippet) { create(:snippet, :repository) }
+    let(:files) { double(:files) }
+
+    subject(:all_files) { snippet.all_files }
+
+    before do
+      allow(snippet.repository).to receive(:ls_files).with(snippet.default_branch).and_return(files)
+    end
+
+    it 'lists files from the repository with the default branch' do
+      expect(all_files).to eq(files)
+    end
+  end
+
   describe '#blobs' do
     context 'when repository does not exist' do
       let(:snippet) { create(:snippet) }
@@ -552,6 +569,23 @@ RSpec.describe Snippet do
         end
       end
     end
+
+    context 'when some blobs are not retrievable from repository' do
+      let(:snippet) { create(:snippet, :repository) }
+      let(:container) { double(:container) }
+      let(:retrievable_filename) { 'retrievable_file'}
+      let(:unretrievable_filename) { 'unretrievable_file'}
+
+      before do
+        allow(snippet).to receive(:list_files).and_return([retrievable_filename, unretrievable_filename])
+        blob = fake_blob(path: retrievable_filename, container: container)
+        allow(snippet.repository).to receive(:blobs_at).and_return([blob, nil])
+      end
+
+      it 'does not include unretrievable blobs' do
+        expect(snippet.blobs.map(&:name)).to contain_exactly(retrievable_filename)
+      end
+    end
   end
 
   describe '#to_json' do