diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-01-06 19:20:58 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-01-06 19:20:58 +0000 |
commit | e85719b439c7ec3f2f8e3d518776f6dc307bd000 (patch) | |
tree | 67218121fe398f3fe2bd8cb27f2aded535950d01 | |
parent | 30235c34b0a56de6f7acde24e6fba7a0116a2e02 (diff) | |
download | gitlab-ce-e85719b439c7ec3f2f8e3d518776f6dc307bd000.tar.gz |
Add latest changes from gitlab-org/security/gitlab@13-6-stable-ee
-rw-r--r-- | app/controllers/oauth/authorizations_controller.rb | 11 | ||||
-rw-r--r-- | changelogs/unreleased/security-implicit-confidential.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/security-package-regex-dos.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/security-trusted-confidential-apps.yml | 5 | ||||
-rw-r--r-- | db/migrate/20201222151823_update_trusted_apps_to_confidential.rb | 23 | ||||
-rw-r--r-- | db/schema_migrations/20201222151823 | 1 | ||||
-rw-r--r-- | db/structure.sql | 2 | ||||
-rw-r--r-- | lib/gitlab/regex.rb | 13 | ||||
-rw-r--r-- | spec/controllers/oauth/authorizations_controller_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/regex_spec.rb | 6 |
10 files changed, 84 insertions, 1 deletions
diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 6e8686ee90b..ade698baa7f 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/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! diff --git a/changelogs/unreleased/security-implicit-confidential.yml b/changelogs/unreleased/security-implicit-confidential.yml new file mode 100644 index 00000000000..bbf2d95b3fb --- /dev/null +++ b/changelogs/unreleased/security-implicit-confidential.yml @@ -0,0 +1,5 @@ +--- +title: Deny implicit flow for confidential apps +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-package-regex-dos.yml b/changelogs/unreleased/security-package-regex-dos.yml new file mode 100644 index 00000000000..79bec83526d --- /dev/null +++ b/changelogs/unreleased/security-package-regex-dos.yml @@ -0,0 +1,5 @@ +--- +title: Fix regular expression backtracking issue in package name validation +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-trusted-confidential-apps.yml b/changelogs/unreleased/security-trusted-confidential-apps.yml new file mode 100644 index 00000000000..b4f7a9eb448 --- /dev/null +++ b/changelogs/unreleased/security-trusted-confidential-apps.yml @@ -0,0 +1,5 @@ +--- +title: Update trusted OAuth applications to set them as confidential +merge_request: +author: +type: security diff --git a/db/migrate/20201222151823_update_trusted_apps_to_confidential.rb b/db/migrate/20201222151823_update_trusted_apps_to_confidential.rb new file mode 100644 index 00000000000..bcb94c65125 --- /dev/null +++ b/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 diff --git a/db/schema_migrations/20201222151823 b/db/schema_migrations/20201222151823 new file mode 100644 index 00000000000..914e96473a0 --- /dev/null +++ b/db/schema_migrations/20201222151823 @@ -0,0 +1 @@ +d3af120a74b4c55345ac7fb524395251cd3c1b3cd9685f711196a134f427845c
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index f29f9178a26..1ed6ea64eca 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22367,6 +22367,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 vulnerability_feedback_unique_idx ON vulnerability_feedback USING btree (project_id, category, feedback_type, project_fingerprint); diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 4ae6297f6f5..96f2b7570b3 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -27,7 +27,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 diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 23d472f6853..f811f13def8 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index ebb37f45b95..776ca81a338 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -292,6 +292,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 |