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

changelogs/unreleased/security-workhorse-package-bypass.yml

+---
+title: Add workhorse request verification to package upload endpoints
+merge_request:
+author:
+type: security

doc/api/graphql/reference/gitlab_schema.graphql

@@ -2414,9 +2414,9 @@ type GrafanaIntegration {
   id: ID!
   """
-  API token for the Grafana integration
+  API token for the Grafana integration. Field is permanently masked.
   """
-  token: String!
+  token: String! @deprecated(reason: "Plain text token has been masked for security reasons")
   """
   Timestamp of the issue's last activity

doc/api/graphql/reference/gitlab_schema.json

@@ -16249,7 +16249,7 @@
             },
             {
               "name": "token",
-              "description": "API token for the Grafana integration",
+              "description": "API token for the Grafana integration. Field is permanently masked.",
               "args": [
               ],
@@ -16262,8 +16262,8 @@
                   "ofType": null
                 }
               },
-              "isDeprecated": false,
-              "deprecationReason": null
+              "isDeprecated": true,
+              "deprecationReason": "Plain text token has been masked for security reasons"
             },
             {
               "name": "updatedAt",

doc/api/graphql/reference/index.md

@@ -370,10 +370,10 @@ Autogenerated return type of EpicTreeReorder
 | ---   |  ---- | ----------  |
 | `id` | ID! | Internal ID of the Grafana integration |
 | `grafanaUrl` | String! | Url for the Grafana host for the Grafana integration |
-| `token` | String! | API token for the Grafana integration |
 | `enabled` | Boolean! | Indicates whether Grafana integration is enabled |
 | `createdAt` | Time! | Timestamp of the issue's creation |
 | `updatedAt` | Time! | Timestamp of the issue's last activity |
+| `token` | String! | API token for the Grafana integration. Field is permanently masked. |
 ## Group

doc/user/asciidoc.md

@@ -221,6 +221,11 @@ include::basics.adoc[]
 include::https://example.org/installation.adoc[]
 ```
+To guarantee good system performance and prevent malicious documents causing
+problems, GitLab enforces a **maximum limit** on the number of include directives
+processed in any one document. Currently a total of 32 documents can be
+included, a number that is inclusive of transitive dependencies.
+
 ### Blocks
 ```asciidoc

lib/api/files.rb

@@ -127,6 +127,7 @@ module API
       get ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do
         assign_file_vars!
+        no_cache_headers
         set_http_headers(blob_data)
         send_git_blob @repo, @blob

lib/api/helpers.rb

@@ -258,11 +258,21 @@ module API
     end
     def require_gitlab_workhorse!
+      verify_workhorse_api!
+
       unless env['HTTP_GITLAB_WORKHORSE'].present?
         forbidden!('Request should be executed via GitLab Workhorse')
       end
     end
+    def verify_workhorse_api!
+      Gitlab::Workhorse.verify_api_request!(request.headers)
+    rescue => e
+      Gitlab::ErrorTracking.track_exception(e)
+
+      forbidden!
+    end
+
     def require_pages_enabled!
       not_found! unless user_project.pages_available?
     end

lib/api/helpers/headers_helpers.rb

@@ -3,6 +3,8 @@
 module API
   module Helpers
     module HeadersHelpers
+      include Gitlab::NoCacheHeaders
+
       def set_http_headers(header_data)
         header_data.each do |key, value|
           if value.is_a?(Enumerable)
@@ -12,6 +14,12 @@ module API
           header "X-Gitlab-#{key.to_s.split('_').collect(&:capitalize).join('-')}", value.to_s
         end
       end
+
+      def no_cache_headers
+        DEFAULT_GITLAB_NO_CACHE_HEADERS.each do |k, v|
+          header k, v
+        end
+      end
     end
   end
 end

lib/banzai/reference_parser/base_parser.rb

@@ -201,12 +201,14 @@ module Banzai
         gather_references(nodes)
       end
-      # Gathers the references for the given HTML nodes.
+      # Gathers the references for the given HTML nodes.  Returns visible
+      # references and a list of nodes which are not visible to the user
       def gather_references(nodes)
         nodes = nodes_user_can_reference(current_user, nodes)
-        nodes = nodes_visible_to_user(current_user, nodes)
-        referenced_by(nodes)
+        visible = nodes_visible_to_user(current_user, nodes)
+        not_visible = nodes - visible
+        { visible: referenced_by(visible), not_visible: not_visible }
       end
       # Returns a Hash containing the projects for a given list of HTML nodes.

lib/gitlab/asciidoc.rb

@@ -11,6 +11,7 @@ module Gitlab
   # the resulting HTML through HTML pipeline filters.
   module Asciidoc
     MAX_INCLUDE_DEPTH = 5
+    MAX_INCLUDES = 32
     DEFAULT_ADOC_ATTRS = {
         'showtitle' => true,
         'sectanchors' => true,
@@ -40,6 +41,7 @@ module Gitlab
                         extensions: extensions }
       context[:pipeline] = :ascii_doc
+      context[:max_includes] = [MAX_INCLUDES, context[:max_includes]].compact.min
       plantuml_setup

lib/gitlab/asciidoc/include_processor.rb

@@ -14,6 +14,8 @@ module Gitlab
         @context = context
         @repository = context[:repository] || context[:project].try(:repository)
+        @max_includes = context[:max_includes].to_i
+        @included = []
         # Note: Asciidoctor calls #freeze on extensions, so we can't set new
         # instance variables after initialization.
@@ -28,8 +30,11 @@ module Gitlab
       def include_allowed?(target, reader)
         doc = reader.document
-        return false if doc.attributes.fetch('max-include-depth').to_i < 1
+        max_include_depth = doc.attributes.fetch('max-include-depth').to_i
+
+        return false if max_include_depth < 1
         return false if target_uri?(target)
+        return false if included.size >= max_includes
         true
       end
@@ -62,7 +67,7 @@ module Gitlab
       private
-      attr_accessor :context, :repository, :cache
+      attr_reader :context, :repository, :cache, :max_includes, :included
       # Gets a Blob at a path for a specific revision.
       # This method will check that the Blob exists and contains readable text.
@@ -77,6 +82,8 @@ module Gitlab
         raise 'Blob not found' unless blob
         raise 'File is not readable' unless blob.readable_text?
+        included << filename
+
         blob
       end

lib/gitlab/no_cache_headers.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module NoCacheHeaders
+    DEFAULT_GITLAB_NO_CACHE_HEADERS = {
+      'Cache-Control' => "#{ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL}, no-store, no-cache",
+      'Pragma'        => 'no-cache', # HTTP 1.0 compatibility
+      'Expires'       => 'Fri, 01 Jan 1990 00:00:00 GMT'
+    }.freeze
+
+    def no_cache_headers
+      raise "#no_cache_headers is not implemented for this object"
+    end
+  end
+end

lib/gitlab/reference_extractor.rb

@@ -6,11 +6,16 @@ module Gitlab
     REFERABLES = %i(user issue label milestone mentioned_user mentioned_group mentioned_project
                     merge_request snippet commit commit_range directly_addressed_user epic).freeze
     attr_accessor :project, :current_user, :author
+    # This counter is increased by a number of references filtered out by
+    # banzai reference exctractor. Note that this counter is stateful and
+    # not idempotent and is increased whenever you call `references`.
+    attr_reader :stateful_not_visible_counter
     def initialize(project, current_user = nil)
       @project = project
       @current_user = current_user
       @references = {}
+      @stateful_not_visible_counter = 0
       super()
     end
@@ -20,11 +25,15 @@ module Gitlab
     end
     def references(type)
-      super(type, project, current_user)
+      refs = super(type, project, current_user)
+      @stateful_not_visible_counter += refs[:not_visible].count
+
+      refs[:visible]
     end
     def reset_memoized_values
       @references = {}
+      @stateful_not_visible_counter = 0
       super()
     end

package.json

@@ -94,6 +94,7 @@
     "jszip": "^3.1.3",
     "jszip-utils": "^0.0.2",
     "katex": "^0.10.0",
+    "lodash": "^4.17.15",
     "marked": "^0.3.12",
     "mermaid": "^8.4.5",
     "monaco-editor": "^0.18.1",

qa/qa/specs/features/api/3_create/repository/files_spec.rb

@@ -59,5 +59,48 @@ module QA
         a_hash_including(message: '202 Accepted')
       )
     end
+
+    describe 'raw file access' do
+      let(:svg_file) do
+        <<-SVG
+          <?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"/>
+            <script type="text/javascript">
+               alert("surprise");
+            </script>
+          </svg>
+        SVG
+      end
+
+      it 'sets no-cache headers as expected' do
+        create_project_request = Runtime::API::Request.new(@api_client, '/projects')
+        post create_project_request.url, path: project_name, name: project_name
+
+        create_file_request = Runtime::API::Request.new(@api_client, "/projects/#{sanitized_project_path}/repository/files/test.svg")
+        post create_file_request.url, branch: 'master', content: svg_file, commit_message: 'Add test.svg'
+
+        get_file_request = Runtime::API::Request.new(@api_client, "/projects/#{sanitized_project_path}/repository/files/test.svg/raw", ref: 'master')
+
+        3.times do
+          response = get get_file_request.url
+
+          # Subsequent responses aren't cached, so headers should match from
+          #   request to request, especially a 200 response rather than a 304
+          #   (indicating a cached response.) Further, :content_disposition
+          #   should include `attachment` for all responses.
+          #
+          expect(response.headers[:cache_control]).to include("no-store")
+          expect(response.headers[:cache_control]).to include("no-cache")
+          expect(response.headers[:pragma]).to eq("no-cache")
+          expect(response.headers[:expires]).to eq("Fri, 01 Jan 1990 00:00:00 GMT")
+          expect(response.headers[:content_disposition]).to include("attachment")
+          expect(response.headers[:content_disposition]).not_to include("inline")
+          expect(response.headers[:content_type]).to include("image/svg+xml")
+        end
+      end
+    end
   end
 end

spec/helpers/projects_helper_spec.rb

@@ -935,14 +935,14 @@ describe ProjectsHelper do
       helper.instance_variable_set(:@project, project)
     end
-    subject { helper.grafana_integration_token }
+    subject { helper.grafana_integration_masked_token }
     it { is_expected.to eq(nil) }
     context 'grafana integration exists' do
       let!(:grafana_integration) { create(:grafana_integration, project: project) }
-      it { is_expected.to eq(grafana_integration.token) }
+      it { is_expected.to eq(grafana_integration.masked_token) }
     end
   end

spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb

@@ -19,7 +19,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do
         it 'returns empty array' do
           link['data-group'] = project.group.id.to_s
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 1)
         end
       end
@@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do
         end
         it 'returns groups' do
-          expect(subject.gather_references([link])).to eq([group])
+          expect_gathered_references(subject.gather_references([link]), [group], 0)
         end
       end
@@ -38,7 +38,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do
         it 'returns an empty Array' do
           link['data-group'] = 'test-non-existing'
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 1)
         end
       end
     end

spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb

@@ -19,7 +19,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do
         it 'returns empty Array' do
           link['data-project'] = project.id.to_s
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 1)
         end
       end
@@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do
         end
         it 'returns an Array of referenced projects' do
-          expect(subject.gather_references([link])).to eq([project])
+          expect_gathered_references(subject.gather_references([link]), [project], 0)
         end
       end
@@ -38,7 +38,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do
         it 'returns an empty Array' do
           link['data-project'] = 'inexisting-project-id'
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 1)
         end
       end
     end

spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb

@@ -22,7 +22,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do
         end
         it 'returns empty list of users' do
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 0)
         end
       end
     end
@@ -35,7 +35,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do
         end
         it 'returns empty list of users' do
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 0)
         end
       end
     end
@@ -44,7 +44,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do
       it 'returns an Array of users' do
         link['data-user'] = user.id.to_s
-        expect(subject.referenced_by([link])).to eq([user])
+        expect_gathered_references(subject.gather_references([link]), [user], 0)
       end
     end
   end

spec/lib/banzai/reference_parser/project_parser_spec.rb

@@ -17,7 +17,7 @@ describe Banzai::ReferenceParser::ProjectParser do
         it 'returns an Array of projects' do
           link['data-project'] = project.id.to_s
-          expect(subject.gather_references([link])).to eq([project])
+          expect_gathered_references(subject.gather_references([link]), [project], 0)
         end
       end
@@ -25,7 +25,7 @@ describe Banzai::ReferenceParser::ProjectParser do
         it 'returns an empty Array' do
           link['data-project'] = ''
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 1)
         end
       end
@@ -35,7 +35,7 @@ describe Banzai::ReferenceParser::ProjectParser do
           link['data-project'] = private_project.id.to_s
-          expect(subject.gather_references([link])).to eq([])
+          expect_gathered_references(subject.gather_references([link]), [], 1)
         end
         it 'returns an Array when authorized' do
@@ -43,7 +43,7 @@ describe Banzai::ReferenceParser::ProjectParser do
           link['data-project'] = private_project.id.to_s
-          expect(subject.gather_references([link])).to eq([private_project])
+          expect_gathered_references(subject.gather_references([link]), [private_project], 0)
         end
       end
     end