Commit 143d3227 authored by Francisco Javier López's avatar Francisco Javier López Committed by Douwe Maan
Browse files

Add validation to webhook and service URLs to ensure they are not blocked because of SSRF

parent 83731cb05f88
......@@ -101,13 +101,19 @@ export default class IntegrationSettingsForm {
return axios.put(this.testEndPoint, formData)
.then(({ data }) => {
if (data.error) {
flash(`${data.message} ${data.service_response}`, 'alert', document, {
title: 'Save anyway',
clickHandler: (e) => {
e.preventDefault();
this.$form.submit();
},
});
let flashActions;
if (data.test_failed) {
flashActions = {
title: 'Save anyway',
clickHandler: (e) => {
e.preventDefault();
this.$form.submit();
},
};
}
flash(`${data.message} ${data.service_response}`, 'alert', document, flashActions);
} else {
this.$form.submit();
}
......
......@@ -41,13 +41,13 @@ def service_test_response
if outcome[:success]
{}
else
{ error: true, message: 'Test failed.', service_response: outcome[:result].to_s }
{ error: true, message: 'Test failed.', service_response: outcome[:result].to_s, test_failed: true }
end
else
{ error: true, message: 'Validations failed.', service_response: @service.errors.full_messages.join(',') }
{ error: true, message: 'Validations failed.', service_response: @service.errors.full_messages.join(','), test_failed: false }
end
rescue Gitlab::HTTP::BlockedUrlError => e
{ error: true, message: 'Test failed.', service_response: e.message }
{ error: true, message: 'Test failed.', service_response: e.message, test_failed: true }
end
def success_message
......
......@@ -18,7 +18,7 @@ class Badge < ActiveRecord::Base
scope :order_created_at_asc, -> { reorder(created_at: :asc) }
validates :link_url, :image_url, url_placeholder: { protocols: %w(http https), placeholder_regex: PLACEHOLDERS_REGEX }
validates :link_url, :image_url, url: { protocols: %w(http https) }
validates :type, presence: true
def rendered_link_url(project = nil)
......
......@@ -32,7 +32,7 @@ class Environment < ActiveRecord::Base
validates :external_url,
length: { maximum: 255 },
allow_nil: true,
addressable_url: true
url: true
delegate :stop_action, :manual_actions, to: :last_deployment, allow_nil: true
......
class GenericCommitStatus < CommitStatus
before_validation :set_default_values
validates :target_url, addressable_url: true,
validates :target_url, url: true,
length: { maximum: 255 },
allow_nil: true
......
......@@ -11,4 +11,9 @@ class SystemHook < WebHook
default_value_for :push_events, false
default_value_for :repository_update_events, true
default_value_for :merge_requests_events, false
# Allow urls pointing localhost and the local network
def allow_local_requests?
true
end
end
......@@ -3,7 +3,9 @@ class WebHook < ActiveRecord::Base
has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
validates :url, presence: true, url: true
validates :url, presence: true, public_url: { allow_localhost: lambda(&:allow_local_requests?),
allow_local_network: lambda(&:allow_local_requests?) }
validates :token, format: { without: /\n/ }
def execute(data, hook_name)
......@@ -13,4 +15,9 @@ def execute(data, hook_name)
def async_execute(data, hook_name)
WebHookService.new(self, data, hook_name).async_execute
end
# Allow urls pointing localhost and the local network
def allow_local_requests?
false
end
end
......@@ -289,8 +289,9 @@ class Project < ActiveRecord::Base
validates :namespace, presence: true
validates :name, uniqueness: { scope: :namespace_id }
validates :import_url, addressable_url: true, if: :external_import?
validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?]
validates :import_url, url: { protocols: %w(http https ssh git),
allow_localhost: false,
ports: VALID_IMPORT_PORTS }, if: [:external_import?, :import_url_changed?]
validates :star_count, numericality: { greater_than_or_equal_to: 0 }
validate :check_limit, on: :create
validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
......
......@@ -3,7 +3,7 @@ class BambooService < CiService
prop_accessor :bamboo_url, :build_key, :username, :password
validates :bamboo_url, presence: true, url: true, if: :activated?
validates :bamboo_url, presence: true, public_url: true, if: :activated?
validates :build_key, presence: true, if: :activated?
validates :username,
presence: true,
......
class BugzillaService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated?
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
......
......@@ -8,7 +8,7 @@ class BuildkiteService < CiService
prop_accessor :project_url, :token
boolean_accessor :enable_ssl_verification
validates :project_url, presence: true, url: true, if: :activated?
validates :project_url, presence: true, public_url: true, if: :activated?
validates :token, presence: true, if: :activated?
after_save :compose_service_hook, if: :activated?
......
......@@ -8,7 +8,7 @@ class ChatNotificationService < Service
prop_accessor :webhook, :username, :channel
boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch
validates :webhook, presence: true, url: true, if: :activated?
validates :webhook, presence: true, public_url: true, if: :activated?
def initialize_properties
# Custom serialized properties initialization
......
class CustomIssueTrackerService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated?
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
......
......@@ -4,7 +4,7 @@ class DroneCiService < CiService
prop_accessor :drone_url, :token
boolean_accessor :enable_ssl_verification
validates :drone_url, presence: true, url: true, if: :activated?
validates :drone_url, presence: true, public_url: true, if: :activated?
validates :token, presence: true, if: :activated?
after_save :compose_service_hook, if: :activated?
......
class ExternalWikiService < Service
prop_accessor :external_wiki_url
validates :external_wiki_url, presence: true, url: true, if: :activated?
validates :external_wiki_url, presence: true, public_url: true, if: :activated?
def title
'External Wiki'
......
class GitlabIssueTrackerService < IssueTrackerService
include Gitlab::Routing
validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated?
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
......
......@@ -3,8 +3,8 @@ class JiraService < IssueTrackerService
include ApplicationHelper
include ActionView::Helpers::AssetUrlHelper
validates :url, url: true, presence: true, if: :activated?
validates :api_url, url: true, allow_blank: true
validates :url, public_url: true, presence: true, if: :activated?
validates :api_url, public_url: true, allow_blank: true
validates :username, presence: true, if: :activated?
validates :password, presence: true, if: :activated?
......
......@@ -24,7 +24,7 @@ class KubernetesService < DeploymentService
prop_accessor :ca_pem
with_options presence: true, if: :activated? do
validates :api_url, url: true
validates :api_url, public_url: true
validates :token
end
......
......@@ -3,7 +3,7 @@ class MockCiService < CiService
ALLOWED_STATES = %w[failed canceled running pending success success_with_warnings skipped not_found].freeze
prop_accessor :mock_service_url
validates :mock_service_url, presence: true, url: true, if: :activated?
validates :mock_service_url, presence: true, public_url: true, if: :activated?
def title
'MockCI'
......
......@@ -6,7 +6,7 @@ class PrometheusService < MonitoringService
boolean_accessor :manual_configuration
with_options presence: true, if: :manual_configuration? do
validates :api_url, url: true
validates :api_url, public_url: true
end
before_save :synchronize_service_state
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment