Merge remote-tracking branch 'dev/12-2-stable' into 12-2-stable

CHANGELOG.md

@@ -2,6 +2,26 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
+## 12.2.9
+
+### Security (14 changes)
+
+- Standardize error response when route is missing.
+- Do not display project labels that are not visible for user accessing group labels.
+- Show cross-referenced label and milestones in issues' activities only to authorized users.
+- Analyze incoming GraphQL queries and check for recursion.
+- Disallow unprivileged users from commenting on private repository commits.
+- Don't allow maintainers of a target project to delete the source branch of a merge request from a fork.
+- Require Maintainer permission on group where project is transferred to.
+- Don't leak private members in project member autocomplete suggestions.
+- Return 404 on LFS request if project doesn't exist.
+- Mask sentry auth token in Error Tracking dashboard.
+- Fixes a Open Redirect issue in `InternalRedirect`.
+- Sanitize search text to prevent XSS.
+- Sanitize all wiki markup formats with GitLab sanitization pipelines.
+- Fix stored XSS issue for grafana_url.
+
+
 ## 12.2.8
 - No changes.

VERSION

-12.2.8
+12.2.9

app/assets/javascripts/project_find_file.js

@@ -5,6 +5,7 @@ import fuzzaldrinPlus from 'fuzzaldrin-plus';
 import axios from '~/lib/utils/axios_utils';
 import flash from '~/flash';
 import { __ } from '~/locale';
+import sanitize from 'sanitize-html';
 // highlight text(awefwbwgtc -> <b>a</b>wefw<b>b</b>wgt<b>c</b> )
 const highlighter = function(element, text, matches) {
@@ -75,7 +76,7 @@ export default class ProjectFindFile {
   findFile() {
     var result, searchText;
-    searchText = this.inputElement.val();
+    searchText = sanitize(this.inputElement.val());
     result =
       searchText.length > 0 ? fuzzaldrinPlus.filter(this.filePaths, searchText) : this.filePaths;
     return this.renderList(result, searchText);

app/controllers/application_controller.rb

@@ -14,7 +14,7 @@ class ApplicationController < ActionController::Base
   include SessionlessAuthentication
   include ConfirmEmailWarning
-  before_action :authenticate_user!
+  before_action :authenticate_user!, except: [:route_not_found]
   before_action :enforce_terms!, if: :should_enforce_terms?
   before_action :validate_user_service_ticket!
   before_action :check_password_expiration
@@ -92,7 +92,9 @@ class ApplicationController < ActionController::Base
     if current_user
       not_found
     else
-      authenticate_user!
+      store_location_for(:user, request.fullpath) unless request.xhr?
+
+      redirect_to new_user_session_path, alert: I18n.t('devise.failure.unauthenticated')
     end
   end

app/controllers/concerns/internal_redirect.rb

@@ -6,7 +6,7 @@ module InternalRedirect
   def safe_redirect_path(path)
     return unless path
     # Verify that the string starts with a `/` and a known route character.
-    return unless path =~ %r{^/[-\w].*$}
+    return unless path =~ %r{\A/[-\w].*\z}
     uri = URI(path)
     # Ignore anything path of the redirect except for the path, querystring and,

app/controllers/concerns/lfs_request.rb

@@ -34,6 +34,7 @@ module LfsRequest
   end
   def lfs_check_access!
+    return render_lfs_not_found unless project
     return if download_request? && lfs_download_access?
     return if upload_request? && lfs_upload_access?

app/finders/labels_finder.rb

@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder
       end
       label_ids << Label.where(group_id: projects.group_ids)
-      label_ids << Label.where(project_id: projects.select(:id)) unless only_group_labels?
+      label_ids << Label.where(project_id: ids_user_can_read_labels(projects)) unless only_group_labels?
     end
     label_ids
@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder
       groups.select { |group| authorized_to_read_labels?(group) }
     end
   end
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def ids_user_can_read_labels(projects)
+    Project.where(id: projects.select(:id)).ids_with_issuables_available_for(current_user)
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
 end

app/graphql/gitlab_schema.rb

@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema
   use Gitlab::Graphql::GenericTracing
   query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
-
-  query(Types::QueryType)
-
-  default_max_page_size 100
+  query_analyzer Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer.new
   max_complexity DEFAULT_MAX_COMPLEXITY
   max_depth DEFAULT_MAX_DEPTH
-  mutation(Types::MutationType)
+  query Types::QueryType
+  mutation Types::MutationType
+
+  default_max_page_size 100
   class << self
     def multiplex(queries, **kwargs)

app/helpers/markup_helper.rb

@@ -133,15 +133,7 @@ module MarkupHelper
       issuable_state_filter_enabled: true
     )
-    html =
-      case wiki_page.format
-      when :markdown
-        markdown_unsafe(text, context)
-      when :asciidoc
-        asciidoc_unsafe(text)
-      else
-        wiki_page.formatted_content.html_safe
-      end
+    html = markup_unsafe(wiki_page.path, text, context)
     prepare_for_rendering(html, context)
   end

app/models/application_setting.rb

@@ -7,6 +7,13 @@ class ApplicationSetting < ApplicationRecord
   include IgnorableColumn
   include ChronicDurationAttribute
+  GRAFANA_URL_RULES = {
+    allow_localhost: true,
+    allow_local_network: true,
+    enforce_sanitization: true,
+    require_absolute: false
+  }.freeze
+
   add_authentication_token_field :runners_registration_token, encrypted: -> { Feature.enabled?(:application_settings_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
   add_authentication_token_field :health_check_access_token
@@ -55,6 +62,11 @@ class ApplicationSetting < ApplicationRecord
             allow_nil: false,
             qualified_domain_array: true
+  validates :grafana_url,
+            allow_blank: true,
+            allow_nil: true,
+            addressable_url: GRAFANA_URL_RULES
+
   validates :session_expire_delay,
             presence: true,
             numericality: { only_integer: true, greater_than_or_equal_to: 0 }
@@ -72,7 +84,6 @@ class ApplicationSetting < ApplicationRecord
   validates :after_sign_out_path,
             allow_blank: true,
             addressable_url: true
-
   validates :admin_notification_email,
             devise_email: true,
             allow_blank: true
@@ -303,6 +314,14 @@ class ApplicationSetting < ApplicationRecord
     current_without_cache
   end
+  def grafana_url
+    if Gitlab::UrlBlocker.blocked_url?(self[:grafana_url], GRAFANA_URL_RULES)
+      ApplicationSetting.column_defaults["grafana_url"]
+    else
+      self[:grafana_url]
+    end
+  end
+
   # By default, the backend is Rails.cache, which uses
   # ActiveSupport::Cache::RedisStore. Since loading ApplicationSetting
   # can cause a significant amount of load on Redis, let's cache it in

app/models/concerns/mentionable/reference_regexes.rb

@@ -13,7 +13,9 @@ module Mentionable
     def self.other_patterns
       [
         Commit.reference_pattern,
-        MergeRequest.reference_pattern
+        MergeRequest.reference_pattern,
+        Label.reference_pattern,
+        Milestone.reference_pattern
       ]
     end

app/models/discussion.rb

@@ -16,6 +16,7 @@ class Discussion
             :commit_id,
             :for_commit?,
             :for_merge_request?,
+            :noteable_ability_name,
             :to_ability_name,
             :editable?,
             :visible_for?,

app/models/member.rb

@@ -8,6 +8,7 @@ class Member < ApplicationRecord
   include Gitlab::Access
   include Presentable
   include Gitlab::Utils::StrongMemoize
+  include FromUnion
   attr_accessor :raw_invite_token

app/models/merge_request.rb

@@ -67,6 +67,14 @@ class MergeRequest < ApplicationRecord
   has_many :merge_request_assignees
   has_many :assignees, class_name: "User", through: :merge_request_assignees
+  KNOWN_MERGE_PARAMS = [
+    :auto_merge_strategy,
+    :should_remove_source_branch,
+    :force_remove_source_branch,
+    :commit_message,
+    :squash_commit_message,
+    :sha
+  ].freeze
   serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
   after_create :ensure_merge_request_diff

app/models/milestone.rb

@@ -254,6 +254,10 @@ class Milestone < ApplicationRecord
     group || project
   end
+  def to_ability_name
+    model_name.singular
+  end
+
   def group_milestone?
     group_id.present?
   end

app/models/note.rb

@@ -353,6 +353,10 @@ class Note < ApplicationRecord
   end
   def to_ability_name
+    model_name.singular
+  end
+
+  def noteable_ability_name
     for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore
   end

app/models/project.rb

@@ -581,11 +581,11 @@ class Project < ApplicationRecord
       joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
     end
-    # Returns ids of projects with milestones available for given user
+    # Returns ids of projects with issuables available for given user
     #
-    # Used on queries to find milestones which user can see
-    # For example: Milestone.where(project_id: ids_with_milestone_available_for(user))
-    def ids_with_milestone_available_for(user)
+    # Used on queries to find milestones or labels which user can see
+    # For example: Milestone.where(project_id: ids_with_issuables_available_for(user))
+    def ids_with_issuables_available_for(user)
       with_issues_enabled = with_issues_available_for_user(user).select(:id)
       with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id)
@@ -1223,6 +1223,10 @@ class Project < ApplicationRecord
     end
   end
+  def to_ability_name
+    model_name.singular
+  end
+
   # rubocop: disable CodeReuse/ServiceClass
   def execute_hooks(data, hooks_scope = :push_hooks)
     run_after_commit_or_now do

app/models/system_note_metadata.rb

@@ -10,6 +10,7 @@ class SystemNoteMetadata < ApplicationRecord
     commit cross_reference
     close duplicate
     moved merge
+    label milestone
   ].freeze
   ICON_TYPES = %w[

app/models/wiki_page.rb

@@ -138,6 +138,12 @@ class WikiPage
     @version ||= @page.version
   end
+  def path
+    return unless persisted?
+
+    @path ||= @page.path
+  end
+
   def versions(options = {})
     return [] unless persisted?

app/policies/commit_policy.rb

@@ -4,4 +4,5 @@ class CommitPolicy < BasePolicy
   delegate { @subject.project }
   rule { can?(:download_code) }.enable :read_commit
+  rule { ~can?(:read_commit) }.prevent :create_note
 end