Add latest changes from gitlab-org/security/gitlab@14-9-stable-ee

10 files changed, 157 insertions, 53 deletions

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 @@
+<script>
+import { GlAlert } from '@gitlab/ui';
+import { __ } from '~/locale';
+
+export default {
+  i18n: {
+    bodyText: __('Warning: Displaying this diagram might cause performance issues on this page.'),
+    buttonText: __('Display'),
+  },
+  components: {
+    GlAlert,
+  },
+};
+</script>
+
+<template>
+  <gl-alert
+    :primary-button-text="$options.i18n.buttonText"
+    variant="warning"
+    @dismiss="$emit('closeAlert')"
+    @primaryAction="$emit('showImage')"
+  >
+    {{ $options.i18n.bodyText }}
+  </gl-alert>
+</template>
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<Element>} 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