New upstream version 13.5.6

CHANGELOG.md

@@ -2,6 +2,19 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 13.5.6 (2021-01-07)
+
+### Security (7 changes)
+
+- Forbid public cache for private repos.
+- Deny implicit flow for confidential apps.
+- Update NuGet regular expression to protect against ReDoS.
+- Fix regular expression backtracking issue in package name validation.
+- Upgrade GitLab Pages to 1.28.2.
+- Update trusted OAuth applications to set them as confidential.
+- Upgrade Workhorse to 8.51.2.
+
+
 ## 13.5.5 (2020-12-07)
 
 ### Security (10 changes)

GITALY_SERVER_VERSION

@@ -1 +1 @@
-13.5.5
+13.5.6

GITLAB_PAGES_VERSION

@@ -1 +1 @@
-1.28.0
+1.28.2

GITLAB_WORKHORSE_VERSION

@@ -1 +1 @@
-8.51.0
+8.51.2

VERSION

@@ -1 +1 @@
-13.5.5
+13.5.6

app/controllers/oauth/authorizations_controller.rb

@@ -24,6 +24,17 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
     end
   end
 
+  def create
+    # Confidential apps require the client_secret to be sent with the request.
+    # Doorkeeper allows implicit grant flow requests (response_type=token) to
+    # work without client_secret regardless of the confidential setting.
+    if pre_auth.authorizable? && pre_auth.response_type == 'token' && pre_auth.client.application.confidential
+      render "doorkeeper/authorizations/error"
+    else
+      super
+    end
+  end
+
   private
 
   def verify_confirmed_email!

app/controllers/projects/raw_controller.rb

@@ -20,7 +20,7 @@ class Projects::RawController < Projects::ApplicationController
   def show
     @blob = @repository.blob_at(@commit.id, @path)
 
-    send_blob(@repository, @blob, inline: (params[:inline] != 'false'), allow_caching: @project.public?)
+    send_blob(@repository, @blob, inline: (params[:inline] != 'false'), allow_caching: Guest.can?(:download_code, @project))
   end
 
   private

app/controllers/projects/repositories_controller.rb

@@ -51,7 +51,7 @@ class Projects::RepositoriesController < Projects::ApplicationController
   end
 
   def set_cache_headers
-    expires_in cache_max_age(archive_metadata['CommitId']), public: project.public?
+    expires_in cache_max_age(archive_metadata['CommitId']), public: Guest.can?(:download_code, project)
     fresh_when(etag: archive_metadata['ArchivePath'])
   end
 

db/migrate/20201222151823_update_trusted_apps_to_confidential.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class UpdateTrustedAppsToConfidential < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'tmp_index_oauth_applications_on_id_where_trusted'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :oauth_applications, :id, where: 'trusted = true', name: INDEX_NAME
+
+    execute('UPDATE oauth_applications SET confidential = true WHERE trusted = true')
+  end
+
+  def down
+    # We won't be able to tell which trusted applications weren't confidential before the migration
+    # and setting all trusted applications are not confidential would introduce security issues
+
+    remove_concurrent_index_by_name :oauth_applications, INDEX_NAME
+  end
+end

db/schema_migrations/20201222151823

@@ -0,0 +1 @@
+d3af120a74b4c55345ac7fb524395251cd3c1b3cd9685f711196a134f427845c

db/structure.sql

@@ -21962,6 +21962,8 @@ CREATE INDEX tmp_idx_index_issues_with_outdate_blocking_count ON issues USING bt
 
 CREATE INDEX tmp_index_for_email_unconfirmation_migration ON emails USING btree (id) WHERE (confirmed_at IS NOT NULL);
 
+CREATE INDEX tmp_index_oauth_applications_on_id_where_trusted ON oauth_applications USING btree (id) WHERE (trusted = true);
+
 CREATE UNIQUE INDEX unique_merge_request_metrics_by_merge_request_id ON merge_request_metrics USING btree (merge_request_id);
 
 CREATE UNIQUE INDEX users_security_dashboard_projects_unique_index ON users_security_dashboard_projects USING btree (project_id, user_id);

lib/api/nuget_packages.rb

@@ -11,7 +11,7 @@ module API
     helpers ::API::Helpers::Packages::BasicAuthHelpers
 
     POSITIVE_INTEGER_REGEX = %r{\A[1-9]\d*\z}.freeze
-    NON_NEGATIVE_INTEGER_REGEX = %r{\A0|[1-9]\d*\z}.freeze
+    NON_NEGATIVE_INTEGER_REGEX = %r{\A(0|[1-9]\d*)\z}.freeze
 
     PACKAGE_FILENAME = 'package.nupkg'
 

lib/gitlab/regex.rb

@@ -23,7 +23,18 @@ module Gitlab
       end
 
       def package_name_regex
-        @package_name_regex ||= %r{\A\@?(([\w\-\.\+]*)\/)*([\w\-\.]+)@?(([\w\-\.\+]*)\/)*([\w\-\.]*)\z}.freeze
+        @package_name_regex ||=
+          %r{
+              \A\@?
+              (?> # atomic group to prevent backtracking
+                (([\w\-\.\+]*)\/)*([\w\-\.]+)
+              )
+              @?
+              (?> # atomic group to prevent backtracking
+                (([\w\-\.\+]*)\/)*([\w\-\.]*)
+              )
+              \z
+            }x.freeze
       end
 
       def maven_file_name_regex

lib/gitlab/usage_data_counters/hll_redis_counter.rb

@@ -180,12 +180,10 @@ module Gitlab
         end
 
         def weekly_redis_keys(events:, start_date:, end_date:)
-          weeks = end_date.to_date.cweek - start_date.to_date.cweek
-          weeks = 1 if weeks == 0
-
-          (0..(weeks - 1)).map do |week_increment|
-            events.map { |event| redis_key(event, start_date + week_increment * 7.days) }
-          end.flatten
+          end_date = end_date.end_of_week - 1.week
+          (start_date.to_date..end_date.to_date).map do |date|
+            events.map { |event| redis_key(event, date) }
+          end.flatten.uniq
         end
       end
     end

spec/controllers/oauth/authorizations_controller_spec.rb

@@ -95,6 +95,20 @@ RSpec.describe Oauth::AuthorizationsController do
     subject { post :create, params: params }
 
     include_examples 'OAuth Authorizations require confirmed user'
+
+    context 'when application is confidential' do
+      before do
+        application.update(confidential: true)
+        params[:response_type] = 'token'
+      end
+
+      it 'does not allow the implicit flow' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template('doorkeeper/authorizations/error')
+      end
+    end
   end
 
   describe 'DELETE #destroy' do

spec/controllers/projects/raw_controller_spec.rb

@@ -225,6 +225,25 @@ RSpec.describe Projects::RawController do
         end
       end
     end
+
+    describe 'caching' do
+      def request_file
+        get(:show, params: { namespace_id: project.namespace, project_id: project, id: 'master/README.md' })
+      end
+
+      context 'when a public project has private repo' do
+        let(:project) { create(:project, :public, :repository, :repository_private) }
+        let(:user) { create(:user, maintainer_projects: [project]) }
+
+        it 'does not set public caching header' do
+          sign_in user
+          request_file
+
+          expect(response.cache_control[:public]).to eq(false)
+          expect(response.cache_control[:max_age]).to eq(60)
+        end
+      end
+    end
   end
 
   def execute_raw_requests(requests:, project:, file_path:, **params)

spec/controllers/projects/repositories_controller_spec.rb

@@ -135,6 +135,18 @@ RSpec.describe Projects::RepositoriesController do
             expect(response.header['ETag']).to be_present
             expect(response.header['Cache-Control']).to include('max-age=60, public')
           end
+
+          context 'and repo is private' do
+            let(:project) { create(:project, :repository, :public, :repository_private) }
+
+            it 'sets appropriate caching headers' do
+              get_archive
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response.header['ETag']).to be_present
+              expect(response.header['Cache-Control']).to include('max-age=60, private')
+            end
+          end
         end
 
         context 'when ref is a commit SHA' do

spec/lib/gitlab/regex_spec.rb

@@ -283,6 +283,12 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('my package name') }
     it { is_expected.not_to match('!!()()') }
     it { is_expected.not_to match("..\n..\foo") }
+
+    it 'has no backtracking issue' do
+      Timeout.timeout(1) do
+        expect(subject).not_to match("-" * 50000 + ";")
+      end
+    end
   end
 
   describe '.maven_file_name_regex' do

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -210,6 +210,50 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
       context 'when no slot is set' do
         it { expect(described_class.unique_events(event_names: no_slot, start_date: 7.days.ago, end_date: Date.current)).to eq(1) }
       end
+
+      context 'when data crosses into new year' do
+        it 'does not raise error' do
+          expect { described_class.unique_events(event_names: [weekly_event], start_date: DateTime.parse('2020-12-26'), end_date: DateTime.parse('2021-02-01')) }
+            .not_to raise_error
+        end
+      end
+    end
+  end
+
+  describe '.weekly_redis_keys' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:weekly_event) { 'g_compliance_dashboard' }
+    let(:redis_event) { described_class.send(:event_for, weekly_event) }
+
+    subject(:weekly_redis_keys) { described_class.send(:weekly_redis_keys, events: [redis_event], start_date: DateTime.parse(start_date), end_date: DateTime.parse(end_date)) }
+
+    where(:start_date, :end_date, :keys) do
+      '2020-12-21' | '2020-12-21' | []
+      '2020-12-21' | '2020-12-20' | []
+      '2020-12-21' | '2020-11-21' | []
+      '2021-01-01' | '2020-12-28' | []
+      '2020-12-21' | '2020-12-28' | ['g_{compliance}_dashboard-2020-52']
+      '2020-12-21' | '2021-01-01' | ['g_{compliance}_dashboard-2020-52']
+      '2020-12-27' | '2021-01-01' | ['g_{compliance}_dashboard-2020-52']
+      '2020-12-26' | '2021-01-04' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53']
+      '2020-12-26' | '2021-01-11' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53', 'g_{compliance}_dashboard-2021-01']
+      '2020-12-26' | '2021-01-17' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53', 'g_{compliance}_dashboard-2021-01']
+      '2020-12-26' | '2021-01-18' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53', 'g_{compliance}_dashboard-2021-01', 'g_{compliance}_dashboard-2021-02']
+    end
+
+    with_them do
+      it "returns the correct keys" do
+        expect(subject).to match(keys)
+      end
+    end
+
+    it 'returns 1 key for last for week' do
+      expect(described_class.send(:weekly_redis_keys, events: [redis_event], start_date: 7.days.ago.to_date, end_date: Date.current).size).to eq 1
+    end
+
+    it 'returns 4 key for last for weeks' do
+      expect(described_class.send(:weekly_redis_keys, events: [redis_event], start_date: 4.weeks.ago.to_date, end_date: Date.current).size).to eq 4
     end
   end
 

spec/lib/gitlab/usage_data_spec.rb

@@ -50,6 +50,32 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
 
       expect { described_class.uncached_data }.to raise_error('Stopped calculating recorded_at')
     end
+
+    context 'when generating usage ping in critical weeks' do
+      it 'does not raise error when generated in last week of the year' do
+        travel_to(DateTime.parse('2020-12-29')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      it 'does not raise error when generated in first week of the year' do
+        travel_to(DateTime.parse('2021-01-01')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      it 'does not raise error when generated in second week of the year' do
+        travel_to(DateTime.parse('2021-01-07')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      it 'does not raise error when generated in 3rd week of the year' do
+        travel_to(DateTime.parse('2021-01-14')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+    end
   end
 
   describe 'usage_activity_by_stage_package' do

spec/support/shared_examples/lib/gitlab/usage_data_counters/incident_management_activity_shared_examples.rb

@@ -3,8 +3,8 @@
 RSpec.shared_examples 'an incident management tracked event' do |event|
   describe ".track_event", :clean_gitlab_redis_shared_state do
     let(:counter) { Gitlab::UsageDataCounters::HLLRedisCounter }
-    let(:start_time) { 1.minute.ago }
-    let(:end_time) { 1.minute.from_now }
+    let(:start_time) { 1.week.ago }
+    let(:end_time) { 1.week.from_now }
 
     it "tracks the event using redis" do
       # Allow other subsequent calls