summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-10-27 12:41:41 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-10-27 12:41:41 +0000
commitc1c828ac7f7b3c2e51d81921bbef9d474cd4d0a4 (patch)
tree32fabcdfa49cd8eab122cf5efecb47db6d5e59bf
parent547a5884d1ab6a22d9fc9ce79e5cf6f0310bc23d (diff)
downloadgitlab-ce-c1c828ac7f7b3c2e51d81921bbef9d474cd4d0a4.tar.gz
Add latest changes from gitlab-org/security/gitlab@14-4-stable-ee
-rw-r--r--app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue19
-rw-r--r--app/assets/javascripts/snippets/components/show.vue8
-rw-r--r--app/assets/stylesheets/framework/highlight.scss6
-rw-r--r--app/controllers/projects_controller.rb1
-rw-r--r--app/helpers/projects_helper.rb7
-rw-r--r--app/models/project.rb4
-rw-r--r--app/policies/project_policy.rb1
-rw-r--r--app/views/layouts/project.html.haml1
-rw-r--r--db/migrate/20210929144453_add_warn_about_potentially_unwanted_characters_to_project_settings.rb16
-rw-r--r--db/schema_migrations/202109291444531
-rw-r--r--db/structure.sql1
-rw-r--r--lib/api/helpers/projects_helpers.rb1
-rw-r--r--lib/gitlab/import_export/project/import_export.yml1
-rw-r--r--lib/gitlab/unicode.rb19
-rw-r--r--lib/rouge/formatters/html_gitlab.rb14
-rw-r--r--locale/gitlab.pot9
-rw-r--r--spec/controllers/projects_controller_spec.rb28
-rw-r--r--spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js13
-rw-r--r--spec/helpers/projects_helper_spec.rb22
-rw-r--r--spec/lib/gitlab/unicode_spec.rb33
-rw-r--r--spec/lib/rouge/formatters/html_gitlab_spec.rb21
-rw-r--r--spec/models/project_spec.rb13
-rw-r--r--spec/requests/api/project_attributes.yml1
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 26da0436dd8..0760f97d7c1 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -409,6 +409,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 03e7fb5ffc4..e3b63d122d2 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -376,6 +376,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
@@ -532,6 +538,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 00a572b775d..2ceba10e86e 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -423,8 +423,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 59aa47beff9..87573c9ad13 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 d3882446f57..c73ade92f80 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -18156,6 +18156,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 c962e0f677b..618ef9a4f43 100644
--- a/lib/gitlab/import_export/project/import_export.yml
+++ b/lib/gitlab/import_export/project/import_export.yml
@@ -325,6 +325,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 bdb3c99969b..56e7413c530 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -25736,6 +25736,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 ""
@@ -26849,6 +26852,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 ""
@@ -27038,6 +27044,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 3d966848c5b..b34cfedb767 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -323,6 +323,34 @@ RSpec.describe ProjectsController do
expect(response).to render_template('_files')
expect(response.body).to have_content('LICENSE') # would be 'MIT license' if stub not works
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 1100f4a3ad5..5d52c9178cb 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -961,4 +961,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 4bc9b256dce..7c92c62e30b 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 10220448936..2e5c5af4eb0 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -667,6 +667,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