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

CHANGELOG.md

@@ -2,6 +2,32 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
+## 14.8.5 (2022-03-31)
+
+### Security (21 changes)
+
+- [Update to commonmarker 0.23.4](gitlab-org/security/gitlab@51532ccc5f1b6b053d4ca6c54496607e62f8f25c) ([merge request](gitlab-org/security/gitlab!2282))
+- [Revert merge request approval groups behavior](gitlab-org/security/gitlab@dd9724e429033974da6c3852dc6fd33f0f2b0a46) ([merge request](gitlab-org/security/gitlab!2334))
+- [Disallow login if password matches a fixed list](gitlab-org/security/gitlab@6779d5f2948425a7ad7f19a6e10f82cc10b80989) ([merge request](gitlab-org/security/gitlab!2358))
+- [Update devise-two-factor to 4.0.2](gitlab-org/security/gitlab@0329d2d82a9064c0bae36e7b993ee40df7c999bc) ([merge request](gitlab-org/security/gitlab!2350))
+- [Limit the number of tags associated with a CI runner](gitlab-org/security/gitlab@8d5938c08fe66c22f1bc54ff76cc9daf2de86b1a) ([merge request](gitlab-org/security/gitlab!2302))
+- [GitLab Pages Security Updates for 14.9](gitlab-org/security/gitlab@5a5a862c8a9e37ca2ea84133f92b216eaa7cd148) ([merge request](gitlab-org/security/gitlab!2328))
+- [Upgrade swagger-ui dependency](gitlab-org/security/gitlab@afcb570867db61347bb6a4e243bb2557340191be) ([merge request](gitlab-org/security/gitlab!2337))
+- [Modify release link format check to avoid regex if string is too long](gitlab-org/security/gitlab@a3ab0ff9c470c1c6e5b4fd055ddd02dffce32652) ([merge request](gitlab-org/security/gitlab!2243))
+- [Masks variables in error messages](gitlab-org/security/gitlab@94236bbdb8eef6600562bdc4e242e07eaed8c50f) ([merge request](gitlab-org/security/gitlab!2291))
+- [Escape user provided string to prevent XSS](gitlab-org/security/gitlab@03e695d4c34546582b503b3f7712246206b56b99) ([merge request](gitlab-org/security/gitlab!2314))
+- [Monkey patch of RDoc to prevent Ruby segfault](gitlab-org/security/gitlab@14eec4487387bc0c999f1c48b046a3ed3848c5a1) ([merge request](gitlab-org/security/gitlab!2232))
+- [Project import maps members' created_by_id users based on source user ID](gitlab-org/security/gitlab@7fd7ab3f57e8d8b4e0aed42aebe9a8b7436a6255) ([merge request](gitlab-org/security/gitlab!2238))
+- [Redact InvalidURIError error messages](gitlab-org/security/gitlab@0592c182bfd60aee501c4c66f47a71c9469f2bcd) ([merge request](gitlab-org/security/gitlab!2296))
+- [Fix access for approval rules API](gitlab-org/security/gitlab@987e06bacba224519adf94cda73b5a8b2e7b917a) ([merge request](gitlab-org/security/gitlab!2323))
+- [Fix kroki exploit](gitlab-org/security/gitlab@bf056c683af25ec4b94c0efa7166eea399ed6502) ([merge request](gitlab-org/security/gitlab!2277))
+- [Fix blind SSRF when looking up SSH host keys for mirroring](gitlab-org/security/gitlab@3c853a32a73aba15e309d05111b744455a360cca) ([merge request](gitlab-org/security/gitlab!2310))
+- [Escape original content in reference redactor](gitlab-org/security/gitlab@00ee99bc3834d9d59572272064c9ad6abeae5975) ([merge request](gitlab-org/security/gitlab!2318))
+- [Security fix for CI/CD analytics visibility](gitlab-org/security/gitlab@691d69be77ae3c8e0a2598b75ccf336b672fd540) ([merge request](gitlab-org/security/gitlab!2273))
+- [Latest commit exposed through fork of a private project](gitlab-org/security/gitlab@6ca7a3b040edac06b23a697bfc2bf46f457d6b81) ([merge request](gitlab-org/security/gitlab!2271))
+- [Fix Asana integration restricted branch filter](gitlab-org/security/gitlab@4c1db692b4e99fab6cdbb818cf02fb879f6d4886) ([merge request](gitlab-org/security/gitlab!2218))
+- [Revert "JH need more complex passwords"](gitlab-org/security/gitlab@919aa2b28645d49fb71508362a0c61da39893c69) ([merge request](gitlab-org/security/gitlab!2353))
+
 ## 14.8.4 (2022-03-16)
 ### Added (1 change)

GITALY_SERVER_VERSION

-14.8.4
\ No newline at end of file
+14.8.5
\ No newline at end of file

GITLAB_PAGES_VERSION

-1.54.0
+1.54.1

Gemfile

@@ -65,7 +65,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'
@@ -153,7 +153,7 @@ gem 'html-pipeline', '~> 2.13.2'
 gem 'deckar01-task_list', '2.3.1'
 gem 'gitlab-markup', '~> 1.8.0'
 gem 'github-markup', '~> 1.7.0', require: 'github/markup'
-gem 'commonmarker', '~> 0.23.2'
+gem 'commonmarker', '~> 0.23.4'
 gem 'kramdown', '~> 2.3.1'
 gem 'RedCloth', '~> 4.3.2'
 gem 'rdoc', '~> 6.3.2'

Gemfile.lock

@@ -199,7 +199,7 @@ GEM
       open4 (~> 1.3)
     coderay (1.1.3)
     colored2 (3.1.2)
-    commonmarker (0.23.2)
+    commonmarker (0.23.4)
     concurrent-ruby (1.1.9)
     connection_pool (2.2.5)
     contracts (0.11.0)
@@ -264,11 +264,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)
@@ -1415,7 +1415,7 @@ DEPENDENCIES
   capybara-screenshot (~> 1.0.22)
   carrierwave (~> 1.3)
   charlock_holmes (~> 0.7.7)
-  commonmarker (~> 0.23.2)
+  commonmarker (~> 0.23.4)
   concurrent-ruby (~> 1.1)
   connection_pool (~> 2.0)
   countries (~> 3.0)
@@ -1429,7 +1429,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.8.4-ee
\ No newline at end of file
+14.8.5-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

@@ -61,12 +61,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
@@ -105,5 +102,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

@@ -880,6 +880,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

@@ -234,7 +234,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
@@ -490,7 +489,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))
@@ -593,13 +592,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
@@ -657,6 +657,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