...@@ -2,6 +2,24 @@ ...@@ -2,6 +2,24 @@
documentation](doc/development/changelog.md) for instructions on adding your own documentation](doc/development/changelog.md) for instructions on adding your own
entry. entry.
   
## 12.4.1
### Security (12 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 all wiki markup formats with GitLab sanitization pipelines.
## 12.4.0 ## 12.4.0
   
### Security (14 changes) ### Security (14 changes)
... ...
......
12.4.0 12.4.1
...@@ -16,7 +16,7 @@ class ApplicationController < ActionController::Base ...@@ -16,7 +16,7 @@ class ApplicationController < ActionController::Base
include Gitlab::Tracking::ControllerConcern include Gitlab::Tracking::ControllerConcern
include Gitlab::Experimentation::ControllerConcern include Gitlab::Experimentation::ControllerConcern
before_action :authenticate_user! before_action :authenticate_user!, except: [:route_not_found]
before_action :enforce_terms!, if: :should_enforce_terms? before_action :enforce_terms!, if: :should_enforce_terms?
before_action :validate_user_service_ticket! before_action :validate_user_service_ticket!
before_action :check_password_expiration before_action :check_password_expiration
...@@ -97,7 +97,9 @@ class ApplicationController < ActionController::Base ...@@ -97,7 +97,9 @@ class ApplicationController < ActionController::Base
if current_user if current_user
not_found not_found
else 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
end end
... ...
......
...@@ -6,7 +6,7 @@ module InternalRedirect ...@@ -6,7 +6,7 @@ module InternalRedirect
def safe_redirect_path(path) def safe_redirect_path(path)
return unless path return unless path
# Verify that the string starts with a `/` and a known route character. # 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) uri = URI(path)
# Ignore anything path of the redirect except for the path, querystring and, # Ignore anything path of the redirect except for the path, querystring and,
... ...
......
...@@ -34,6 +34,7 @@ module LfsRequest ...@@ -34,6 +34,7 @@ module LfsRequest
end end
def lfs_check_access! def lfs_check_access!
return render_lfs_not_found unless project
return if download_request? && lfs_download_access? return if download_request? && lfs_download_access?
return if upload_request? && lfs_upload_access? return if upload_request? && lfs_upload_access?
... ...
......
...@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder ...@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder
end end
label_ids << Label.where(group_id: projects.group_ids) 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 end
label_ids label_ids
...@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder ...@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder
groups.select { |group| authorized_to_read_labels?(group) } groups.select { |group| authorized_to_read_labels?(group) }
end end
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 end
...@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema ...@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema
use Gitlab::Graphql::GenericTracing use Gitlab::Graphql::GenericTracing
query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
query_analyzer Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer.new
query(Types::QueryType)
default_max_page_size 100
max_complexity DEFAULT_MAX_COMPLEXITY max_complexity DEFAULT_MAX_COMPLEXITY
max_depth DEFAULT_MAX_DEPTH max_depth DEFAULT_MAX_DEPTH
mutation(Types::MutationType) query Types::QueryType
mutation Types::MutationType
default_max_page_size 100
class << self class << self
def multiplex(queries, **kwargs) def multiplex(queries, **kwargs)
... ...
......
...@@ -133,15 +133,7 @@ module MarkupHelper ...@@ -133,15 +133,7 @@ module MarkupHelper
issuable_state_filter_enabled: true issuable_state_filter_enabled: true
) )
html = html = markup_unsafe(wiki_page.path, text, context)
case wiki_page.format
when :markdown
markdown_unsafe(text, context)
when :asciidoc
asciidoc_unsafe(text)
else
wiki_page.formatted_content.html_safe
end
prepare_for_rendering(html, context) prepare_for_rendering(html, context)
end end
... ...
......
...@@ -13,7 +13,9 @@ module Mentionable ...@@ -13,7 +13,9 @@ module Mentionable
def self.other_patterns def self.other_patterns
[ [
Commit.reference_pattern, Commit.reference_pattern,
MergeRequest.reference_pattern MergeRequest.reference_pattern,
Label.reference_pattern,
Milestone.reference_pattern
] ]
end end
... ...
......
...@@ -16,6 +16,7 @@ class Discussion ...@@ -16,6 +16,7 @@ class Discussion
:commit_id, :commit_id,
:for_commit?, :for_commit?,
:for_merge_request?, :for_merge_request?,
:noteable_ability_name,
:to_ability_name, :to_ability_name,
:editable?, :editable?,
:visible_for?, :visible_for?,
... ...
......
...@@ -8,6 +8,7 @@ class Member < ApplicationRecord ...@@ -8,6 +8,7 @@ class Member < ApplicationRecord
include Gitlab::Access include Gitlab::Access
include Presentable include Presentable
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include FromUnion
attr_accessor :raw_invite_token attr_accessor :raw_invite_token
... ...
......
...@@ -69,6 +69,14 @@ class MergeRequest < ApplicationRecord ...@@ -69,6 +69,14 @@ class MergeRequest < ApplicationRecord
has_many :merge_request_assignees has_many :merge_request_assignees
has_many :assignees, class_name: "User", through: :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 serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
after_create :ensure_merge_request_diff after_create :ensure_merge_request_diff
... ...
......
...@@ -261,6 +261,10 @@ class Milestone < ApplicationRecord ...@@ -261,6 +261,10 @@ class Milestone < ApplicationRecord
group || project group || project
end end
def to_ability_name
model_name.singular
end
def group_milestone? def group_milestone?
group_id.present? group_id.present?
end end
... ...
......
...@@ -361,6 +361,10 @@ class Note < ApplicationRecord ...@@ -361,6 +361,10 @@ class Note < ApplicationRecord
end end
def to_ability_name def to_ability_name
model_name.singular
end
def noteable_ability_name
for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore
end end
... ...
......
...@@ -609,11 +609,11 @@ class Project < ApplicationRecord ...@@ -609,11 +609,11 @@ class Project < ApplicationRecord
joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id) joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
end 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 # Used on queries to find milestones or labels which user can see
# For example: Milestone.where(project_id: ids_with_milestone_available_for(user)) # For example: Milestone.where(project_id: ids_with_issuables_available_for(user))
def ids_with_milestone_available_for(user) def ids_with_issuables_available_for(user)
with_issues_enabled = with_issues_available_for_user(user).select(:id) with_issues_enabled = with_issues_available_for_user(user).select(:id)
with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id) with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id)
...@@ -1260,6 +1260,10 @@ class Project < ApplicationRecord ...@@ -1260,6 +1260,10 @@ class Project < ApplicationRecord
end end
end end
def to_ability_name
model_name.singular
end
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
def execute_hooks(data, hooks_scope = :push_hooks) def execute_hooks(data, hooks_scope = :push_hooks)
run_after_commit_or_now do run_after_commit_or_now do
... ...
......
...@@ -10,6 +10,7 @@ class SystemNoteMetadata < ApplicationRecord ...@@ -10,6 +10,7 @@ class SystemNoteMetadata < ApplicationRecord
commit cross_reference commit cross_reference
close duplicate close duplicate
moved merge moved merge
label milestone
].freeze ].freeze
ICON_TYPES = %w[ ICON_TYPES = %w[
... ...
......
...@@ -134,6 +134,12 @@ class WikiPage ...@@ -134,6 +134,12 @@ class WikiPage
@version ||= @page.version @version ||= @page.version
end end
def path
return unless persisted?
@path ||= @page.path
end
def versions(options = {}) def versions(options = {})
return [] unless persisted? return [] unless persisted?
... ...
......
...@@ -4,4 +4,5 @@ class CommitPolicy < BasePolicy ...@@ -4,4 +4,5 @@ class CommitPolicy < BasePolicy
delegate { @subject.project } delegate { @subject.project }
rule { can?(:download_code) }.enable :read_commit rule { can?(:download_code) }.enable :read_commit
rule { ~can?(:read_commit) }.prevent :create_note
end end
...@@ -131,6 +131,8 @@ class GroupPolicy < BasePolicy ...@@ -131,6 +131,8 @@ class GroupPolicy < BasePolicy
rule { owner | admin }.enable :read_statistics rule { owner | admin }.enable :read_statistics
rule { maintainer & can?(:create_projects) }.enable :transfer_projects
def access_level def access_level
return GroupMember::NO_ACCESS if @user.nil? return GroupMember::NO_ACCESS if @user.nil?
... ...
......
...@@ -15,6 +15,8 @@ class NamespacePolicy < BasePolicy ...@@ -15,6 +15,8 @@ class NamespacePolicy < BasePolicy
end end
rule { personal_project & ~can_create_personal_project }.prevent :create_projects rule { personal_project & ~can_create_personal_project }.prevent :create_projects
rule { (owner | admin) & can?(:create_projects) }.enable :transfer_projects
end end
NamespacePolicy.prepend_if_ee('EE::NamespacePolicy') NamespacePolicy.prepend_if_ee('EE::NamespacePolicy')