Merge dev.gitlab.org master into GitLab.com master

CHANGELOG.md

@@ -478,6 +478,24 @@ entry.
 - Removes EE differences for environment_item.vue.
+## 11.9.12 (2019-05-30)
+
+### Security (12 changes, 1 of them is from the community)
+
+- Protect Gitlab::HTTP against DNS rebinding attack.
+- Fix project visibility level validation. (Peter Marko)
+- Update Knative version.
+- Add DNS rebinding protection settings.
+- Prevent XSS injection in note imports.
+- Prevent invalid branch for merge request.
+- Filter relative links in wiki for XSS.
+- Fix confidential issue label disclosure on milestone view.
+- Fix url redaction for issue links.
+- Resolve: Milestones leaked via search API.
+- Prevent bypass of restriction disabling web password sign in.
+- Hide confidential issue title on unsubscribe for anonymous users.
+
+
 ## 11.9.10 (2019-04-26)
 ### Security (5 changes)

app/controllers/concerns/import_url_params.rb

+# frozen_string_literal: true
+
+module ImportUrlParams
+  def import_url_params
+    { import_url: import_params_to_full_url(params[:project]) }
+  end
+
+  def import_params_to_full_url(params)
+    Gitlab::UrlSanitizer.new(
+      params[:import_url],
+      credentials: {
+        user: params[:import_url_user],
+        password: params[:import_url_password]
+      }
+    ).full_url
+  end
+end

app/controllers/concerns/milestone_actions.rb

@@ -26,16 +26,22 @@ module MilestoneActions
     end
   end
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def labels
     respond_to do |format|
       format.html { redirect_to milestone_redirect_path }
       format.json do
+        milestone_labels = @milestone.issue_labels_visible_by_user(current_user)
+
         render json: tabs_json("shared/milestones/_labels_tab", {
-          labels: @milestone.labels.map { |label| label.present(issuable_subject: @milestone.parent) } # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          labels: milestone_labels.map do |label|
+            label.present(issuable_subject: @milestone.parent)
+          end
         })
       end
     end
   end
+  # rubocop:enable Gitlab/ModuleWithInstanceVariables
   private

app/controllers/projects/imports_controller.rb

@@ -2,6 +2,7 @@
 class Projects::ImportsController < Projects::ApplicationController
   include ContinueParams
+  include ImportUrlParams
   # Authorize
   before_action :authorize_admin_project!
@@ -67,10 +68,12 @@ class Projects::ImportsController < Projects::ApplicationController
   end
   def import_params_attributes
-    [:import_url]
+    []
   end
   def import_params
-    params.require(:project).permit(import_params_attributes)
+    params.require(:project)
+      .permit(import_params_attributes)
+      .merge(import_url_params)
   end
 end

app/controllers/projects_controller.rb

@@ -7,6 +7,7 @@ class ProjectsController < Projects::ApplicationController
   include PreviewMarkdown
   include SendFileUpload
   include RecordUserLastActivity
+  include ImportUrlParams
   prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
@@ -333,6 +334,7 @@ class ProjectsController < Projects::ApplicationController
   def project_params(attributes: [])
     params.require(:project)
       .permit(project_params_attributes + attributes)
+      .merge(import_url_params)
   end
   def project_params_attributes

app/controllers/sessions_controller.rb

@@ -18,6 +18,7 @@ class SessionsController < Devise::SessionsController
   prepend_before_action :store_redirect_uri, only: [:new]
   prepend_before_action :ldap_servers, only: [:new, :create]
   prepend_before_action :require_no_authentication_without_flash, only: [:new, :create]
+  prepend_before_action :ensure_password_authentication_enabled!, if: :password_based_login?, only: [:create]
   before_action :auto_sign_in_with_provider, only: [:new]
   before_action :load_recaptcha
@@ -138,6 +139,14 @@ class SessionsController < Devise::SessionsController
   end
   # rubocop: enable CodeReuse/ActiveRecord
+  def ensure_password_authentication_enabled!
+    render_403 unless Gitlab::CurrentSettings.password_authentication_enabled_for_web?
+  end
+
+  def password_based_login?
+    user_params[:login].present? || user_params[:password].present?
+  end
+
   def user_params
     params.require(:user).permit(:login, :password, :remember_me, :otp_attempt, :device_response)
   end

app/helpers/application_settings_helper.rb

@@ -160,6 +160,7 @@ module ApplicationSettingsHelper
       :akismet_api_key,
       :akismet_enabled,
       :allow_local_requests_from_hooks_and_services,
+      :dns_rebinding_protection_enabled,
       :archive_builds_in_human_readable,
       :authorized_keys_enabled,
       :auto_devops_enabled,

app/helpers/notifications_helper.rb

@@ -100,4 +100,8 @@ module NotificationsHelper
       css_class: "icon notifications-icon js-notifications-icon"
     )
   end
+
+  def show_unsubscribe_title?(noteable)
+    can?(current_user, "read_#{noteable.to_ability_name}".to_sym, noteable)
+  end
 end

app/models/application_setting_implementation.rb

@@ -21,6 +21,7 @@ module ApplicationSettingImplementation
         after_sign_up_text: nil,
         akismet_enabled: false,
         allow_local_requests_from_hooks_and_services: false,
+        dns_rebinding_protection_enabled: true,
         authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
         container_registry_token_expire_delay: 5,
         default_artifacts_expire_in: '30 days',

app/models/merge_request.rb

@@ -589,6 +589,8 @@ class MergeRequest < ApplicationRecord
       return
     end
+    [:source_branch, :target_branch].each { |attr| validate_branch_name(attr) }
+
     if opened?
       similar_mrs = target_project
         .merge_requests
@@ -609,6 +611,16 @@ class MergeRequest < ApplicationRecord
     end
   end
+  def validate_branch_name(attr)
+    return unless changes_include?(attr)
+
+    branch = read_attribute(attr)
+
+    return unless branch
+
+    errors.add(attr) unless Gitlab::GitRefValidator.validate_merge_request_branch(branch)
+  end
+
   def validate_target_project
     return true if target_project.merge_requests_enabled?

app/models/project.rb

@@ -407,6 +407,7 @@ class Project < ApplicationRecord
   scope :with_builds_enabled, -> { with_feature_enabled(:builds) }
   scope :with_issues_enabled, -> { with_feature_enabled(:issues) }
   scope :with_issues_available_for_user, ->(current_user) { with_feature_available_for_user(:issues, current_user) }
+  scope :with_merge_requests_available_for_user, ->(current_user) { with_feature_available_for_user(:merge_requests, current_user) }
   scope :with_merge_requests_enabled, -> { with_feature_enabled(:merge_requests) }
   scope :with_remote_mirrors, -> { joins(:remote_mirrors).where(remote_mirrors: { enabled: true }).distinct }
@@ -597,6 +598,17 @@ class Project < ApplicationRecord
     def group_ids
       joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
     end
+
+    # Returns ids of projects with milestones available for given user
+    #
+    # Used on queries to find milestones which user can see
+    # For example: Milestone.where(project_id: ids_with_milestone_available_for(user))
+    def ids_with_milestone_available_for(user)
+      with_issues_enabled = with_issues_available_for_user(user).select(:id)
+      with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id)
+
+      from_union([with_issues_enabled, with_merge_requests_enabled]).select(:id)
+    end
   end
   def all_pipelines

app/views/admin/application_settings/_outbound.html.haml

@@ -8,4 +8,12 @@
         = f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do
           Allow requests to the local network from hooks and services
+    .form-group
+      .form-check
+        = f.check_box :dns_rebinding_protection_enabled, class: 'form-check-input'
+        = f.label :dns_rebinding_protection_enabled, class: 'form-check-label' do
+          = _('Enforce DNS rebinding attack protection')
+        %span.form-text.text-muted
+          = _('Resolves IP addresses once and uses them to submit requests')
+
   = f.submit 'Save changes', class: "btn btn-success"

app/views/sent_notifications/unsubscribe.html.haml

 - noteable = @sent_notification.noteable
 - noteable_type = @sent_notification.noteable_type.titleize.downcase
-- noteable_text = %(#{noteable.title} (#{noteable.to_reference}))
+- noteable_text = show_unsubscribe_title?(noteable) ? %(#{noteable.title} (#{noteable.to_reference})) : %(#{noteable.to_reference})
 - page_title _("Unsubscribe"), noteable_text, noteable_type.pluralize, @sent_notification.project.full_name
 %h3.page-title

app/views/shared/_import_form.html.haml

 - ci_cd_only = local_assigns.fetch(:ci_cd_only, false)
-.form-group.import-url-data
-  = f.label :import_url, class: 'label-bold' do
-    %span
-      = _('Git repository URL')
-  = f.text_field :import_url, autocomplete: 'off', class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git', required: true
+- import_url = Gitlab::UrlSanitizer.new(f.object.import_url)
+.import-url-data
+  .form-group
+    = f.label :import_url, class: 'label-bold' do
+      %span
+        = _('Git repository URL')
+    = f.text_field :import_url, value: import_url.sanitized_url,
+        autocomplete: 'off', class: 'form-control', placeholder: 'https://gitlab.company.com/group/project.git', required: true
+  .row
+    .form-group.col-md-6
+      = f.label :import_url_user, class: 'label-bold' do
+        %span
+          = _('Username (optional)')
+      = f.text_field :import_url_user, value: import_url.user, class: 'form-control', required: false, autocomplete: 'new-password'
+
+    .form-group.col-md-6
+      = f.label :import_url_password, class: 'label-bold' do
+        %span
+          = _('Password (optional)')
+      = f.password_field :import_url_password, class: 'form-control', required: false, autocomplete: 'new-password'
   .info-well.prepend-top-20
     .well-segment
@@ -13,7 +28,7 @@
         %li
           = _('The repository must be accessible over <code>http://</code>, <code>https://</code> or <code>git://</code>.').html_safe
         %li
-          = _('If your HTTP repository is not publicly accessible, add authentication information to the URL: <code>https://username:password@gitlab.company.com/group/project.git</code>.').html_safe
+          = _('If your HTTP repository is not publicly accessible, add your credentials.')
         %li
           = import_will_timeout_message(ci_cd_only)
         %li

changelogs/unreleased/dm-http-hostname-override.yml

+---
+title: Protect Gitlab::HTTP against DNS rebinding attack
+merge_request:
+author:
+type: security

changelogs/unreleased/security-60039.yml

+---
+title: Prevent invalid branch for merge request
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fix-confidential-issue-label-visibility-master.yml

+---
+title: Fix confidential issue label disclosure on milestone view
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fix-project-existence-disclosure-master.yml

+---
+title: Fix url redaction for issue links
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fix_milestones_search_api_leak.yml

+---
+title: 'Resolve: Milestones leaked via search API'
+merge_request:
+author:
+type: security

changelogs/unreleased/security-id-leaked-password-in-import-url-frontend.yml

+---
+title: Add extra fields for handling basic auth on import by url page
+merge_request:
+author:
+type: security