From 1153e17b2d34c50834251038269ac11f18219bdf Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 31 Mar 2022 00:00:32 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@14-9-stable-ee --- .../components/diagram_performance_warning.vue | 25 +++++++++ .../javascripts/behaviors/markdown/constants.js | 16 ++++++ .../javascripts/behaviors/markdown/render_gfm.js | 2 + .../javascripts/behaviors/markdown/render_kroki.js | 63 ++++++++++++++++++++++ .../behaviors/markdown/render_mermaid.js | 21 +------- .../behaviors/markdown/render_sandboxed_mermaid.js | 20 +------ .../merge_requests/creations_controller.rb | 2 +- app/models/integrations/asana.rb | 15 ++++-- app/models/ssh_host_key.rb | 36 ++++++++++--- app/policies/project_policy.rb | 10 ++-- 10 files changed, 157 insertions(+), 53 deletions(-) create mode 100644 app/assets/javascripts/behaviors/components/diagram_performance_warning.vue create mode 100644 app/assets/javascripts/behaviors/markdown/render_kroki.js (limited to 'app') diff --git a/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue b/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue new file mode 100644 index 00000000000..31b2682b546 --- /dev/null +++ b/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue @@ -0,0 +1,25 @@ + + + diff --git a/app/assets/javascripts/behaviors/markdown/constants.js b/app/assets/javascripts/behaviors/markdown/constants.js index b4545d6c6c6..13f8d9ef0cf 100644 --- a/app/assets/javascripts/behaviors/markdown/constants.js +++ b/app/assets/javascripts/behaviors/markdown/constants.js @@ -1,3 +1,19 @@ // https://prosemirror.net/docs/ref/#model.ParseRule.priority export const DEFAULT_PARSE_RULE_PRIORITY = 50; export const HIGHER_PARSE_RULE_PRIORITY = 1 + DEFAULT_PARSE_RULE_PRIORITY; + +export const unrestrictedPages = [ + // Group wiki + 'groups:wikis:show', + 'groups:wikis:edit', + 'groups:wikis:create', + + // Project wiki + 'projects:wikis:show', + 'projects:wikis:edit', + 'projects:wikis:create', + + // Project files + 'projects:show', + 'projects:blob:show', +]; diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js index c4e09efe263..6236e3bdefc 100644 --- a/app/assets/javascripts/behaviors/markdown/render_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js @@ -2,6 +2,7 @@ import $ from 'jquery'; import syntaxHighlight from '~/syntax_highlight'; import initUserPopovers from '../../user_popovers'; import highlightCurrentUser from './highlight_current_user'; +import { renderKroki } from './render_kroki'; import renderMath from './render_math'; import renderMermaid from './render_mermaid'; import renderSandboxedMermaid from './render_sandboxed_mermaid'; @@ -13,6 +14,7 @@ import renderMetrics from './render_metrics'; // $.fn.renderGFM = function renderGFM() { syntaxHighlight(this.find('.js-syntax-highlight').get()); + renderKroki(this.find('.js-render-kroki[hidden]').get()); renderMath(this.find('.js-render-math')); if (gon.features?.sandboxedMermaid) { renderSandboxedMermaid(this.find('.js-render-mermaid')); diff --git a/app/assets/javascripts/behaviors/markdown/render_kroki.js b/app/assets/javascripts/behaviors/markdown/render_kroki.js new file mode 100644 index 00000000000..7843df0cd8e --- /dev/null +++ b/app/assets/javascripts/behaviors/markdown/render_kroki.js @@ -0,0 +1,63 @@ +import Vue from 'vue'; +import DiagramPerformanceWarning from '../components/diagram_performance_warning.vue'; +import { unrestrictedPages } from './constants'; + +/** + * Create alert element. + * + * @param {Element} krokiImage Kroki `img` element + * @return {Element} Alert element + */ +function createAlert(krokiImage) { + const app = new Vue({ + el: document.createElement('div'), + name: 'DiagramPerformanceWarningRoot', + render(createElement) { + return createElement(DiagramPerformanceWarning, { + on: { + closeAlert() { + app.$destroy(); + app.$el.remove(); + }, + showImage() { + krokiImage.removeAttribute('hidden'); + app.$destroy(); + app.$el.remove(); + }, + }, + }); + }, + }); + + return app.$el; +} + +/** + * Add warning alert to hidden Kroki images, + * or show Kroki image if on an unrestricted page. + * + * Kroki images are given a hidden attribute by the + * backend when the original markdown source is large. + * + * @param {Array} krokiImages Array of hidden Kroki `img` elements + */ +export function renderKroki(krokiImages) { + const pageName = document.querySelector('body').dataset.page; + const isUnrestrictedPage = unrestrictedPages.includes(pageName); + + krokiImages.forEach((krokiImage) => { + if (isUnrestrictedPage) { + krokiImage.removeAttribute('hidden'); + return; + } + + const parent = krokiImage.closest('.js-markdown-code'); + + // A single Kroki image is processed multiple times for some reason, + // so this condition ensures we only create one alert per Kroki image + if (!parent.hasAttribute('data-kroki-processed')) { + parent.setAttribute('data-kroki-processed', 'true'); + parent.after(createAlert(krokiImage)); + } + }); +} diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index d78c456ed5b..f9cf3af98bb 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -3,6 +3,7 @@ import { once, countBy } from 'lodash'; import createFlash from '~/flash'; import { darkModeEnabled } from '~/lib/utils/color_utils'; import { __, sprintf } from '~/locale'; +import { unrestrictedPages } from './constants'; // Renders diagrams and flowcharts from text using Mermaid in any element with the // `js-render-mermaid` class. @@ -30,24 +31,6 @@ let renderedMermaidBlocks = 0; let mermaidModule = {}; -// Whitelist pages where we won't impose any restrictions -// on mermaid rendering -const WHITELISTED_PAGES = [ - // Group wiki - 'groups:wikis:show', - 'groups:wikis:edit', - 'groups:wikis:create', - - // Project wiki - 'projects:wikis:show', - 'projects:wikis:edit', - 'projects:wikis:create', - - // Project files - 'projects:show', - 'projects:blob:show', -]; - export function initMermaid(mermaid) { let theme = 'neutral'; @@ -163,7 +146,7 @@ function renderMermaids($els) { * up the entire thread and causing a DoS. */ if ( - !WHITELISTED_PAGES.includes(pageName) && + !unrestrictedPages.includes(pageName) && ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT || renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT || diff --git a/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js index 85a991a1ec9..6922ec9c5a5 100644 --- a/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js @@ -9,6 +9,7 @@ import { } from '~/lib/utils/url_utility'; import { darkModeEnabled } from '~/lib/utils/color_utils'; import { setAttributes } from '~/lib/utils/dom_utils'; +import { unrestrictedPages } from './constants'; // Renders diagrams and flowcharts from text using Mermaid in any element with the // `js-render-mermaid` class. @@ -36,23 +37,6 @@ const BUFFER_IFRAME_HEIGHT = 10; const elsProcessingMap = new WeakMap(); let renderedMermaidBlocks = 0; -// Pages without any restrictions on mermaid rendering -const PAGES_WITHOUT_RESTRICTIONS = [ - // Group wiki - 'groups:wikis:show', - 'groups:wikis:edit', - 'groups:wikis:create', - - // Project wiki - 'projects:wikis:show', - 'projects:wikis:edit', - 'projects:wikis:create', - - // Project files - 'projects:show', - 'projects:blob:show', -]; - function shouldLazyLoadMermaidBlock(source) { /** * If source contains `&`, which means that it might @@ -149,7 +133,7 @@ function renderMermaids($els) { * up the entire thread and causing a DoS. */ if ( - !PAGES_WITHOUT_RESTRICTIONS.includes(pageName) && + !unrestrictedPages.includes(pageName) && ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT || renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT || diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 88337242fcd..93e2298ca98 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -81,7 +81,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap def branch_to @target_project = selected_target_project - if @target_project && params[:ref].present? + if @target_project && params[:ref].present? && Ability.allowed?(current_user, :create_merge_request_in, @target_project) @ref = params[:ref] @commit = @target_project.commit(Gitlab::Git::BRANCH_REF_PREFIX + @ref) end diff --git a/app/models/integrations/asana.rb b/app/models/integrations/asana.rb index 054f0606dd2..e7634f51ec4 100644 --- a/app/models/integrations/asana.rb +++ b/app/models/integrations/asana.rb @@ -59,12 +59,9 @@ module Integrations def execute(data) return unless supported_events.include?(data[:object_kind]) - # check the branch restriction is poplulated and branch is not included branch = Gitlab::Git.ref_name(data[:ref]) - branch_restriction = restrict_to_branch.to_s - if branch_restriction.present? && branch_restriction.index(branch).nil? - return - end + + return unless branch_allowed?(branch) user = data[:user_name] project_name = project.full_name @@ -103,5 +100,13 @@ module Integrations end end end + + private + + def branch_allowed?(branch_name) + return true if restrict_to_branch.blank? + + restrict_to_branch.to_s.gsub(/\s+/, '').split(',').include?(branch_name) + end end end diff --git a/app/models/ssh_host_key.rb b/app/models/ssh_host_key.rb index bb928118edf..ac7ba9530dd 100644 --- a/app/models/ssh_host_key.rb +++ b/app/models/ssh_host_key.rb @@ -46,11 +46,11 @@ class SshHostKey .select(&:valid?) end - attr_reader :project, :url, :compare_host_keys + attr_reader :project, :url, :ip, :compare_host_keys def initialize(project:, url:, compare_host_keys: nil) @project = project - @url = normalize_url(url) + @url, @ip = normalize_url(url) @compare_host_keys = compare_host_keys end @@ -90,9 +90,11 @@ class SshHostKey end def calculate_reactive_cache + input = [ip, url.hostname].compact.join(' ') + known_hosts, errors, status = Open3.popen3({}, *%W[ssh-keyscan -T 5 -p #{url.port} -f-]) do |stdin, stdout, stderr, wait_thr| - stdin.puts(url.host) + stdin.puts(input) stdin.close [ @@ -127,11 +129,31 @@ class SshHostKey end def normalize_url(url) - full_url = ::Addressable::URI.parse(url) - raise ArgumentError, "Invalid URL" unless full_url&.scheme == 'ssh' + url, real_hostname = Gitlab::UrlBlocker.validate!( + url, + schemes: %w[ssh], + allow_localhost: allow_local_requests?, + allow_local_network: allow_local_requests?, + dns_rebind_protection: Gitlab::CurrentSettings.dns_rebinding_protection_enabled? + ) + + # When DNS rebinding protection is required, the hostname is replaced by the + # resolved IP. However, `url` is used in `id`, so we can't change it. Track + # the resolved IP separately instead. + if real_hostname + ip = url.hostname + url.hostname = real_hostname + end + + # Ensure ssh://foo and ssh://foo:22 share the same cache + url.port = url.inferred_port - Addressable::URI.parse("ssh://#{full_url.host}:#{full_url.inferred_port}") - rescue Addressable::URI::InvalidURIError + [url, ip] + rescue Gitlab::UrlBlocker::BlockedUrlError raise ArgumentError, "Invalid URL" end + + def allow_local_requests? + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? + end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 09085bef9f0..2ffafb79134 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -240,7 +240,6 @@ class ProjectPolicy < BasePolicy rule { can?(:guest_access) }.policy do enable :read_project - enable :create_merge_request_in enable :read_issue_board enable :read_issue_board_list enable :read_wiki @@ -497,7 +496,7 @@ class ProjectPolicy < BasePolicy prevent(*create_read_update_admin_destroy(:issue_board_list)) end - rule { merge_requests_disabled | repository_disabled }.policy do + rule { merge_requests_disabled | repository_disabled | ~can?(:download_code) }.policy do prevent :create_merge_request_in prevent :create_merge_request_from prevent(*create_read_update_admin_destroy(:merge_request)) @@ -600,13 +599,14 @@ class ProjectPolicy < BasePolicy enable :read_cycle_analytics enable :read_pages_content enable :read_analytics - enable :read_ci_cd_analytics enable :read_insights # NOTE: may be overridden by IssuePolicy enable :read_issue end + rule { can?(:public_access) & public_builds }.enable :read_ci_cd_analytics + rule { public_builds }.policy do enable :read_build end @@ -664,6 +664,10 @@ class ProjectPolicy < BasePolicy enable :read_security_configuration end + rule { can?(:guest_access) & can?(:read_commit_status) }.policy do + enable :create_merge_request_in + end + # Design abilities could also be prevented in the issue policy. rule { design_management_disabled }.policy do prevent :read_design -- cgit v1.2.3