New upstream version 11.8.6

CHANGELOG.md

@@ -2,6 +2,27 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.8.6 (2019-03-28)
+
+### Security (7 changes)
+
+- Disallow guest users from accessing Releases.
+- Fix PDF.js vulnerability.
+- Hide "related branches" when user does not have permission.
+- Fix XSS in resolve conflicts form.
+- Added rake task for removing EXIF data from existing uploads.
+- Disallow updating namespace when updating a project.
+- Use UntrustedRegexp for matching refs policy.
+
+
+## 11.8.5 (2019-03-27)
+
+- Unreleased due to QA failure.
+
+## 11.8.4 (2019-03-26)
+
+- Unreleased due to QA failure.
+
 ## 11.8.3 (2019-03-19)
 
 ### Security (1 change)

GITLAB_WORKHORSE_VERSION

@@ -1 +1 @@
-8.3.1
+8.3.3

VERSION

@@ -1 +1 @@
-11.8.3
+11.8.6

app/assets/javascripts/issue.js

@@ -16,7 +16,9 @@ export default class Issue {
     Issue.createMrDropdownWrap = document.querySelector('.create-mr-dropdown-wrap');
 
     Issue.initMergeRequests();
-    Issue.initRelatedBranches();
+    if (document.querySelector('#related-branches')) {
+      Issue.initRelatedBranches();
+    }
 
     this.closeButtons = $('a.btn-close');
     this.reopenButtons = $('a.btn-reopen');

app/assets/javascripts/pdf/index.vue

@@ -28,7 +28,7 @@ export default {
   },
   watch: { pdf: 'load' },
   mounted() {
-    pdfjsLib.PDFJS.workerSrc = workerSrc;
+    pdfjsLib.GlobalWorkerOptions.workerSrc = workerSrc;
     if (this.hasPDF) this.load();
   },
   methods: {

app/controllers/projects/issues_controller.rb

@@ -39,6 +39,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :authorize_create_merge_request_from!, only: [:create_merge_request]
 
   before_action :authorize_import_issues!, only: [:import_csv]
+  before_action :authorize_download_code!, only: [:related_branches]
 
   before_action :set_suggested_issues_feature_flags, only: [:new]
 

app/controllers/projects_controller.rb

@@ -47,7 +47,7 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def create
-    @project = ::Projects::CreateService.new(current_user, project_params).execute
+    @project = ::Projects::CreateService.new(current_user, project_params(attributes: project_params_create_attributes)).execute
 
     if @project.saved?
       cookies[:issue_board_welcome_hidden] = { path: project_path(@project), value: nil, expires: Time.at(0) }
@@ -328,9 +328,9 @@ class ProjectsController < Projects::ApplicationController
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def project_params
+  def project_params(attributes: [])
     params.require(:project)
-      .permit(project_params_attributes)
+      .permit(project_params_attributes + attributes)
   end
 
   def project_params_attributes
@@ -349,11 +349,10 @@ class ProjectsController < Projects::ApplicationController
       :last_activity_at,
       :lfs_enabled,
       :name,
-      :namespace_id,
       :only_allow_merge_if_all_discussions_are_resolved,
       :only_allow_merge_if_pipeline_succeeds,
-      :printing_merge_request_link_enabled,
       :path,
+      :printing_merge_request_link_enabled,
       :public_builds,
       :request_access_enabled,
       :runners_token,
@@ -375,6 +374,10 @@ class ProjectsController < Projects::ApplicationController
     ]
   end
 
+  def project_params_create_attributes
+    [:namespace_id]
+  end
+
   def custom_import_params
     {}
   end

app/models/label.rb

@@ -126,6 +126,10 @@ class Label < ActiveRecord::Base
     fuzzy_search(query, [:title, :description])
   end
 
+  def self.by_ids(ids)
+    where(id: ids)
+  end
+
   def open_issues_count(user = nil)
     issues_count(user, state: 'opened')
   end

app/policies/project_policy.rb

@@ -178,7 +178,6 @@ class ProjectPolicy < BasePolicy
     enable :read_cycle_analytics
     enable :award_emoji
     enable :read_pages_content
-    enable :read_release
   end
 
   # These abilities are not allowed to admins that are not members of the project,
@@ -204,6 +203,7 @@ class ProjectPolicy < BasePolicy
     enable :read_deployment
     enable :read_merge_request
     enable :read_sentry_issue
+    enable :read_release
   end
 
   # We define `:public_user_access` separately because there are cases in gitlab-ee

app/services/issuable_base_service.rb

@@ -70,10 +70,14 @@ class IssuableBaseService < BaseService
   end
 
   def filter_labels
-    filter_labels_in_param(:add_label_ids)
+    params[:add_label_ids] = labels_service.filter_labels_ids_in_param(:add_label_ids) if params[:add_label_ids]
-    filter_labels_in_param(:remove_label_ids)
+    params[:remove_label_ids] = labels_service.filter_labels_ids_in_param(:remove_label_ids) if params[:remove_label_ids]
-    filter_labels_in_param(:label_ids)
+
-    find_or_create_label_ids
+    if params[:label_ids]
+      params[:label_ids] = labels_service.filter_labels_ids_in_param(:label_ids)
+    elsif params[:labels]
+      params[:label_ids] = labels_service.find_or_create_by_titles.map(&:id)
+    end
   end
 
   # rubocop: disable CodeReuse/ActiveRecord
@@ -101,6 +105,10 @@ class IssuableBaseService < BaseService
     end.compact
   end
 
+  def labels_service
+    @labels_service ||= ::Labels::AvailableLabelsService.new(current_user, parent, params)
+  end
+
   def process_label_ids(attributes, existing_label_ids: nil)
     label_ids = attributes.delete(:label_ids)
     add_label_ids = attributes.delete(:add_label_ids)
@@ -118,10 +126,6 @@ class IssuableBaseService < BaseService
     new_label_ids
   end
 
-  def available_labels
-    @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: true).execute
-  end
-
   def handle_quick_actions_on_create(issuable)
     merge_quick_actions_into_params!(issuable)
   end

app/services/labels/available_labels_service.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+module Labels
+  class AvailableLabelsService
+    attr_reader :current_user, :parent, :params
+
+    def initialize(current_user, parent, params)
+      @current_user = current_user
+      @parent = parent
+      @params = params
+    end
+
+    def find_or_create_by_titles
+      labels = params.delete(:labels)
+
+      return [] unless labels
+
+      labels = labels.split(',') if labels.is_a?(String)
+
+      labels.map do |label_name|
+        label = Labels::FindOrCreateService.new(
+          current_user,
+          parent,
+          include_ancestor_groups: true,
+          title: label_name.strip,
+          available_labels: available_labels
+        ).execute
+
+        label
+      end.compact
+    end
+
+    def filter_labels_ids_in_param(key)
+      return [] if params[key].to_a.empty?
+
+      # rubocop:disable CodeReuse/ActiveRecord
+      available_labels.by_ids(params[key]).pluck(:id)
+      # rubocop:enable CodeReuse/ActiveRecord
+    end
+
+    private
+
+    def available_labels
+      @available_labels ||= LabelsFinder.new(current_user, finder_params).execute
+    end
+
+    def finder_params
+      params = { include_ancestor_groups: true }
+
+      case parent
+      when Group
+        params[:group_id] = parent.id
+        params[:only_group_labels] = true
+      when Project
+        params[:project_id] = parent.id
+      end
+
+      params
+    end
+  end
+end

app/views/projects/issues/show.html.haml

@@ -77,8 +77,9 @@
     #merge-requests{ data: { url: referenced_merge_requests_project_issue_path(@project, @issue) } }
       // This element is filled in using JavaScript.
 
-    #related-branches{ data: { url: related_branches_project_issue_path(@project, @issue) } }
+    - if can?(current_user, :download_code, @project)
-      // This element is filled in using JavaScript.
+      #related-branches{ data: { url: related_branches_project_issue_path(@project, @issue) } }
+        // This element is filled in using JavaScript.
 
   .content-block.emoji-block.emoji-block-sticky
     .row

app/views/projects/merge_requests/conflicts/_submit_form.html.haml

@@ -6,7 +6,7 @@
   .form-group.row
     .col-md-4
       %h4= _('Resolve conflicts on source branch')
-      .resolve-info
+      .resolve-info{ "v-pre": true }
         = translation.html_safe
     .col-md-8
       %label.label-bold{ "for" => "commit-message" }

doc/administration/raketasks/uploads/sanitize.md

@@ -0,0 +1,62 @@
+# Uploads Sanitize tasks
+
+## Requirements
+
+You need `exiftool` installed on your system. If you installed GitLab:
+
+-   Using the Omnibus package, you're all set.
+-   From source, make sure `exiftool` is installed:
+
+    ```sh
+    # Debian/Ubuntu
+    sudo apt-get install libimage-exiftool-perl
+
+    # RHEL/CentOS
+    sudo yum install perl-Image-ExifTool
+    ```
+
+## Remove EXIF data from existing uploads
+
+Since 11.9 EXIF data are automatically stripped from JPG or TIFF image uploads.
+Because EXIF data may contain sensitive information (e.g. GPS location), you
+can remove EXIF data also from existing images which were uploaded before
+with the following command:
+
+```bash
+sudo RAILS_ENV=production -u git -H bundle exec rake gitlab:uploads:sanitize:remove_exif
+```
+
+This command by default runs in dry mode and it doesn't remove EXIF data. It can be used for
+checking if (and how many) images should be sanitized.
+
+The rake task accepts following parameters.
+
+Parameter | Type | Description
+--------- | ---- | -----------
+`start_id` | integer | Only uploads with equal or greater ID will be processed
+`stop_id` | integer | Only uploads with equal or smaller ID will be processed
+`dry_run` | boolean | Do not remove EXIF data, only check if EXIF data are present or not, default: true
+`sleep_time` | float | Pause for number of seconds after processing each image, default: 0.3 seconds
+
+If you have too many uploads, you can speed up sanitization by setting
+`sleep_time` to a lower value or by running multiple rake tasks in parallel,
+each with a separate range of upload IDs (by setting `start_id` and `stop_id`).
+
+To run the command without dry mode and remove EXIF data from all uploads, you can use:
+
+```bash
+sudo RAILS_ENV=production -u git -H bundle exec rake gitlab:uploads:sanitize:remove_exif[,,false,] 2>&1 | tee exif.log
+```
+
+To run the command without dry mode on uploads with ID between 100 and 5000 and pause for 0.1 second, you can use:
+
+```bash
+sudo RAILS_ENV=production -u git -H bundle exec rake gitlab:uploads:sanitize:remove_exif[100,5000,false,0.1] 2>&1 | tee exif.log
+```
+
+Because the output of commands will be probably long, the output is written also into exif.log file.
+
+If sanitization fails for an upload, an error message should be in the output of the rake task (typical reasons may
+be that the file is missing in the storage or it's not a valid image). Please
+[report](https://gitlab.com/gitlab-org/gitlab-ce/issues/new) any issues at `gitlab.com` and use
+prefix 'EXIF' in issue title with the error output and (if possible) the image.

doc/ci/yaml/README.md

@@ -255,6 +255,19 @@ job:
     - branches
 ```
 
+Pattern matching is case-sensitive by default. Use `i` flag modifier, like
+`/pattern/i` to make a pattern case-insensitive:
+
+```yaml
+job:
+  # use regexp
+  only:
+    - /^issue-.*$/i
+  # use special keyword
+  except:
+    - branches
+```
+
 In this example, `job` will run only for refs that are tagged, or if a build is
 explicitly requested via an API trigger or a [Pipeline Schedule][schedules]:
 

doc/raketasks/README.md

@@ -15,3 +15,4 @@ comments: false
 - [Import](import.md) of git repositories in bulk
 - [Rebuild authorized_keys file](http://docs.gitlab.com/ce/raketasks/maintenance.html#rebuild-authorized_keys-file) task for administrators
 - [Migrate Uploads](../administration/raketasks/uploads/migrate.md)
+- [Sanitize Uploads](../administration/raketasks/uploads/sanitize.md)

doc/update/11.7-to-11.8.md

@@ -30,8 +30,8 @@ sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production
 
 ### 3. Update Ruby
 
-NOTE: Beginning in GitLab 11.0, we only support Ruby 2.4 or higher, and dropped
+NOTE: Beginning in GitLab 11.6, we only support Ruby 2.5 or higher, and dropped
-support for Ruby 2.3. Be sure to upgrade if necessary.
+support for Ruby 2.4. Be sure to upgrade if necessary.
 
 You can check which version you are running with `ruby -v`.
 

ee/changelogs/unreleased/security-milestone-labels.yml

@@ -0,0 +1,5 @@
+---
+title: Check label_ids parent when updating issue board
+merge_request:
+author:
+type: security

lib/gitlab/ci/build/policy/refs.rb

@@ -35,8 +35,8 @@ module Gitlab
             # patterns can be matched only when branch or tag is used
             # the pattern matching does not work for merge requests pipelines
             if pipeline.branch? || pipeline.tag?
-              if pattern.first == "/" && pattern.last == "/"
+              if regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(pattern)
-                Regexp.new(pattern[1...-1]) =~ pipeline.ref
+                regexp.match?(pipeline.ref)
               else
                 pattern == pipeline.ref
               end

lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb

@@ -13,13 +13,13 @@ module Gitlab
             def initialize(regexp)
               @value = regexp
 
-              unless Gitlab::UntrustedRegexp.valid?(@value)
+              unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
                 raise Lexer::SyntaxError, 'Invalid regular expression!'
               end
             end
 
             def evaluate(variables = {})
-              Gitlab::UntrustedRegexp.fabricate(@value)
+              Gitlab::UntrustedRegexp::RubySyntax.fabricate!(@value)
             rescue RegexpError
               raise Expression::RuntimeError, 'Invalid regular expression!'
             end

lib/gitlab/config/entry/legacy_validation_helpers.rb

@@ -45,17 +45,15 @@ module Gitlab
         end
 
         def validate_regexp(value)
-          !value.nil? && Regexp.new(value.to_s) && true
+          Gitlab::UntrustedRegexp::RubySyntax.valid?(value)
-        rescue RegexpError, TypeError
-          false
         end
 
         def validate_string_or_regexp(value)
           return true if value.is_a?(Symbol)
           return false unless value.is_a?(String)
 
-          if value.first == '/' && value.last == '/'
+          if Gitlab::UntrustedRegexp::RubySyntax.matches_syntax?(value)
-            validate_regexp(value[1...-1])
+            validate_regexp(value)
           else
             true
           end

lib/gitlab/config/entry/validators.rb

@@ -120,17 +120,13 @@ module Gitlab
 
           private
 
-          def look_like_regexp?(value)
+          def matches_syntax?(value)
-            value.is_a?(String) && value.start_with?('/') &&
+            Gitlab::UntrustedRegexp::RubySyntax.matches_syntax?(value)
-              value.end_with?('/')
           end
 
           def validate_regexp(value)
-            look_like_regexp?(value) &&
+            matches_syntax?(value) &&
-              Regexp.new(value.to_s[1...-1]) &&
+              Gitlab::UntrustedRegexp::RubySyntax.valid?(value)
-              true
-          rescue RegexpError
-            false
           end
         end
 
@@ -149,7 +145,7 @@ module Gitlab
 
           def validate_string_or_regexp(value)
             return false unless value.is_a?(String)
-            return validate_regexp(value) if look_like_regexp?(value)
+            return validate_regexp(value) if matches_syntax?(value)
 
             true
           end

lib/gitlab/sanitizers/exif.rb

@@ -0,0 +1,158 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Sanitizers
+    class Exif
+      # these tags are not removed from the image
+      WHITELISTED_TAGS = %w(
+        ResolutionUnit
+        XResolution
+        YResolution
+        YCbCrSubSampling
+        YCbCrPositioning
+        BitsPerSample
+        ImageHeight
+        ImageWidth
+        ImageSize
+        Copyright
+        CopyrightNotice
+        Orientation
+      ).freeze
+
+      # these tags are common in exiftool output, these
+      # do not contain any sensitive information, but
+      # we don't need to preserve them when removing
+      # exif tags
+      IGNORED_TAGS = %w(
+        ColorComponents
+        EncodingProcess
+        ExifByteOrder
+        ExifToolVersion
+        JFIFVersion
+        Directory
+        FileAccessDate
+        FileInodeChangeDate
+        FileModifyDate
+        FileName
+        FilePermissions
+        FileSize
+        SourceFile
+        Megapixels
+        FileType
+        FileTypeExtension
+        MIMEType
+      ).freeze
+
+      ALLOWED_TAGS = WHITELISTED_TAGS + IGNORED_TAGS
+      EXCLUDE_PARAMS = WHITELISTED_TAGS.map { |tag| "-#{tag}" }
+
+      attr_reader :logger
+
+      def initialize(logger: Rails.logger)
+        @logger = logger
+      end
+
+      # rubocop: disable CodeReuse/ActiveRecord
+      def batch_clean(start_id: nil, stop_id: nil, dry_run: true, sleep_time: nil)
+        relation = Upload.where('lower(path) like ? or lower(path) like ? or lower(path) like ?',
+                                '%.jpg', '%.jpeg', '%.tiff')
+
+        logger.info "running in dry run mode, no images will be rewritten" if dry_run
+
+        find_params = {
+          start: start_id.present? ? start_id.to_i : nil,
+          finish: stop_id.present? ? stop_id.to_i : Upload.last&.id
+        }
+
+        relation.find_each(find_params) do |upload|
+          begin
+            clean(upload.build_uploader, dry_run: dry_run)
+            sleep sleep_time if sleep_time
+          rescue => err
+            logger.error "failed to sanitize #{upload_ref(upload)}: #{err.message}"
+            logger.debug err.backtrace.join("\n ")
+          end
+        end
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
+      def clean(uploader, dry_run: true)
+        Dir.mktmpdir('gitlab-exif') do |tmpdir|
+          src_path = fetch_upload_to_file(uploader, tmpdir)
+
+          to_remove = extra_tags(src_path)
+
+          if to_remove.empty?
+            logger.info "#{upload_ref(uploader.upload)}: only whitelisted tags present, skipping"
+            break
+          end
+
+          logger.info "#{upload_ref(uploader.upload)}: found exif tags to remove: #{to_remove}"
+
+          break if dry_run
+
+          remove_and_store(tmpdir, src_path, uploader)
+        end
+      end
+
+      def extra_tags(path)
+        exif_tags(path).keys - ALLOWED_TAGS
+      end
+
+      private
+
+      def remove_and_store(tmpdir, src_path, uploader)
+        exec_remove_exif!(src_path)
+        logger.info "#{upload_ref(uploader.upload)}: exif removed, storing"
+        File.open(src_path, 'r') { |f| uploader.store!(f) }
+      end
+
+      def exec_remove_exif!(path)
+        # IPTC and XMP-iptcExt groups may keep copyright information so
+        # we always preserve them
+        cmd = ["exiftool", "-all=", "-tagsFromFile", "@", *EXCLUDE_PARAMS, "--IPTC:all", "--XMP-iptcExt:all", path]
+        output, status = Gitlab::Popen.popen(cmd)
+
+        if status != 0
+          raise "exiftool return code is #{status}: #{output}"
+        end
+
+        if File.size(path) == 0
+          raise "size of file is 0"
+        end
+
+        # exiftool creates backup of the original file in filename_original
+        old_path = "#{path}_original"
+        if File.size(path) == File.size(old_path)
+          raise "size of sanitized file is same as original size"
+        end
+      end
+
+      def fetch_upload_to_file(uploader, dir)
+        # upload is stored into the file with the original name - this filename
+        # is used by carrierwave when storing the file back to the storage
+        filename = File.join(dir, uploader.filename)
+
+        File.open(filename, 'w') do |file|
+          file.binmode
+          file.write uploader.read
+        end
+
+        filename
+      end
+
+      def upload_ref(upload)
+        "#{upload.id}:#{upload.path}"
+      end
+
+      def exif_tags(path)
+        cmd = ["exiftool", "-all", "-j", "-sort", "--IPTC:all", "--XMP-iptcExt:all", path]
+        output, status = Gitlab::Popen.popen(cmd)
+
+        raise "failed to get exif tags: #{output}" if status != 0
+
+        JSON.parse(output).first
+      end
+    end
+  end
+end

lib/gitlab/untrusted_regexp.rb

@@ -35,6 +35,10 @@ module Gitlab
       matches
     end
 
+    def match?(text)
+      text.present? && scan(text).present?
+    end
+
     def replace(text, rewrite)
       RE2.Replace(text, regexp, rewrite)
     end
@@ -43,37 +47,6 @@ module Gitlab
       self.source == other.source
     end
 
-    # Handles regular expressions with the preferred RE2 library where possible
-    # via UntustedRegex. Falls back to Ruby's built-in regular expression library
-    # when the syntax would be invalid in RE2.
-    #
-    # One difference between these is `(?m)` multi-line mode. Ruby regex enables
-    # this by default, but also handles `^` and `$` differently.
-    # See: https://www.regular-expressions.info/modifiers.html
-    def self.with_fallback(pattern, multiline: false)
-      UntrustedRegexp.new(pattern, multiline: multiline)
-    rescue RegexpError
-      Regexp.new(pattern)
-    end
-
-    def self.valid?(pattern)
-      !!self.fabricate(pattern)
-    rescue RegexpError
-      false
-    end
-
-    def self.fabricate(pattern)
-      matches = pattern.match(%r{^/(?<regexp>.+)/(?<flags>[ismU]*)$})
-
-      raise RegexpError, 'Invalid regular expression!' if matches.nil?
-
-      expression = matches[:regexp]
-      flags = matches[:flags]
-      expression.prepend("(?#{flags})") if flags.present?
-
-      self.new(expression, multiline: false)
-    end
-
     private
 
     attr_reader :regexp

lib/gitlab/untrusted_regexp/ruby_syntax.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+module Gitlab
+  class UntrustedRegexp
+    # This class implements support for Ruby syntax of regexps
+    # and converts that to RE2 representation:
+    # /<regexp>/<flags>
+    class RubySyntax
+      PATTERN = %r{^/(?<regexp>.+)/(?<flags>[ismU]*)$}.freeze
+
+      # Checks if pattern matches a regexp pattern
+      # but does not enforce it's validity
+      def self.matches_syntax?(pattern)
+        pattern.is_a?(String) && pattern.match(PATTERN).present?
+      end
+
+      # The regexp can match the pattern `/.../`, but may not be fabricatable:
+      # it can be invalid or incomplete: `/match ( string/`
+      def self.valid?(pattern)
+        !!self.fabricate(pattern)
+      end
+
+      def self.fabricate(pattern)
+        self.fabricate!(pattern)
+      rescue RegexpError
+        nil
+      end
+
+      def self.fabricate!(pattern)
+        raise RegexpError, 'Pattern is not string!' unless pattern.is_a?(String)
+
+        matches = pattern.match(PATTERN)
+        raise RegexpError, 'Invalid regular expression!' if matches.nil?
+
+        expression = matches[:regexp]
+        flags = matches[:flags]
+        expression.prepend("(?#{flags})") if flags.present?
+
+        UntrustedRegexp.new(expression, multiline: false)
+      end
+    end
+  end
+end

lib/tasks/gitlab/uploads/sanitize.rake

@@ -0,0 +1,18 @@
+namespace :gitlab do
+  namespace :uploads do
+    namespace :sanitize do
+      desc 'GitLab | Uploads | Remove EXIF from images.'
+      task :remove_exif, [:start_id, :stop_id, :dry_run, :sleep_time] => :environment do |task, args|
+        args.with_defaults(dry_run: 'true')
+        args.with_defaults(sleep_time: 0.3)
+
+        logger = Logger.new(STDOUT)
+
+        sanitizer = Gitlab::Sanitizers::Exif.new(logger: logger)
+        sanitizer.batch_clean(start_id: args.start_id, stop_id: args.stop_id,
+                              dry_run: args.dry_run != 'false',
+                              sleep_time: args.sleep_time.to_f)
+      end
+    end
+  end
+end

spec/controllers/projects_controller_spec.rb

@@ -369,6 +369,23 @@ describe ProjectsController do
         end
       end
 
+      it 'does not update namespace' do
+        controller.instance_variable_set(:@project, project)
+
+        params = {
+          namespace_id: 'test'
+        }
+
+        expect do
+          put :update,
+            params: {
+              namespace_id: project.namespace,
+              id: project.id,
+              project: params
+            }
+        end.not_to change { project.namespace.reload }
+      end
+
       def update_project(**parameters)
         put :update,
             params: {

spec/features/issues/user_creates_branch_and_merge_request_spec.rb

@@ -1,6 +1,7 @@
 require 'rails_helper'
 
 describe 'User creates branch and merge request on issue page', :js do
+  let(:membership_level) { :developer }
   let(:user) { create(:user) }
   let!(:project) { create(:project, :repository) }
   let(:issue) { create(:issue, project: project, title: 'Cherry-Coloured Funk') }
@@ -17,7 +18,7 @@ describe 'User creates branch and merge request on issue page', :js do
 
   context 'when signed in' do
     before do
-      project.add_developer(user)
+      project.add_user(user, membership_level)
 
       sign_in(user)
     end
@@ -167,6 +168,39 @@ describe 'User creates branch and merge request on issue page', :js do
         expect(page).not_to have_css('.create-mr-dropdown-wrap')
       end
     end
+
+    context 'when related branch exists' do
+      let!(:project) { create(:project, :repository, :private) }
+      let(:branch_name) { "#{issue.iid}-foo" }
+
+      before do
+        project.repository.create_branch(branch_name, 'master')
+
+        visit project_issue_path(project, issue)
+      end
+
+      context 'when user is developer' do
+        it 'shows related branches' do
+          expect(page).to have_css('#related-branches')
+
+          wait_for_requests
+
+          expect(page).to have_content(branch_name)
+        end
+      end
+
+      context 'when user is guest' do
+        let(:membership_level) { :guest }
+
+        it 'does not show related branches' do
+          expect(page).not_to have_css('#related-branches')
+
+          wait_for_requests
+
+          expect(page).not_to have_content(branch_name)
+        end
+      end
+    end
   end
 
   private

spec/features/merge_request/user_resolves_conflicts_spec.rb

@@ -164,6 +164,21 @@ describe 'Merge request > User resolves conflicts', :js do
         expect(page).to have_content('Gregor Samsa woke from troubled dreams')
       end
     end
+
+    context "with malicious branch name" do
+      let(:bad_branch_name) { "malicious-branch-{{toString.constructor('alert(/xss/)')()}}" }
+      let(:branch) { project.repository.create_branch(bad_branch_name, 'conflict-resolvable') }
+      let(:merge_request) { create_merge_request(branch.name) }
+
+      before do
+        visit project_merge_request_path(project, merge_request)
+        click_link('conflicts', href: %r{/conflicts\Z})
+      end
+
+      it "renders bad name without xss issues" do
+        expect(find('.resolve-conflicts-form .resolve-info')).to have_content(bad_branch_name)
+      end
+    end
   end
 
   UNRESOLVABLE_CONFLICTS = {

spec/javascripts/pdf/index_spec.js

@@ -1,11 +1,11 @@
 import Vue from 'vue';
-import { PDFJS } from 'vendor/pdf';
+import { GlobalWorkerOptions } from 'vendor/pdf';
 import workerSrc from 'vendor/pdf.worker.min';
 
 import PDFLab from '~/pdf/index.vue';
 import pdf from '../fixtures/blob/pdf/test.pdf';
 
-PDFJS.workerSrc = workerSrc;
+GlobalWorkerOptions.workerSrc = workerSrc;
 const Component = Vue.extend(PDFLab);
 
 describe('PDF component', () => {

spec/javascripts/pdf/page_spec.js

@@ -12,7 +12,7 @@ describe('Page component', () => {
   let testPage;
 
   beforeEach(done => {
-    pdfjsLib.PDFJS.workerSrc = workerSrc;
+    pdfjsLib.GlobalWorkerOptions.workerSrc = workerSrc;
     pdfjsLib
       .getDocument(testPDF)
       .then(pdf => pdf.getPage(1))

spec/lib/gitlab/ci/build/policy/refs_spec.rb

@@ -78,10 +78,23 @@ describe Gitlab::Ci::Build::Policy::Refs do
           .to be_satisfied_by(pipeline)
       end
 
+      it 'is satisfied when case-insensitive regexp matches pipeline ref' do
+        expect(described_class.new(['/DOCS-.*/i']))
+          .to be_satisfied_by(pipeline)
+      end
+
       it 'is not satisfied when regexp does not match pipeline ref' do
         expect(described_class.new(['/fix-.*/']))
           .not_to be_satisfied_by(pipeline)
       end
     end
+
+    context 'malicious regexp' do
+      let(:pipeline) { build_stubbed(:ci_pipeline, ref: malicious_text) }
+
+      subject { described_class.new([malicious_regexp_ruby]) }
+
+      include_examples 'malicious regexp'
+    end
   end
 end

spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb

@@ -85,7 +85,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do
     end
 
     it 'raises error if evaluated regexp is not valid' do
-      allow(Gitlab::UntrustedRegexp).to receive(:valid?).and_return(true)
+      allow(Gitlab::UntrustedRegexp::RubySyntax).to receive(:valid?).and_return(true)
 
       regexp = described_class.new('/invalid ( .*/')
 

spec/lib/gitlab/ci/trace/stream_spec.rb

@@ -414,7 +414,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
 
       context 'malicious regexp' do
         let(:data) { malicious_text }
-        let(:regex) { malicious_regexp }
+        let(:regex) { malicious_regexp_re2 }
 
         include_examples 'malicious regexp'
       end

spec/lib/gitlab/route_map_spec.rb

@@ -60,7 +60,7 @@ describe Gitlab::RouteMap do
 
       subject do
         map = described_class.new(<<-"MAP".strip_heredoc)
-        - source: '#{malicious_regexp}'
+        - source: '#{malicious_regexp_re2}'
           public: '/'
         MAP
 

spec/lib/gitlab/sanitizers/exif_spec.rb

@@ -0,0 +1,120 @@
+require 'spec_helper'
+
+describe Gitlab::Sanitizers::Exif do
+  let(:sanitizer) { described_class.new }
+
+  describe '#batch_clean' do
+    context 'with image uploads' do
+      let!(:uploads) { create_list(:upload, 3, :with_file, :issuable_upload) }
+
+      it 'processes all uploads if range ID is not set' do
+        expect(sanitizer).to receive(:clean).exactly(3).times
+
+        sanitizer.batch_clean
+      end
+
+      it 'processes only uploads in the selected range' do
+        expect(sanitizer).to receive(:clean).once
+
+        sanitizer.batch_clean(start_id: uploads[1].id, stop_id: uploads[1].id)
+      end
+
+      it 'pauses if sleep_time is set' do
+        expect(sanitizer).to receive(:sleep).exactly(3).times.with(1.second)
+        expect(sanitizer).to receive(:clean).exactly(3).times
+
+        sanitizer.batch_clean(sleep_time: 1)
+      end
+    end
+
+    it 'filters only jpg/tiff images' do
+      create(:upload, path: 'filename.jpg')
+      create(:upload, path: 'filename.jpeg')
+      create(:upload, path: 'filename.JPG')
+      create(:upload, path: 'filename.tiff')
+      create(:upload, path: 'filename.TIFF')
+      create(:upload, path: 'filename.png')
+      create(:upload, path: 'filename.txt')
+
+      expect(sanitizer).to receive(:clean).exactly(5).times
+      sanitizer.batch_clean
+    end
+  end
+
+  describe '#clean' do
+    let(:uploader) { create(:upload, :with_file, :issuable_upload).build_uploader }
+
+    context "no dry run" do
+      it "removes exif from the image" do
+        uploader.store!(fixture_file_upload('spec/fixtures/rails_sample.jpg'))
+
+        original_upload = uploader.upload
+        expected_args = ["exiftool", "-all=", "-tagsFromFile", "@", *Gitlab::Sanitizers::Exif::EXCLUDE_PARAMS, "--IPTC:all", "--XMP-iptcExt:all", kind_of(String)]
+
+        expect(sanitizer).to receive(:extra_tags).and_return(["", 0])
+        expect(sanitizer).to receive(:exec_remove_exif!).once.and_call_original
+        expect(uploader).to receive(:store!).and_call_original
+        expect(Gitlab::Popen).to receive(:popen).with(expected_args) do |args|
+          File.write("#{args.last}_original", "foo") if args.last.start_with?(Dir.tmpdir)
+
+          [expected_args, 0]
+        end
+
+        sanitizer.clean(uploader, dry_run: false)
+
+        expect(uploader.upload.id).not_to eq(original_upload.id)
+        expect(uploader.upload.path).to eq(original_upload.path)
+      end
+
+      it "ignores image without exif" do
+        expected_args = ["exiftool", "-all", "-j", "-sort", "--IPTC:all", "--XMP-iptcExt:all", kind_of(String)]
+
+        expect(Gitlab::Popen).to receive(:popen).with(expected_args).and_return(["[{}]", 0])
+        expect(sanitizer).not_to receive(:exec_remove_exif!)
+        expect(uploader).not_to receive(:store!)
+
+        sanitizer.clean(uploader, dry_run: false)
+      end
+
+      it "raises an error if the exiftool fails with an error" do
+        expect(Gitlab::Popen).to receive(:popen).and_return(["error", 1])
+
+        expect { sanitizer.clean(uploader, dry_run: false) }.to raise_exception(RuntimeError, "failed to get exif tags: error")
+      end
+    end
+
+    context "dry run" do
+      it "doesn't change the image" do
+        expect(sanitizer).to receive(:extra_tags).and_return({ 'foo' => 'bar' })
+        expect(sanitizer).not_to receive(:exec_remove_exif!)
+        expect(uploader).not_to receive(:store!)
+
+        sanitizer.clean(uploader, dry_run: true)
+      end
+    end
+  end
+
+  describe "#extra_tags" do
+    it "returns a list of keys for exif file" do
+      tags = '[{
+                "DigitalSourceType": "some source",
+                "ImageHeight": 654
+              }]'
+
+      expect(Gitlab::Popen).to receive(:popen).and_return([tags, 0])
+
+      expect(sanitizer.extra_tags('filename')).not_to be_empty
+    end
+
+    it "returns an empty list for file with only whitelisted and ignored tags" do
+      tags = '[{
+                "ImageHeight": 654,
+                "Megapixels": 0.641
+              }]'
+
+      expect(Gitlab::Popen).to receive(:popen).and_return([tags, 0])
+
+      expect(sanitizer.extra_tags('some file')).to be_empty
+    end
+  end
+end

spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb

@@ -0,0 +1,72 @@
+require 'fast_spec_helper'
+require 'support/shared_examples/malicious_regexp_shared_examples'
+
+describe Gitlab::UntrustedRegexp::RubySyntax do
+  describe '.matches_syntax?' do
+    it 'returns true if regexp is valid' do
+      expect(described_class.matches_syntax?('/some .* thing/'))
+        .to be true
+    end
+
+    it 'returns true if regexp is invalid, but resembles regexp' do
+      expect(described_class.matches_syntax?('/some ( thing/'))
+        .to be true
+    end
+  end
+
+  describe '.valid?' do
+    it 'returns true if regexp is valid' do
+      expect(described_class.valid?('/some .* thing/'))
+        .to be true
+    end
+
+    it 'returns false if regexp is invalid' do
+      expect(described_class.valid?('/some ( thing/'))
+        .to be false
+    end
+  end
+
+  describe '.fabricate' do
+    context 'when regexp is valid' do
+      it 'fabricates regexp without flags' do
+        expect(described_class.fabricate('/some .* thing/')).not_to be_nil
+      end
+    end
+
+    context 'when regexp is a raw pattern' do
+      it 'returns error' do
+        expect(described_class.fabricate('some .* thing')).to be_nil
+      end
+    end
+  end
+
+  describe '.fabricate!' do
+    context 'when regexp is using /regexp/ scheme with flags' do
+      it 'fabricates regexp with a single flag' do
+        regexp = described_class.fabricate!('/something/i')
+
+        expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?i)something')
+        expect(regexp.scan('SOMETHING')).to be_one
+      end
+
+      it 'fabricates regexp with multiple flags' do
+        regexp = described_class.fabricate!('/something/im')
+
+        expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?im)something')
+      end
+
+      it 'fabricates regexp without flags' do
+        regexp = described_class.fabricate!('/something/')
+
+        expect(regexp).to eq Gitlab::UntrustedRegexp.new('something')
+      end
+    end
+
+    context 'when regexp is a raw pattern' do
+      it 'raises an error' do
+        expect { described_class.fabricate!('some .* thing') }
+          .to raise_error(RegexpError)
+      end
+    end
+  end
+end

spec/lib/gitlab/untrusted_regexp_spec.rb

@@ -2,48 +2,6 @@ require 'fast_spec_helper'
 require 'support/shared_examples/malicious_regexp_shared_examples'
 
 describe Gitlab::UntrustedRegexp do
-  describe '.valid?' do
-    it 'returns true if regexp is valid' do
-      expect(described_class.valid?('/some ( thing/'))
-        .to be false
-    end
-
-    it 'returns true if regexp is invalid' do
-      expect(described_class.valid?('/some .* thing/'))
-        .to be true
-    end
-  end
-
-  describe '.fabricate' do
-    context 'when regexp is using /regexp/ scheme with flags' do
-      it 'fabricates regexp with a single flag' do
-        regexp = described_class.fabricate('/something/i')
-
-        expect(regexp).to eq described_class.new('(?i)something')
-        expect(regexp.scan('SOMETHING')).to be_one
-      end
-
-      it 'fabricates regexp with multiple flags' do
-        regexp = described_class.fabricate('/something/im')
-
-        expect(regexp).to eq described_class.new('(?im)something')
-      end
-
-      it 'fabricates regexp without flags' do
-        regexp = described_class.fabricate('/something/')
-
-        expect(regexp).to eq described_class.new('something')
-      end
-    end
-
-    context 'when regexp is a raw pattern' do
-      it 'raises an error' do
-        expect { described_class.fabricate('some .* thing') }
-          .to raise_error(RegexpError)
-      end
-    end
-  end
-
   describe '#initialize' do
     subject { described_class.new(pattern) }
 
@@ -92,11 +50,41 @@ describe Gitlab::UntrustedRegexp do
     end
   end
 
-  describe '#scan' do
+  describe '#match?' do
-    subject { described_class.new(regexp).scan(text) }
+    subject { described_class.new(regexp).match?(text) }
+
     context 'malicious regexp' do
       let(:text) { malicious_text }
-      let(:regexp) { malicious_regexp }
+      let(:regexp) { malicious_regexp_re2 }
+
+      include_examples 'malicious regexp'
+    end
+
+    context 'matching regexp' do
+      let(:regexp) { 'foo' }
+      let(:text) { 'foo' }
+
+      it 'returns an array of nil matches' do
+        is_expected.to eq(true)
+      end
+    end
+
+    context 'non-matching regexp' do
+      let(:regexp) { 'boo' }
+      let(:text) { 'foo' }
+
+      it 'returns an array of nil matches' do
+        is_expected.to eq(false)
+      end
+    end
+  end
+
+  describe '#scan' do
+    subject { described_class.new(regexp).scan(text) }
+
+    context 'malicious regexp' do
+      let(:text) { malicious_text }
+      let(:regexp) { malicious_regexp_re2 }
 
       include_examples 'malicious regexp'
     end

spec/policies/project_policy_spec.rb

@@ -15,7 +15,7 @@ describe ProjectPolicy do
       read_project_for_iids read_issue_iid read_label
       read_milestone read_project_snippet read_project_member read_note
       create_project create_issue create_note upload_file create_merge_request_in
-      award_emoji read_release
+      award_emoji
     ]
   end
 
@@ -24,7 +24,7 @@ describe ProjectPolicy do
       download_code fork_project create_project_snippet update_issue
       admin_issue admin_label admin_list read_commit_status read_build
       read_container_image read_pipeline read_environment read_deployment
-      read_merge_request download_wiki_code read_sentry_issue
+      read_merge_request download_wiki_code read_sentry_issue read_release
     ]
   end
 

spec/requests/api/releases_spec.rb

@@ -4,12 +4,14 @@ describe API::Releases do
   let(:project) { create(:project, :repository, :private) }
   let(:maintainer) { create(:user) }
   let(:reporter) { create(:user) }
+  let(:guest) { create(:user) }
   let(:non_project_member) { create(:user) }
   let(:commit) { create(:commit, project: project) }
 
   before do
     project.add_maintainer(maintainer)
     project.add_reporter(reporter)
+    project.add_guest(guest)
 
     project.repository.add_tag(maintainer, 'v0.1', commit.id)
     project.repository.add_tag(maintainer, 'v0.2', commit.id)
@@ -66,6 +68,24 @@ describe API::Releases do
       end
     end
 
+    context 'when user is a guest' do
+      it 'responds 403 Forbidden' do
+        get api("/projects/#{project.id}/releases", guest)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      context 'when project is public' do
+        let(:project) { create(:project, :repository, :public) }
+
+        it 'responds 200 OK' do
+          get api("/projects/#{project.id}/releases", guest)
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+    end
+
     context 'when user is not a project member' do
       it 'cannot find the project' do
         get api("/projects/#{project.id}/releases", non_project_member)
@@ -189,6 +209,24 @@ describe API::Releases do
           end
         end
       end
+
+      context 'when user is a guest' do
+        it 'responds 403 Forbidden' do
+          get api("/projects/#{project.id}/releases/v0.1", guest)
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+
+        context 'when project is public' do
+          let(:project) { create(:project, :repository, :public) }
+
+          it 'responds 200 OK' do
+            get api("/projects/#{project.id}/releases/v0.1", guest)
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
+        end
+      end
     end
 
     context 'when specified tag is not found in the project' do

spec/services/labels/available_labels_service_spec.rb

@@ -0,0 +1,86 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Labels::AvailableLabelsService do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :public, group: group) }
+  let(:group) { create(:group) }
+
+  let(:project_label) { create(:label, project: project) }
+  let(:other_project_label) { create(:label) }
+  let(:group_label) { create(:group_label, group: group) }
+  let(:other_group_label) { create(:group_label) }
+  let(:labels) { [project_label, other_project_label, group_label, other_group_label] }
+
+  context '#find_or_create_by_titles' do
+    let(:label_titles) { labels.map(&:title).push('non existing title') }
+
+    context 'when parent is a project' do
+      context 'when a user is not a project member' do
+        it 'returns only relevant label ids' do
+          result = described_class.new(user, project, labels: label_titles).find_or_create_by_titles
+
+          expect(result).to match_array([project_label, group_label])
+        end
+      end
+
+      context 'when a user is a project member' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'creates new labels for not found titles' do
+          result = described_class.new(user, project, labels: label_titles).find_or_create_by_titles
+
+          expect(result.count).to eq(5)
+          expect(result).to include(project_label, group_label)
+          expect(result).not_to include(other_project_label, other_group_label)
+        end
+      end
+    end
+
+    context 'when parent is a group' do
+      context 'when a user is not a group member' do
+        it 'returns only relevant label ids' do
+          result = described_class.new(user, group, labels: label_titles).find_or_create_by_titles
+
+          expect(result).to match_array([group_label])
+        end
+      end
+
+      context 'when a user is a group member' do
+        before do
+          group.add_developer(user)
+        end
+
+        it 'creates new labels for not found titles' do
+          result = described_class.new(user, group, labels: label_titles).find_or_create_by_titles
+
+          expect(result.count).to eq(5)
+          expect(result).to include(group_label)
+          expect(result).not_to include(project_label, other_project_label, other_group_label)
+        end
+      end
+    end
+  end
+
+  context '#filter_labels_ids_in_param' do
+    let(:label_ids) { labels.map(&:id).push(99999) }
+
+    context 'when parent is a project' do
+      it 'returns only relevant label ids' do
+        result = described_class.new(user, project, ids: label_ids).filter_labels_ids_in_param(:ids)
+
+        expect(result).to match_array([project_label.id, group_label.id])
+      end
+    end
+
+    context 'when parent is a group' do
+      it 'returns only relevant label ids' do
+        result = described_class.new(user, group, ids: label_ids).filter_labels_ids_in_param(:ids)
+
+        expect(result).to match_array([group_label.id])
+      end
+    end
+  end
+end

spec/support/shared_examples/malicious_regexp_shared_examples.rb

@@ -2,7 +2,8 @@ require 'timeout'
 
 shared_examples 'malicious regexp' do
   let(:malicious_text) { 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!' }
-  let(:malicious_regexp) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' }
+  let(:malicious_regexp_re2) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' }
+  let(:malicious_regexp_ruby) { '/^(([a-z])+.)+[A-Z]([a-z])+$/i' }
 
   it 'takes under a second' do
     expect { Timeout.timeout(1) { subject } }.not_to raise_error