Merge remote-tracking branch 'dev/14-9-stable-ee' into 14-9-stable-ee

CHANGELOG.md

@@ -2,6 +2,31 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
+## 14.9.2 (2022-03-31)
+
+### Security (20 changes)
+
+- [Quarantine UsageDataNonSqlMetrics failing test](gitlab-org/security/gitlab@123fc00ff9f407284ce05007ddc373e1bd0aeede) ([merge request](gitlab-org/security/gitlab!2364))
+- [Disallow login if password matches a fixed list](gitlab-org/security/gitlab@1a128ae3fb17b3d83974bb08034e4ba7a7d54e3b) ([merge request](gitlab-org/security/gitlab!2357))
+- [Update devise-two-factor to 4.0.2](gitlab-org/security/gitlab@17c70b13dcd437c05de63b3286245af8e6f42210) ([merge request](gitlab-org/security/gitlab!2349))
+- [Limit the number of tags associated with a CI runner](gitlab-org/security/gitlab@ed5daced882a0206e050c4f676a888ac1c2417b1) ([merge request](gitlab-org/security/gitlab!2303))
+- [GitLab Pages Security Updates for 14.9](gitlab-org/security/gitlab@79709cabf71a57a336f490636a7e32a208fe0229) ([merge request](gitlab-org/security/gitlab!2327))
+- [Upgrade swagger-ui dependency](gitlab-org/security/gitlab@14280c1d844be3ffc2f30f5321a818a7b6c51770) ([merge request](gitlab-org/security/gitlab!2336))
+- [Modify release link format check to avoid regex if string is too long](gitlab-org/security/gitlab@f516d883b46e1441410476dc140d69fde51cdf0f) ([merge request](gitlab-org/security/gitlab!2307))
+- [Masks variables in error messages](gitlab-org/security/gitlab@9cf62118390c0cfba3d36a4231a30a7836f06e2f) ([merge request](gitlab-org/security/gitlab!2308))
+- [Escape user provided string to prevent XSS](gitlab-org/security/gitlab@2da3502aef64ed1b01c13d82418950cf284098c6) ([merge request](gitlab-org/security/gitlab!2313))
+- [Monkey patch of RDoc to prevent Ruby segfault](gitlab-org/security/gitlab@0ae4925089a1b5fd7c9abeeb0756b3a50e05799a) ([merge request](gitlab-org/security/gitlab!2321))
+- [Project import maps members' created_by_id users based on source user ID](gitlab-org/security/gitlab@3826f2a7c652d3f74e45bfef8888601ca1c86ba1) ([merge request](gitlab-org/security/gitlab!2301))
+- [Redact InvalidURIError error messages](gitlab-org/security/gitlab@59b60e9cf8f79d6f41000d34a4434c5a04988030) ([merge request](gitlab-org/security/gitlab!2295))
+- [Fix access for approval rules API](gitlab-org/security/gitlab@7890215aa29624cd67c5bc8ac25175f2866479b7) ([merge request](gitlab-org/security/gitlab!2322))
+- [Fix kroki exploit](gitlab-org/security/gitlab@b2a44b407ab85ca056a271ba4e708128ef08d25f) ([merge request](gitlab-org/security/gitlab!2306))
+- [Fix blind SSRF when looking up SSH host keys for mirroring](gitlab-org/security/gitlab@5a9509b52584302c508bd6dff1454f80aae371ea) ([merge request](gitlab-org/security/gitlab!2309))
+- [Escape original content in reference redactor](gitlab-org/security/gitlab@b33b170a2c2df8285999f3631e8a53d35e0eed22) ([merge request](gitlab-org/security/gitlab!2317))
+- [Security fix for CI/CD analytics visibility](gitlab-org/security/gitlab@f3febd00b440475b2aca0b9bd6728fa5f8750288) ([merge request](gitlab-org/security/gitlab!2304))
+- [Latest commit exposed through fork of a private project](gitlab-org/security/gitlab@3f20d4f294a12ceb33bec19d86790f582fb7fb48) ([merge request](gitlab-org/security/gitlab!2294))
+- [Fix Asana integration restricted branch filter](gitlab-org/security/gitlab@08aa0f55b1b715f7311ee6502cd6f8a1b875f878) ([merge request](gitlab-org/security/gitlab!2300))
+- [Revert "JH need more complex passwords"](gitlab-org/security/gitlab@e2fb87ec5d4e235d6b83454980cec9c049849a1c) ([merge request](gitlab-org/security/gitlab!2352))
+
 ## 14.9.1 (2022-03-23)
 ### Fixed (1 change)

GITALY_SERVER_VERSION

-14.9.1
\ No newline at end of file
+14.9.2
\ No newline at end of file

GITLAB_PAGES_VERSION

-1.56.0
+1.56.1

Gemfile

@@ -67,7 +67,7 @@ gem 'akismet', '~> 3.0'
 gem 'invisible_captcha', '~> 1.1.0'
 # Two-factor authentication
-gem 'devise-two-factor', '~> 4.0.0'
+gem 'devise-two-factor', '~> 4.0.2'
 gem 'rqrcode-rails3', '~> 0.1.7'
 gem 'attr_encrypted', '~> 3.1.0'
 gem 'u2f', '~> 0.2.1'

Gemfile.lock

@@ -270,11 +270,11 @@ GEM
       railties (>= 4.1.0)
       responders
       warden (~> 1.2.3)
-    devise-two-factor (4.0.0)
-      activesupport (< 6.2)
+    devise-two-factor (4.0.2)
+      activesupport (< 7.1)
       attr_encrypted (>= 1.3, < 4, != 2)
       devise (~> 4.0)
-      railties (< 6.2)
+      railties (< 7.1)
       rotp (~> 6.0)
     diff-lcs (1.4.4)
     diff_match_patch (0.1.0)
@@ -1457,7 +1457,7 @@ DEPENDENCIES
   derailed_benchmarks
   device_detector
   devise (~> 4.7.2)
-  devise-two-factor (~> 4.0.0)
+  devise-two-factor (~> 4.0.2)
   diff_match_patch (~> 0.1.0)
   diffy (~> 3.3)
   discordrb-webhooks (~> 3.4)

VERSION

-14.9.1-ee
\ No newline at end of file
+14.9.2-ee
\ No newline at end of file

app/assets/javascripts/behaviors/components/diagram_performance_warning.vue

+<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>

app/assets/javascripts/behaviors/markdown/constants.js

 // 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',
+];

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'));

app/assets/javascripts/behaviors/markdown/render_kroki.js

+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));
+    }
+  });
+}

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 ||

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 ||

app/assets/javascripts/blob/openapi/index.js

 import { SwaggerUIBundle } from 'swagger-ui-dist';
 import createFlash from '~/flash';
-import { removeParams, updateHistory } from '~/lib/utils/url_utility';
 import { __ } from '~/locale';
 export default () => {
@@ -8,14 +7,10 @@ export default () => {
   Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')])
     .then(() => {
-      // Temporary fix to prevent an XSS attack due to "useUnsafeMarkdown"
-      // Once we upgrade Swagger to "4.0.0", we can safely remove this as it will be deprecated
-      // Follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/339696
-      updateHistory({ url: removeParams(['useUnsafeMarkdown']), replace: true });
       SwaggerUIBundle({
         url: el.dataset.endpoint,
         dom_id: '#js-openapi-viewer',
-        useUnsafeMarkdown: false,
+        deepLinking: true,
       });
     })
     .catch((error) => {

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

app/models/ci/runner.rb

@@ -65,6 +65,8 @@ module Ci
     FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze
     MINUTES_COST_FACTOR_FIELDS = %i[public_projects_minutes_cost_factor private_projects_minutes_cost_factor].freeze
+    TAG_LIST_MAX_LENGTH = 50
+
     has_many :builds
     has_many :runner_projects, inverse_of: :runner, autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
     has_many :projects, through: :runner_projects, disable_joins: true
@@ -520,6 +522,11 @@ module Ci
         errors.add(:tags_list,
           'can not be empty when runner is not allowed to pick untagged jobs')
       end
+
+      if tag_list_changed? && tag_list.count > TAG_LIST_MAX_LENGTH
+        errors.add(:tags_list,
+          "Too many tags specified. Please limit the number of tags to #{TAG_LIST_MAX_LENGTH}")
+      end
     end
     def no_projects

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

app/models/releases/link.rb

@@ -9,10 +9,20 @@ module Releases
     # See https://gitlab.com/gitlab-org/gitlab/-/issues/218753
     # Regex modified to prevent catastrophic backtracking
     FILEPATH_REGEX = %r{\A\/[^\/](?!.*\/\/.*)[\-\.\w\/]+[\da-zA-Z]+\z}.freeze
+    FILEPATH_MAX_LENGTH = 128
     validates :url, presence: true, addressable_url: { schemes: %w(http https ftp) }, uniqueness: { scope: :release }
     validates :name, presence: true, uniqueness: { scope: :release }
-    validates :filepath, uniqueness: { scope: :release }, format: { with: FILEPATH_REGEX }, allow_blank: true, length: { maximum: 128 }
+    validates :filepath, uniqueness: { scope: :release }, allow_blank: true
+    validate :filepath_format_valid?
+
+    # we use a custom validator here to prevent running the regex if the string is too long
+    # see https://gitlab.com/gitlab-org/gitlab/-/issues/273771
+    def filepath_format_valid?
+      return if filepath.nil? # valid use case
+      return errors.add(:filepath, "is too long (maximum is #{FILEPATH_MAX_LENGTH} characters)") if filepath.length > FILEPATH_MAX_LENGTH
+      return errors.add(:filepath, 'is in an invalid format') unless FILEPATH_REGEX.match? filepath
+    end
     scope :sorted, -> { order(created_at: :desc) }

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'
-    Addressable::URI.parse("ssh://#{full_url.host}:#{full_url.inferred_port}")
-  rescue Addressable::URI::InvalidURIError
+    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
+    [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

app/models/user.rb

@@ -879,6 +879,23 @@ class User < ApplicationRecord
     reset_password_sent_at.present? && reset_password_sent_at >= 1.minute.ago
   end
+  # See https://gitlab.com/gitlab-org/security/gitlab/-/issues/638
+  DISALLOWED_PASSWORDS = %w[123qweQWE!@#000000000].freeze
+
+  # Overwrites valid_password? from Devise::Models::DatabaseAuthenticatable
+  # In constant-time, check both that the password isn't on a denylist AND
+  # that the password is the user's password
+  def valid_password?(password)
+    password_allowed = true
+    DISALLOWED_PASSWORDS.each do |disallowed_password|
+      password_allowed = false if Devise.secure_compare(password, disallowed_password)
+    end
+
+    original_result = super
+
+    password_allowed && original_result
+  end
+
   def remember_me!
     super if ::Gitlab::Database.read_write?
   end

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