From 3dd03a1a19e6b788ec1296044e28f7727e5149a6 Mon Sep 17 00:00:00 2001
From: GitLab Bot
Date: Wed, 29 Mar 2023 23:48:05 +0000
Subject: Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee
---
.../environments/environment_names_finder.rb | 11 +--------
app/models/project_feature.rb | 3 ++-
.../explore/projects/page_out_of_bounds.html.haml | 2 +-
.../environments/environment_names_finder_spec.rb | 26 +++++++++++++++++-----
spec/policies/project_policy_spec.rb | 4 ++--
.../projects/page_out_of_bounds.html.haml_spec.rb | 26 ++++++++++++++++++++++
6 files changed, 53 insertions(+), 19 deletions(-)
create mode 100644 spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb
diff --git a/app/finders/environments/environment_names_finder.rb b/app/finders/environments/environment_names_finder.rb
index d4928f0fc84..ffb689f45e2 100644
--- a/app/finders/environments/environment_names_finder.rb
+++ b/app/finders/environments/environment_names_finder.rb
@@ -32,18 +32,9 @@ module Environments
end
def namespace_environments
- # We assume reporter access is needed for the :read_environment permission
- # here. This expection is also present in
- # IssuableFinder::Params#min_access_level, which is used for filtering out
- # merge requests that don't have the right permissions.
- #
- # We use this approach so we don't need to load every project into memory
- # just to verify if we can see their environments. Doing so would not be
- # efficient, and possibly mess up pagination if certain projects are not
- # meant to be visible.
projects = project_or_group
.all_projects
- .public_or_visible_to_user(current_user, Gitlab::Access::REPORTER)
+ .filter_by_feature_visibility(:environments, current_user)
Environment.for_project(projects)
end
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index 053ccfac050..52e623db7b0 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -36,7 +36,8 @@ class ProjectFeature < ApplicationRecord
merge_requests: Gitlab::Access::REPORTER,
metrics_dashboard: Gitlab::Access::REPORTER,
container_registry: Gitlab::Access::REPORTER,
- package_registry: Gitlab::Access::REPORTER
+ package_registry: Gitlab::Access::REPORTER,
+ environments: Gitlab::Access::REPORTER
}.freeze
PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
diff --git a/app/views/explore/projects/page_out_of_bounds.html.haml b/app/views/explore/projects/page_out_of_bounds.html.haml
index ef5ee2c679e..e13768a3ccb 100644
--- a/app/views/explore/projects/page_out_of_bounds.html.haml
+++ b/app/views/explore/projects/page_out_of_bounds.html.haml
@@ -18,5 +18,5 @@
%h5= _("Maximum page reached")
%p= _("Sorry, you have exceeded the maximum browsable page number. Please use the API to explore further.")
- = render Pajamas::ButtonComponent.new(href: request.params.merge(page: @max_page_number)) do
+ = render Pajamas::ButtonComponent.new(href: safe_params.merge(page: @max_page_number)) do
= _("Back to page %{number}") % { number: @max_page_number }
diff --git a/spec/finders/environments/environment_names_finder_spec.rb b/spec/finders/environments/environment_names_finder_spec.rb
index 438f9e9ea7c..c2336c59119 100644
--- a/spec/finders/environments/environment_names_finder_spec.rb
+++ b/spec/finders/environments/environment_names_finder_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
describe '#execute' do
let!(:group) { create(:group) }
let!(:public_project) { create(:project, :public, namespace: group) }
+ let_it_be_with_reload(:public_project_with_private_environments) { create(:project, :public) }
let!(:private_project) { create(:project, :private, namespace: group) }
let!(:user) { create(:user) }
@@ -14,6 +15,11 @@ RSpec.describe Environments::EnvironmentNamesFinder do
create(:environment, name: 'gprd', project: public_project)
create(:environment, name: 'gprd', project: private_project)
create(:environment, name: 'gcny', project: private_project)
+ create(:environment, name: 'gprivprd', project: public_project_with_private_environments)
+ create(:environment, name: 'gprivstg', project: public_project_with_private_environments)
+
+ public_project_with_private_environments.update!(namespace: group)
+ public_project_with_private_environments.project_feature.update!(environments_access_level: Featurable::PRIVATE)
end
context 'using a group' do
@@ -23,7 +29,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
names = described_class.new(group, user).execute
- expect(names).to eq(%w[gcny gprd gstg])
+ expect(names).to eq(%w[gcny gprd gprivprd gprivstg gstg])
end
end
@@ -33,7 +39,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
names = described_class.new(group, user).execute
- expect(names).to eq(%w[gcny gprd gstg])
+ expect(names).to eq(%w[gcny gprd gprivprd gprivstg gstg])
end
end
@@ -57,8 +63,18 @@ RSpec.describe Environments::EnvironmentNamesFinder do
end
end
+ context 'with a public project reporter which has private environments' do
+ it 'returns environment names for public projects' do
+ public_project_with_private_environments.add_reporter(user)
+
+ names = described_class.new(group, user).execute
+
+ expect(names).to eq(%w[gprd gprivprd gprivstg gstg])
+ end
+ end
+
context 'with a group guest' do
- it 'returns environment names for all public projects' do
+ it 'returns environment names for public projects' do
group.add_guest(user)
names = described_class.new(group, user).execute
@@ -68,7 +84,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
end
context 'with a non-member' do
- it 'returns environment names for all public projects' do
+ it 'returns environment names for only public projects with public environments' do
names = described_class.new(group, user).execute
expect(names).to eq(%w[gprd gstg])
@@ -76,7 +92,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
end
context 'without a user' do
- it 'returns environment names for all public projects' do
+ it 'returns environment names for only public projects with public environments' do
names = described_class.new(group).execute
expect(names).to eq(%w[gprd gstg])
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 3759539677a..383f56b6ef5 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -2068,7 +2068,7 @@ RSpec.describe ProjectPolicy, feature_category: :system_access do
:public | ProjectFeature::ENABLED | :anonymous | true
:public | ProjectFeature::PRIVATE | :maintainer | true
:public | ProjectFeature::PRIVATE | :developer | true
- :public | ProjectFeature::PRIVATE | :guest | true
+ :public | ProjectFeature::PRIVATE | :guest | false
:public | ProjectFeature::PRIVATE | :anonymous | false
:public | ProjectFeature::DISABLED | :maintainer | false
:public | ProjectFeature::DISABLED | :developer | false
@@ -2080,7 +2080,7 @@ RSpec.describe ProjectPolicy, feature_category: :system_access do
:internal | ProjectFeature::ENABLED | :anonymous | false
:internal | ProjectFeature::PRIVATE | :maintainer | true
:internal | ProjectFeature::PRIVATE | :developer | true
- :internal | ProjectFeature::PRIVATE | :guest | true
+ :internal | ProjectFeature::PRIVATE | :guest | false
:internal | ProjectFeature::PRIVATE | :anonymous | false
:internal | ProjectFeature::DISABLED | :maintainer | false
:internal | ProjectFeature::DISABLED | :developer | false
diff --git a/spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb b/spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb
new file mode 100644
index 00000000000..1ace28be5b4
--- /dev/null
+++ b/spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'explore/projects/page_out_of_bounds.html.haml', feature_category: :projects do
+ let(:page_limit) { 10 }
+ let(:unsafe_param) { 'hacked_using_unsafe_param!' }
+
+ before do
+ assign(:max_page_number, page_limit)
+
+ controller.params[:action] = 'index'
+ controller.params[:host] = unsafe_param
+ controller.params[:protocol] = unsafe_param
+ controller.params[:sort] = 'name_asc'
+ end
+
+ it 'removes unsafe params from the link' do
+ render
+
+ href = "/explore/projects?page=#{page_limit}&sort=name_asc"
+ button_text = format(_("Back to page %{number}"), number: page_limit)
+ expect(rendered).to have_link(button_text, href: href)
+ expect(rendered).not_to include(unsafe_param)
+ end
+end
--
cgit v1.2.3
From 56ff640a2f919e9d0e450964081381a8eccef5e4 Mon Sep 17 00:00:00 2001
From: GitLab Bot
Date: Wed, 29 Mar 2023 23:49:36 +0000
Subject: Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee
---
app/finders/notes_finder.rb | 8 +++
app/models/concerns/taskable.rb | 36 +++++++++-----
.../merge_requests/push_options_handler_service.rb | 10 +++-
lib/gitlab/regex.rb | 58 +++++++++++-----------
lib/gitlab/untrusted_regexp.rb | 11 ++++
spec/finders/notes_finder_spec.rb | 20 ++++++++
spec/lib/gitlab/regex_spec.rb | 10 ++--
spec/lib/gitlab/untrusted_regexp_spec.rb | 32 ++++++++++++
spec/models/concerns/taskable_spec.rb | 6 +--
.../push_options_handler_service_spec.rb | 15 ++++++
10 files changed, 155 insertions(+), 51 deletions(-)
diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index c542ffbce7e..81017290f12 100644
--- a/app/finders/notes_finder.rb
+++ b/app/finders/notes_finder.rb
@@ -30,6 +30,7 @@ class NotesFinder
notes = init_collection
notes = since_fetch_at(notes)
notes = notes.with_notes_filter(@params[:notes_filter]) if notes_filter?
+ notes = redact_internal(notes)
sort(notes)
end
@@ -181,6 +182,13 @@ class NotesFinder
notes.order_by(sort)
end
+
+ def redact_internal(notes)
+ subject = @project || target
+ return notes if Ability.allowed?(@current_user, :read_internal_note, subject)
+
+ notes.not_internal
+ end
end
NotesFinder.prepend_mod_with('NotesFinder')
diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb
index f9eba4cc2fe..dee1c820f23 100644
--- a/app/models/concerns/taskable.rb
+++ b/app/models/concerns/taskable.rb
@@ -24,25 +24,37 @@ module Taskable
(\s.+) # followed by whitespace and some text.
}x.freeze
+ ITEM_PATTERN_UNTRUSTED =
+ '^' \
+ '(?:(?:>\s{0,4})*)' \
+ '(?P(?:\s*(?:[-+*]|(?:\d+\.)))+)' \
+ '\s+' \
+ '(?P' \
+ "#{COMPLETE_PATTERN.source}|#{INCOMPLETE_PATTERN.source}" \
+ ')' \
+ '(?P
-
<p> & © Æ Ď
+<p> & © Æ Ď
¾ ℋ ⅆ
∲ ≧̸</p>
@@ -7344,11 +7344,11 @@ stripped in this way:
-
<p><code> b </code></p>
+
<p><code> b </code></p>
@@ -7356,12 +7356,12 @@ stripped in this way:
-
<p><code> </code>
+<p><code> </code>
<code> </code></p>
@@ -7832,11 +7832,11 @@ not part of a [left-flanking delimiter run]:
-
<p>* a *</p>
+
<p>* a *</p>
@@ -9790,7 +9790,7 @@ Other [Unicode whitespace] like non-breaking space doesn't work.
-
[link](/url "title")
+
[link](/url "title")
diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb
index 70535496b12..6f8d34ea387 100644
--- a/lib/api/repositories.rb
+++ b/lib/api/repositories.rb
@@ -203,6 +203,10 @@ module API
render_api_error!("Target project id:#{params[:from_project_id]} is not a fork of project id:#{params[:id]}", 400)
end
+ unless can?(current_user, :read_code, target_project)
+ forbidden!("You don't have access to this fork's parent project")
+ end
+
cache_key = compare_cache_key(current_user, user_project, target_project, declared_params)
cache_action(cache_key, expires_in: 1.minute) do
diff --git a/lib/extracts_ref.rb b/lib/extracts_ref.rb
index dba1aad639c..49c9772f760 100644
--- a/lib/extracts_ref.rb
+++ b/lib/extracts_ref.rb
@@ -5,7 +5,8 @@
# Can be extended for different types of repository object, e.g. Project or Snippet
module ExtractsRef
InvalidPathError = Class.new(StandardError)
-
+ BRANCH_REF_TYPE = 'heads'
+ TAG_REF_TYPE = 'tags'
# Given a string containing both a Git tree-ish, such as a branch or tag, and
# a filesystem path joined by forward slashes, attempts to separate the two.
#
@@ -91,7 +92,7 @@ module ExtractsRef
def ref_type
return unless params[:ref_type].present?
- params[:ref_type] == 'tags' ? 'tags' : 'heads'
+ params[:ref_type] == TAG_REF_TYPE ? TAG_REF_TYPE : BRANCH_REF_TYPE
end
private
@@ -154,4 +155,13 @@ module ExtractsRef
def repository_container
raise NotImplementedError
end
+
+ def ambiguous_ref?(project, ref)
+ return true if project.repository.ambiguous_ref?(ref)
+
+ return false unless ref&.starts_with?('refs/')
+
+ unprefixed_ref = ref.sub(%r{^refs/(heads|tags)/}, '')
+ project.repository.commit(unprefixed_ref).present?
+ end
end
diff --git a/lib/gitlab/unicode.rb b/lib/gitlab/unicode.rb
index b49c5647dab..f291ea1b4ee 100644
--- a/lib/gitlab/unicode.rb
+++ b/lib/gitlab/unicode.rb
@@ -9,6 +9,12 @@ module Gitlab
# https://idiosyncratic-ruby.com/41-proper-unicoding.html
BIDI_REGEXP = /\p{Bidi Control}/.freeze
+ # Regular expression for identifying space characters
+ #
+ # In web browsers space characters can be confused with simple
+ # spaces which may be misleading
+ SPACE_REGEXP = /\p{Space_Separator}/.freeze
+
class << self
# Warning message used to highlight bidi characters in the GUI
def bidi_warning
diff --git a/lib/rouge/formatters/html_gitlab.rb b/lib/rouge/formatters/html_gitlab.rb
index 436739bed12..a7e95a96b8b 100644
--- a/lib/rouge/formatters/html_gitlab.rb
+++ b/lib/rouge/formatters/html_gitlab.rb
@@ -25,7 +25,10 @@ module Rouge
yield %()
line.each do |token, value|
- yield highlight_unicode_control_characters(span(token, value.chomp! || value))
+ value = value.chomp! || value
+ value = replace_space_characters(value)
+
+ yield highlight_unicode_control_characters(span(token, value))
end
yield ellipsis if @ellipsis_indexes.include?(@line_number - 1) && @ellipsis_svg.present?
@@ -42,6 +45,10 @@ module Rouge
%(#{@ellipsis_svg})
end
+ def replace_space_characters(text)
+ text.gsub(Gitlab::Unicode::SPACE_REGEXP, ' ')
+ end
+
def highlight_unicode_control_characters(text)
text.gsub(Gitlab::Unicode::BIDI_REGEXP) do |char|
%(#{char})
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 97b4b0e614e..6da296d5381 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -43539,6 +43539,9 @@ msgstr ""
msgid "The default branch for this project has been changed. Please update your bookmarks."
msgstr ""
+msgid "The default branch of this project clashes with another ref"
+msgstr ""
+
msgid "The dependency list details information about the components used within your project."
msgstr ""
diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb
index 4101bd7f658..4e68ffdda2a 100644
--- a/spec/controllers/admin/hooks_controller_spec.rb
+++ b/spec/controllers/admin/hooks_controller_spec.rb
@@ -59,6 +59,7 @@ RSpec.describe Admin::HooksController do
enable_ssl_verification: false,
url_variables: [
{ key: 'token', value: 'some secret value' },
+ { key: 'baz', value: 'qux' },
{ key: 'foo', value: nil }
]
}
@@ -71,7 +72,7 @@ RSpec.describe Admin::HooksController do
expect(flash[:notice]).to include('was updated')
expect(hook).to have_attributes(hook_params.except(:url_variables))
expect(hook).to have_attributes(
- url_variables: { 'token' => 'some secret value', 'baz' => 'woo' }
+ url_variables: { 'token' => 'some secret value', 'baz' => 'qux' }
)
end
end
diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb
index fca99d37000..7cfbd86cdcb 100644
--- a/spec/controllers/concerns/confirm_email_warning_spec.rb
+++ b/spec/controllers/concerns/confirm_email_warning_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe ConfirmEmailWarning do
+RSpec.describe ConfirmEmailWarning, feature_category: :system_access do
before do
stub_application_setting_enum('email_confirmation_setting', 'soft')
end
@@ -82,6 +82,38 @@ RSpec.describe ConfirmEmailWarning do
it { is_expected.to set_confirm_warning_for(user.email) }
end
end
+
+ context 'when user is being impersonated' do
+ let(:impersonator) { create(:admin) }
+
+ before do
+ allow(controller).to receive(:session).and_return({ impersonator_id: impersonator.id })
+
+ get :index
+ end
+
+ it { is_expected.to set_confirm_warning_for(user.email) }
+
+ context 'when impersonated user email has html in their email' do
+ let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: "malicious@test.com