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

21 files changed, 34 insertions, 22 deletions

diff --git a/app/models/badge.rb b/app/models/badge.rb
index f7e10c2ebfc..265c5d872d4 100644
--- a/app/models/badge.rb
+++ b/app/models/badge.rb
@@ -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)
diff --git a/app/models/environment.rb b/app/models/environment.rb
index fddb269af4b..8d523dae324 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -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
 
diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb
index 532b8f4ad69..5ac8bde44cd 100644
--- a/app/models/generic_commit_status.rb
+++ b/app/models/generic_commit_status.rb
@@ -1,7 +1,7 @@
 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
 
diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb
index 0528266e5b3..6bef00f26ea 100644
--- a/app/models/hooks/system_hook.rb
+++ b/app/models/hooks/system_hook.rb
@@ -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
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 27729deeac9..e353abdda9c 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -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 @@ class WebHook < ActiveRecord::Base
   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
diff --git a/app/models/project.rb b/app/models/project.rb
index e275ac4dc6f..af9fca62dc3 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -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? }
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index 54e4b3278db..7f4c47a6d14 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -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,
diff --git a/app/models/project_services/bugzilla_service.rb b/app/models/project_services/bugzilla_service.rb
index 046e2809f45..e4e3a80976b 100644
--- a/app/models/project_services/bugzilla_service.rb
+++ b/app/models/project_services/bugzilla_service.rb
@@ -1,5 +1,5 @@
 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
 
diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb
index d2aaff8817a..35884c4560c 100644
--- a/app/models/project_services/buildkite_service.rb
+++ b/app/models/project_services/buildkite_service.rb
@@ -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?
diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb
index 7591ab4f478..ae0debbd3ac 100644
--- a/app/models/project_services/chat_notification_service.rb
+++ b/app/models/project_services/chat_notification_service.rb
@@ -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
diff --git a/app/models/project_services/custom_issue_tracker_service.rb b/app/models/project_services/custom_issue_tracker_service.rb
index b9e3e982b64..456c7f5cee2 100644
--- a/app/models/project_services/custom_issue_tracker_service.rb
+++ b/app/models/project_services/custom_issue_tracker_service.rb
@@ -1,5 +1,5 @@
 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
 
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index a4bf427ac0b..ab4e46da89f 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -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?
diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb
index 1553f169827..a4b1ef09e93 100644
--- a/app/models/project_services/external_wiki_service.rb
+++ b/app/models/project_services/external_wiki_service.rb
@@ -1,7 +1,7 @@
 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'
diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb
index 88c428b4aae..16e32a4139e 100644
--- a/app/models/project_services/gitlab_issue_tracker_service.rb
+++ b/app/models/project_services/gitlab_issue_tracker_service.rb
@@ -1,7 +1,7 @@
 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
 
diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index ed4bbfb6cfc..eb3261c902f 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -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?
 
diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb
index 20fed432e55..ddd4026019b 100644
--- a/app/models/project_services/kubernetes_service.rb
+++ b/app/models/project_services/kubernetes_service.rb
@@ -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
 
diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb
index 2221459c90b..b89dc07a73e 100644
--- a/app/models/project_services/mock_ci_service.rb
+++ b/app/models/project_services/mock_ci_service.rb
@@ -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'
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb
index dcaeb65dc32..df4254e0523 100644
--- a/app/models/project_services/prometheus_service.rb
+++ b/app/models/project_services/prometheus_service.rb
@@ -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
diff --git a/app/models/project_services/redmine_service.rb b/app/models/project_services/redmine_service.rb
index 6acf611eba5..3721093a6d1 100644
--- a/app/models/project_services/redmine_service.rb
+++ b/app/models/project_services/redmine_service.rb
@@ -1,5 +1,5 @@
 class RedmineService < 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
 
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index 145313b8e71..802678147cf 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -3,7 +3,7 @@ class TeamcityService < CiService
 
   prop_accessor :teamcity_url, :build_type, :username, :password
 
-  validates :teamcity_url, presence: true, url: true, if: :activated?
+  validates :teamcity_url, presence: true, public_url: true, if: :activated?
   validates :build_type, presence: true, if: :activated?
   validates :username,
     presence: true,
diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index bbf8fd9c6a7..9722cbb2b7c 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -17,7 +17,6 @@ class RemoteMirror < ActiveRecord::Base
   belongs_to :project, inverse_of: :remote_mirrors
 
   validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true }
-  validates :url, addressable_url: true, if: :url_changed?
 
   before_save :set_new_remote_name, if: :mirror_url_changed?