Merge branch 'sh-backport-10-3-4-security-fixes' into 'master'

Backport 10.3.4 security fixes into master

See merge request gitlab-org/gitlab-ce!16509

14 files changed, 140 insertions, 53 deletions

diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb
index 281269b1190..b0b7b50998f 100644
--- a/lib/api/deploy_keys.rb
+++ b/lib/api/deploy_keys.rb
@@ -4,6 +4,16 @@ module API
 
     before { authenticate! }
 
+    helpers do
+      def add_deploy_keys_project(project, attrs = {})
+        project.deploy_keys_projects.create(attrs)
+      end
+
+      def find_by_deploy_key(project, key_id)
+        project.deploy_keys_projects.find_by!(deploy_key: key_id)
+      end
+    end
+
     desc 'Return all deploy keys'
     params do
       use :pagination
@@ -21,28 +31,31 @@ module API
       before { authorize_admin_project }
 
       desc "Get a specific project's deploy keys" do
-        success Entities::SSHKey
+        success Entities::DeployKeysProject
       end
       params do
         use :pagination
       end
       get ":id/deploy_keys" do
-        present paginate(user_project.deploy_keys), with: Entities::SSHKey
+        keys = user_project.deploy_keys_projects.preload(:deploy_key)
+
+        present paginate(keys), with: Entities::DeployKeysProject
       end
 
       desc 'Get single deploy key' do
-        success Entities::SSHKey
+        success Entities::DeployKeysProject
       end
       params do
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
       end
       get ":id/deploy_keys/:key_id" do
-        key = user_project.deploy_keys.find params[:key_id]
-        present key, with: Entities::SSHKey
+        key = find_by_deploy_key(user_project, params[:key_id])
+
+        present key, with: Entities::DeployKeysProject
       end
 
       desc 'Add new deploy key to currently authenticated user' do
-        success Entities::SSHKey
+        success Entities::DeployKeysProject
       end
       params do
         requires :key, type: String, desc: 'The new deploy key'
@@ -53,24 +66,31 @@ module API
         params[:key].strip!
 
         # Check for an existing key joined to this project
-        key = user_project.deploy_keys.find_by(key: params[:key])
+        key = user_project.deploy_keys_projects
+                          .joins(:deploy_key)
+                          .find_by(keys: { key: params[:key] })
+
         if key
-          present key, with: Entities::SSHKey
+          present key, with: Entities::DeployKeysProject
           break
         end
 
         # Check for available deploy keys in other projects
         key = current_user.accessible_deploy_keys.find_by(key: params[:key])
         if key
-          user_project.deploy_keys << key
-          present key, with: Entities::SSHKey
+          added_key = add_deploy_keys_project(user_project, deploy_key: key, can_push: !!params[:can_push])
+
+          present added_key, with: Entities::DeployKeysProject
           break
         end
 
         # Create a new deploy key
-        key = DeployKey.new(declared_params(include_missing: false))
-        if key.valid? && user_project.deploy_keys << key
-          present key, with: Entities::SSHKey
+        key_attributes = { can_push: !!params[:can_push],
+                           deploy_key_attributes: declared_params.except(:can_push) }
+        key = add_deploy_keys_project(user_project, key_attributes)
+
+        if key.valid?
+          present key, with: Entities::DeployKeysProject
         else
           render_validation_error!(key)
         end
@@ -86,14 +106,21 @@ module API
         at_least_one_of :title, :can_push
       end
       put ":id/deploy_keys/:key_id" do
-        key = DeployKey.find(params.delete(:key_id))
+        deploy_keys_project = find_by_deploy_key(user_project, params[:key_id])
 
-        authorize!(:update_deploy_key, key)
+        authorize!(:update_deploy_key, deploy_keys_project.deploy_key)
 
-        if key.update_attributes(declared_params(include_missing: false))
-          present key, with: Entities::SSHKey
+        can_push = params[:can_push].nil? ? deploy_keys_project.can_push : params[:can_push]
+        title = params[:title] || deploy_keys_project.deploy_key.title
+
+        result = deploy_keys_project.update_attributes(can_push: can_push,
+                                                       deploy_key_attributes: { id: params[:key_id],
+                                                                                title: title })
+
+        if result
+          present deploy_keys_project, with: Entities::DeployKeysProject
         else
-          render_validation_error!(key)
+          render_validation_error!(deploy_keys_project)
         end
       end
 
@@ -122,7 +149,7 @@ module API
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
       end
       delete ":id/deploy_keys/:key_id" do
-        key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id])
+        key = user_project.deploy_keys.find(params[:key_id])
         not_found!('Deploy Key') unless key
 
         destroy_conditionally!(key)
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index c4ef2c74658..971cb83a2d9 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -554,13 +554,18 @@ module API
     end
 
     class SSHKey < Grape::Entity
-      expose :id, :title, :key, :created_at, :can_push
+      expose :id, :title, :key, :created_at
     end
 
     class SSHKeyWithUser < SSHKey
       expose :user, using: Entities::UserPublic
     end
 
+    class DeployKeysProject < Grape::Entity
+      expose :deploy_key, merge: true, using: Entities::SSHKey
+      expose :can_push
+    end
+
     class GPGKey < Grape::Entity
       expose :id, :key, :created_at
     end
@@ -714,10 +719,7 @@ module API
       expose :job_events
       # Expose serialized properties
       expose :properties do |service, options|
-        field_names = service.fields
-          .select { |field| options[:include_passwords] || field[:type] != 'password' }
-          .map { |field| field[:name] }
-        service.properties.slice(*field_names)
+        service.properties.slice(*service.api_field_names)
       end
     end
 
diff --git a/lib/api/services.rb b/lib/api/services.rb
index a7f44e2869c..51e33e2c686 100644
--- a/lib/api/services.rb
+++ b/lib/api/services.rb
@@ -785,7 +785,7 @@ module API
           service_params = declared_params(include_missing: false).merge(active: true)
 
           if service.update_attributes(service_params)
-            present service, with: Entities::ProjectService, include_passwords: current_user.admin?
+            present service, with: Entities::ProjectService
           else
             render_api_error!('400 Bad Request', 400)
           end
diff --git a/lib/api/v3/deploy_keys.rb b/lib/api/v3/deploy_keys.rb
index b90e2061da3..47e54ca85a5 100644
--- a/lib/api/v3/deploy_keys.rb
+++ b/lib/api/v3/deploy_keys.rb
@@ -3,6 +3,16 @@ module API
     class DeployKeys < Grape::API
       before { authenticate! }
 
+      helpers do
+        def add_deploy_keys_project(project, attrs = {})
+          project.deploy_keys_projects.create(attrs)
+        end
+
+        def find_by_deploy_key(project, key_id)
+          project.deploy_keys_projects.find_by!(deploy_key: key_id)
+        end
+      end
+
       get "deploy_keys" do
         authenticated_as_admin!
 
@@ -18,25 +28,28 @@ module API
 
         %w(keys deploy_keys).each do |path|
           desc "Get a specific project's deploy keys" do
-            success ::API::Entities::SSHKey
+            success ::API::Entities::DeployKeysProject
           end
           get ":id/#{path}" do
-            present user_project.deploy_keys, with: ::API::Entities::SSHKey
+            keys = user_project.deploy_keys_projects.preload(:deploy_key)
+
+            present keys, with: ::API::Entities::DeployKeysProject
           end
 
           desc 'Get single deploy key' do
-            success ::API::Entities::SSHKey
+            success ::API::Entities::DeployKeysProject
           end
           params do
             requires :key_id, type: Integer, desc: 'The ID of the deploy key'
           end
           get ":id/#{path}/:key_id" do
-            key = user_project.deploy_keys.find params[:key_id]
-            present key, with: ::API::Entities::SSHKey
+            key = find_by_deploy_key(user_project, params[:key_id])
+
+            present key, with: ::API::Entities::DeployKeysProject
           end
 
           desc 'Add new deploy key to currently authenticated user' do
-            success ::API::Entities::SSHKey
+            success ::API::Entities::DeployKeysProject
           end
           params do
             requires :key, type: String, desc: 'The new deploy key'
@@ -47,24 +60,31 @@ module API
             params[:key].strip!
 
             # Check for an existing key joined to this project
-            key = user_project.deploy_keys.find_by(key: params[:key])
+            key = user_project.deploy_keys_projects
+                              .joins(:deploy_key)
+                              .find_by(keys: { key: params[:key] })
+
             if key
-              present key, with: ::API::Entities::SSHKey
+              present key, with: ::API::Entities::DeployKeysProject
               break
             end
 
             # Check for available deploy keys in other projects
             key = current_user.accessible_deploy_keys.find_by(key: params[:key])
             if key
-              user_project.deploy_keys << key
-              present key, with: ::API::Entities::SSHKey
+              added_key = add_deploy_keys_project(user_project, deploy_key: key, can_push: !!params[:can_push])
+
+              present added_key, with: ::API::Entities::DeployKeysProject
               break
             end
 
             # Create a new deploy key
-            key = DeployKey.new(declared_params(include_missing: false))
-            if key.valid? && user_project.deploy_keys << key
-              present key, with: ::API::Entities::SSHKey
+            key_attributes = { can_push: !!params[:can_push],
+                               deploy_key_attributes: declared_params.except(:can_push) }
+            key = add_deploy_keys_project(user_project, key_attributes)
+
+            if key.valid?
+              present key, with: ::API::Entities::DeployKeysProject
             else
               render_validation_error!(key)
             end
diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb
index 64758dae7d3..2ccbb9da1c5 100644
--- a/lib/api/v3/entities.rb
+++ b/lib/api/v3/entities.rb
@@ -257,10 +257,7 @@ module API
         expose :job_events, as: :build_events
         # Expose serialized properties
         expose :properties do |service, options|
-          field_names = service.fields
-            .select { |field| options[:include_passwords] || field[:type] != 'password' }
-            .map { |field| field[:name] }
-          service.properties.slice(*field_names)
+          service.properties.slice(*service.api_field_names)
         end
       end
 
diff --git a/lib/api/v3/services.rb b/lib/api/v3/services.rb
index 44ed94d2869..20ca1021c71 100644
--- a/lib/api/v3/services.rb
+++ b/lib/api/v3/services.rb
@@ -622,7 +622,7 @@ module API
         end
         get ":id/services/:service_slug" do
           service = user_project.find_or_initialize_service(params[:service_slug].underscore)
-          present service, with: Entities::ProjectService, include_passwords: current_user.admin?
+          present service, with: Entities::ProjectService
         end
       end
 
diff --git a/lib/gitlab/ci/config/entry/validators.rb b/lib/gitlab/ci/config/entry/validators.rb
index eb606b57667..55658900628 100644
--- a/lib/gitlab/ci/config/entry/validators.rb
+++ b/lib/gitlab/ci/config/entry/validators.rb
@@ -64,10 +64,24 @@ module Gitlab
             include LegacyValidationHelpers
 
             def validate_each(record, attribute, value)
-              unless validate_string(value)
+              if validate_string(value)
+                validate_path(record, attribute, value)
+              else
                 record.errors.add(attribute, 'should be a string or symbol')
               end
             end
+
+            private
+
+            def validate_path(record, attribute, value)
+              path = CGI.unescape(value.to_s)
+
+              if path.include?('/')
+                record.errors.add(attribute, 'cannot contain the "/" character')
+              elsif path == '.' || path == '..'
+                record.errors.add(attribute, 'cannot be "." or ".."')
+              end
+            end
           end
 
           class RegexpValidator < ActiveModel::EachValidator
diff --git a/lib/gitlab/import_export/file_importer.rb b/lib/gitlab/import_export/file_importer.rb
index 989342389bc..5c971564a73 100644
--- a/lib/gitlab/import_export/file_importer.rb
+++ b/lib/gitlab/import_export/file_importer.rb
@@ -17,12 +17,16 @@ module Gitlab
       def import
         mkdir_p(@shared.export_path)
 
+        remove_symlinks!
+
         wait_for_archived_file do
           decompress_archive
         end
       rescue => e
         @shared.error(e)
         false
+      ensure
+        remove_symlinks!
       end
 
       private
@@ -43,7 +47,7 @@ module Gitlab
 
         raise Projects::ImportService::Error.new("Unable to decompress #{@archive_file} into #{@shared.export_path}") unless result
 
-        remove_symlinks!
+        result
       end
 
       def remove_symlinks!
diff --git a/lib/gitlab/import_export/saver.rb b/lib/gitlab/import_export/saver.rb
index 6130c124dd1..2daeba90a51 100644
--- a/lib/gitlab/import_export/saver.rb
+++ b/lib/gitlab/import_export/saver.rb
@@ -37,7 +37,7 @@ module Gitlab
       end
 
       def archive_file
-        @archive_file ||= File.join(@shared.export_path, '..', Gitlab::ImportExport.export_filename(project: @project))
+        @archive_file ||= File.join(@shared.archive_path, Gitlab::ImportExport.export_filename(project: @project))
       end
     end
   end
diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb
index 9fd0b709ef2..d03cbc880fd 100644
--- a/lib/gitlab/import_export/shared.rb
+++ b/lib/gitlab/import_export/shared.rb
@@ -9,7 +9,11 @@ module Gitlab
       end
 
       def export_path
-        @export_path ||= Gitlab::ImportExport.export_path(relative_path: opts[:relative_path])
+        @export_path ||= Gitlab::ImportExport.export_path(relative_path: relative_path)
+      end
+
+      def archive_path
+        @archive_path ||= Gitlab::ImportExport.export_path(relative_path: relative_archive_path)
       end
 
       def error(error)
@@ -21,6 +25,14 @@ module Gitlab
 
       private
 
+      def relative_path
+        File.join(opts[:relative_path], SecureRandom.hex)
+      end
+
+      def relative_archive_path
+        File.join(opts[:relative_path], '..')
+      end
+
       def error_out(message, caller)
         Rails.logger.error("Import/Export error raised on #{caller}: #{message}")
       end
diff --git a/lib/gitlab/o_auth.rb b/lib/gitlab/o_auth.rb
new file mode 100644
index 00000000000..5ad8d83bd6e
--- /dev/null
+++ b/lib/gitlab/o_auth.rb
@@ -0,0 +1,6 @@
+module Gitlab
+  module OAuth
+    SignupDisabledError = Class.new(StandardError)
+    SigninDisabledForProviderError = Class.new(StandardError)
+  end
+end
diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb
index d33f33d192f..fff9360ea27 100644
--- a/lib/gitlab/o_auth/user.rb
+++ b/lib/gitlab/o_auth/user.rb
@@ -5,8 +5,6 @@
 #
 module Gitlab
   module OAuth
-    SignupDisabledError = Class.new(StandardError)
-
     class User
       attr_accessor :auth_hash, :gl_user
 
@@ -29,7 +27,8 @@ module Gitlab
       end
 
       def save(provider = 'OAuth')
-        unauthorized_to_create unless gl_user
+        raise SigninDisabledForProviderError if oauth_provider_disabled?
+        raise SignupDisabledError unless gl_user
 
         block_after_save = needs_blocking?
 
@@ -226,8 +225,10 @@ module Gitlab
         Gitlab::AppLogger
       end
 
-      def unauthorized_to_create
-        raise SignupDisabledError
+      def oauth_provider_disabled?
+        Gitlab::CurrentSettings.current_application_settings
+                               .disabled_oauth_sign_in_sources
+                               .include?(auth_hash.provider)
       end
     end
   end
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 0002c7da8f1..7ab85e1c35c 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -67,7 +67,7 @@ module Gitlab
     end
 
     def build_trace_section_regex
-      @build_trace_section_regexp ||= /section_((?:start)|(?:end)):(\d+):([^\r]+)\r\033\[0K/.freeze
+      @build_trace_section_regexp ||= /section_((?:start)|(?:end)):(\d+):([a-zA-Z0-9_.-]+)\r\033\[0K/.freeze
     end
   end
 end
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index b3baaf036d8..fa22f0e37b2 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -27,6 +27,10 @@ module Gitlab
         .gsub(/(\A-+|-+\z)/, '')
     end
 
+    def remove_line_breaks(str)
+      str.gsub(/\r?\n/, '')
+    end
+
     def to_boolean(value)
       return value if [true, false].include?(value)
       return true if value =~ /^(true|t|yes|y|1|on)$/i