diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-10-27 12:41:52 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-10-27 12:41:52 +0000 |
commit | b39ba6a9d46f041f6cff845d3a4a3834ac873869 (patch) | |
tree | 2eb22ce64d4eea81baf6b51fe3ff330d4bdd63a0 | |
parent | ab5ec2e047ff0942db3aab555926bdd07c051087 (diff) | |
download | gitlab-ce-b39ba6a9d46f041f6cff845d3a4a3834ac873869.tar.gz |
Add latest changes from gitlab-org/security/gitlab@14-3-stable-ee
23 files changed, 236 insertions, 4 deletions
diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 261f7af7ef1..c53d367ed71 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -37,6 +37,10 @@ export default { securityAndComplianceLabel: s__('ProjectSettings|Security & Compliance'), snippetsLabel: s__('ProjectSettings|Snippets'), wikiLabel: s__('ProjectSettings|Wiki'), + pucWarningLabel: s__('ProjectSettings|Warn about Potentially Unwanted Characters'), + pucWarningHelpText: s__( + 'ProjectSettings|Highlight the usage of hidden unicode characters. These have innocent uses for right-to-left languages, but can also be used in potential exploits.', + ), }, components: { @@ -178,6 +182,7 @@ export default { securityAndComplianceAccessLevel: featureAccessLevel.PROJECT_MEMBERS, operationsAccessLevel: featureAccessLevel.EVERYONE, containerRegistryAccessLevel: featureAccessLevel.EVERYONE, + warnAboutPotentiallyUnwantedCharacters: true, lfsEnabled: true, requestAccessEnabled: true, highlightChangesClass: false, @@ -752,5 +757,19 @@ export default { }}</template> </gl-form-checkbox> </project-setting-row> + <project-setting-row class="gl-mb-5"> + <input + :value="warnAboutPotentiallyUnwantedCharacters" + type="hidden" + name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]" + /> + <gl-form-checkbox + v-model="warnAboutPotentiallyUnwantedCharacters" + name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]" + > + {{ $options.i18n.pucWarningLabel }} + <template #help>{{ $options.i18n.pucWarningHelpText }}</template> + </gl-form-checkbox> + </project-setting-row> </div> </template> diff --git a/app/assets/javascripts/snippets/components/show.vue b/app/assets/javascripts/snippets/components/show.vue index 46629a569ec..35d88d5ec8e 100644 --- a/app/assets/javascripts/snippets/components/show.vue +++ b/app/assets/javascripts/snippets/components/show.vue @@ -66,7 +66,13 @@ export default { data-qa-selector="clone_button" /> </div> - <snippet-blob v-for="blob in blobs" :key="blob.path" :snippet="snippet" :blob="blob" /> + <snippet-blob + v-for="blob in blobs" + :key="blob.path" + :snippet="snippet" + :blob="blob" + class="project-highlight-puc" + /> </template> </div> </template> diff --git a/app/assets/stylesheets/framework/highlight.scss b/app/assets/stylesheets/framework/highlight.scss index b4a1d9f9977..122c605e603 100644 --- a/app/assets/stylesheets/framework/highlight.scss +++ b/app/assets/stylesheets/framework/highlight.scss @@ -85,3 +85,9 @@ td.line-numbers { line-height: 1; } + +.project-highlight-puc .unicode-bidi::before { + content: '�'; + cursor: pointer; + text-decoration: underline wavy $red-500; +} diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index de4e51a3a2f..d6bf0e340d0 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -399,6 +399,7 @@ class ProjectsController < Projects::ApplicationController show_default_award_emojis squash_option mr_default_target_self + warn_about_potentially_unwanted_characters ] end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d7f1cd505e9..6f8fce1b4dc 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -377,6 +377,12 @@ module ProjectsHelper } end + def project_classes(project) + return "project-highlight-puc" if project.warn_about_potentially_unwanted_characters? + + "" + end + private def tab_ability_map @@ -533,6 +539,7 @@ module ProjectsHelper metricsDashboardAccessLevel: feature.metrics_dashboard_access_level, operationsAccessLevel: feature.operations_access_level, showDefaultAwardEmojis: project.show_default_award_emojis?, + warnAboutPotentiallyUnwantedCharacters: project.warn_about_potentially_unwanted_characters?, securityAndComplianceAccessLevel: project.security_and_compliance_access_level, containerRegistryAccessLevel: feature.container_registry_access_level } diff --git a/app/models/project.rb b/app/models/project.rb index 0ba97138a25..f68fdadf51b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -438,8 +438,8 @@ class Project < ApplicationRecord :container_registry_access_level, :container_registry_enabled?, to: :project_feature, allow_nil: true alias_method :container_registry_enabled, :container_registry_enabled? - delegate :show_default_award_emojis, :show_default_award_emojis=, - :show_default_award_emojis?, + delegate :show_default_award_emojis, :show_default_award_emojis=, :show_default_award_emojis?, + :warn_about_potentially_unwanted_characters, :warn_about_potentially_unwanted_characters=, :warn_about_potentially_unwanted_characters?, to: :project_setting, allow_nil: true delegate :scheduled?, :started?, :in_progress?, :failed?, :finished?, prefix: :import, to: :import_state, allow_nil: true diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 54b11ea6041..b40bfbf5a79 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -221,6 +221,7 @@ class ProjectPolicy < BasePolicy enable :set_note_created_at enable :set_emails_disabled enable :set_show_default_award_emojis + enable :set_warn_about_potentially_unwanted_characters end rule { can?(:guest_access) }.policy do diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index 2df502d2899..a54e0351d2f 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -6,6 +6,7 @@ - display_subscription_banner! - display_namespace_storage_limit_alert! - @left_sidebar = true +- @content_class = [@content_class, project_classes(@project)].compact.join(" ") - content_for :project_javascripts do - project = @target_project || @project diff --git a/db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb b/db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb new file mode 100644 index 00000000000..166afa13371 --- /dev/null +++ b/db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddWarnAboutPotentiallyUnwantedCharactersToProjectSettings < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def up + add_column :project_settings, :warn_about_potentially_unwanted_characters, :boolean, null: false, default: true + end + + def down + remove_column :project_settings, :warn_about_potentially_unwanted_characters + end +end diff --git a/db/schema_migrations/20210929144453 b/db/schema_migrations/20210929144453 new file mode 100644 index 00000000000..753ea50c272 --- /dev/null +++ b/db/schema_migrations/20210929144453 @@ -0,0 +1 @@ +0f808c27d19e6a38d4aa31f2dd820fe226681af84e05c4af47213409b2043e5a
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d7e00112d73..7e04b3190f0 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18112,6 +18112,7 @@ CREATE TABLE project_settings ( cve_id_request_enabled boolean DEFAULT true NOT NULL, mr_default_target_self boolean DEFAULT false NOT NULL, previous_default_branch text, + warn_about_potentially_unwanted_characters boolean DEFAULT true NOT NULL, CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)), CONSTRAINT check_bde223416c CHECK ((show_default_award_emojis IS NOT NULL)) ); diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index becd25595a6..30edbe91125 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -39,6 +39,7 @@ module API optional :emails_disabled, type: Boolean, desc: 'Disable email notifications' optional :show_default_award_emojis, type: Boolean, desc: 'Show default award emojis' + optional :warn_about_potentially_unwanted_characters, type: Boolean, desc: 'Warn about Potentially Unwanted Characters' optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project' optional :resolve_outdated_diff_discussions, type: Boolean, desc: 'Automatically resolve merge request diffs discussions on lines changed with a push' optional :remove_source_branch_after_merge, type: Boolean, desc: 'Remove the source branch by default after merge' diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index a0d84703fbb..25f235f60dc 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -208,6 +208,7 @@ excluded_attributes: - :marked_for_deletion_by_user_id - :compliance_framework_setting - :show_default_award_emojis + - :warn_about_potentially_unwanted_characters - :services - :exported_protected_branches - :repository_size_limit diff --git a/lib/gitlab/unicode.rb b/lib/gitlab/unicode.rb new file mode 100644 index 00000000000..b49c5647dab --- /dev/null +++ b/lib/gitlab/unicode.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + class Unicode + # Regular expression for identifying bidirectional control + # characters in UTF-8 strings + # + # Documentation on how this works: + # https://idiosyncratic-ruby.com/41-proper-unicoding.html + BIDI_REGEXP = /\p{Bidi Control}/.freeze + + class << self + # Warning message used to highlight bidi characters in the GUI + def bidi_warning + _("Potentially unwanted character detected: Unicode BiDi Control") + end + end + end +end diff --git a/lib/rouge/formatters/html_gitlab.rb b/lib/rouge/formatters/html_gitlab.rb index e0e9677fac7..9e76225fc54 100644 --- a/lib/rouge/formatters/html_gitlab.rb +++ b/lib/rouge/formatters/html_gitlab.rb @@ -21,12 +21,24 @@ module Rouge is_first = false yield %(<span id="LC#{@line_number}" class="line" lang="#{@tag}">) - line.each { |token, value| yield span(token, value.chomp! || value) } + + line.each do |token, value| + yield highlight_unicode_control_characters(span(token, value.chomp! || value)) + end + yield %(</span>) @line_number += 1 end end + + private + + def highlight_unicode_control_characters(text) + text.gsub(Gitlab::Unicode::BIDI_REGEXP) do |char| + %(<span class="unicode-bidi has-tooltip" data-toggle="tooltip" title="#{Gitlab::Unicode.bidi_warning}">#{char}</span>) + end + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8649d261fd4..11feb4d2a41 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25417,6 +25417,9 @@ msgstr "" msgid "Postman collection file path or URL" msgstr "" +msgid "Potentially unwanted character detected: Unicode BiDi Control" +msgstr "" + msgid "Pre-defined push rules." msgstr "" @@ -26521,6 +26524,9 @@ msgstr "" msgid "ProjectSettings|Global" msgstr "" +msgid "ProjectSettings|Highlight the usage of hidden unicode characters. These have innocent uses for right-to-left languages, but can also be used in potential exploits." +msgstr "" + msgid "ProjectSettings|Internal" msgstr "" @@ -26710,6 +26716,9 @@ msgstr "" msgid "ProjectSettings|Visualize the project's performance metrics." msgstr "" +msgid "ProjectSettings|Warn about Potentially Unwanted Characters" +msgstr "" + msgid "ProjectSettings|What are badges?" msgstr "" diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 9d070061850..5f3843928dc 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -312,6 +312,34 @@ RSpec.describe ProjectsController do expect { get_show }.not_to change { Gitlab::GitalyClient.get_request_count } end + + describe "PUC highlighting" do + render_views + + before do + expect(controller).to receive(:find_routable!).and_return(public_project) + end + + context "option is enabled" do + it "adds the highlighting class" do + expect(public_project).to receive(:warn_about_potentially_unwanted_characters?).and_return(true) + + get_show + + expect(response.body).to have_css(".project-highlight-puc") + end + end + + context "option is disabled" do + it "doesn't add the highlighting class" do + expect(public_project).to receive(:warn_about_potentially_unwanted_characters?).and_return(false) + + get_show + + expect(response.body).not_to have_css(".project-highlight-puc") + end + end + end end context "when the url contains .atom" do diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js index 1e562419f32..0020269e4e7 100644 --- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js +++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js @@ -27,6 +27,7 @@ const defaultProps = { emailsDisabled: false, packagesEnabled: true, showDefaultAwardEmojis: true, + warnAboutPotentiallyUnwantedCharacters: true, }, isGitlabCom: true, canDisableEmails: true, @@ -97,6 +98,10 @@ describe('Settings Panel', () => { const findEmailSettings = () => wrapper.find({ ref: 'email-settings' }); const findShowDefaultAwardEmojis = () => wrapper.find('input[name="project[project_setting_attributes][show_default_award_emojis]"]'); + const findWarnAboutPuc = () => + wrapper.find( + 'input[name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"]', + ); const findMetricsVisibilitySettings = () => wrapper.find({ ref: 'metrics-visibility-settings' }); const findOperationsSettings = () => wrapper.find({ ref: 'operations-settings' }); @@ -539,6 +544,14 @@ describe('Settings Panel', () => { }); }); + describe('Warn about potentially unwanted characters', () => { + it('should have a "Warn about Potentially Unwanted Characters" input', () => { + wrapper = mountComponent(); + + expect(findWarnAboutPuc().exists()).toBe(true); + }); + }); + describe('Metrics dashboard', () => { it('should show the metrics dashboard access toggle', () => { wrapper = mountComponent(); diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 85b572d3f68..f563526f9d1 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -953,4 +953,26 @@ RSpec.describe ProjectsHelper do ) end end + + describe '#project_classes' do + subject { helper.project_classes(project) } + + it { is_expected.to be_a(String) } + + context 'PUC highlighting enabled' do + before do + project.warn_about_potentially_unwanted_characters = true + end + + it { is_expected.to include('project-highlight-puc') } + end + + context 'PUC highlighting disabled' do + before do + project.warn_about_potentially_unwanted_characters = false + end + + it { is_expected.not_to include('project-highlight-puc') } + end + end end diff --git a/spec/lib/gitlab/unicode_spec.rb b/spec/lib/gitlab/unicode_spec.rb new file mode 100644 index 00000000000..68f3266ecc7 --- /dev/null +++ b/spec/lib/gitlab/unicode_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::Unicode do + describe described_class::BIDI_REGEXP do + using RSpec::Parameterized::TableSyntax + + where(:bidi_string, :match) do + "\u2066" | true # left-to-right isolate + "\u2067" | true # right-to-left isolate + "\u2068" | true # first strong isolate + "\u2069" | true # pop directional isolate + "\u202a" | true # left-to-right embedding + "\u202b" | true # right-to-left embedding + "\u202c" | true # pop directional formatting + "\u202d" | true # left-to-right override + "\u202e" | true # right-to-left override + "\u2066foobar" | true + "" | false + "foo" | false + "\u2713" | false # checkmark + end + + with_them do + let(:utf8_string) { bidi_string.encode("utf-8") } + + it "matches only the bidi characters" do + expect(utf8_string.match?(subject)).to eq(match) + end + end + end +end diff --git a/spec/lib/rouge/formatters/html_gitlab_spec.rb b/spec/lib/rouge/formatters/html_gitlab_spec.rb index d45c8c2a8c5..6fa0ac1c4bc 100644 --- a/spec/lib/rouge/formatters/html_gitlab_spec.rb +++ b/spec/lib/rouge/formatters/html_gitlab_spec.rb @@ -36,5 +36,26 @@ RSpec.describe Rouge::Formatters::HTMLGitlab do is_expected.to eq(code) end end + + context 'when unicode control characters are used' do + let(:lang) { 'javascript' } + let(:tokens) { lexer.lex(code, continue: false) } + let(:code) do + <<~JS + #!/usr/bin/env node + + var accessLevel = "user"; + if (accessLevel != "user // Check if admin ") { + console.log("You are an admin."); + } + JS + end + + it 'highlights the control characters' do + message = "Potentially unwanted character detected: Unicode BiDi Control" + + is_expected.to include(%{<span class="unicode-bidi has-tooltip" data-toggle="tooltip" title="#{message}">}).exactly(4).times + end + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3989ddc31e8..8d6ec188fbd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -683,6 +683,19 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) } it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) } + describe 'project settings' do + %i( + show_default_award_emojis + show_default_award_emojis= + show_default_award_emojis? + warn_about_potentially_unwanted_characters + warn_about_potentially_unwanted_characters= + warn_about_potentially_unwanted_characters? + ).each do |method| + it { is_expected.to delegate_method(method).to(:project_setting).with_arguments(allow_nil: true) } + end + end + include_examples 'ci_cd_settings delegation' do # Skip attributes defined in EE code let(:exclude_attributes) do diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 9174356f123..dd00d413664 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -139,6 +139,7 @@ project_setting: - has_confluence - has_vulnerabilities - prevent_merge_without_jira_issue + - warn_about_potentially_unwanted_characters - previous_default_branch - project_id - push_rule_id |