Replaced UrlValidator with PublicUrlValidator for import_url and remote mirror urls

parent 5c5a5992
...@@ -324,10 +324,9 @@ class Project < ActiveRecord::Base ...@@ -324,10 +324,9 @@ class Project < ActiveRecord::Base
validates :namespace, presence: true validates :namespace, presence: true
validates :name, uniqueness: { scope: :namespace_id } validates :name, uniqueness: { scope: :namespace_id }
validates :import_url, url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS }, validates :import_url, public_url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS }, ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
allow_localhost: false, enforce_user: true }, if: [:external_import?, :import_url_changed?]
enforce_user: true }, if: [:external_import?, :import_url_changed?]
validates :star_count, numericality: { greater_than_or_equal_to: 0 } validates :star_count, numericality: { greater_than_or_equal_to: 0 }
validate :check_limit, on: :create validate :check_limit, on: :create
validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? } validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
......
...@@ -17,7 +17,7 @@ class RemoteMirror < ActiveRecord::Base ...@@ -17,7 +17,7 @@ class RemoteMirror < ActiveRecord::Base
belongs_to :project, inverse_of: :remote_mirrors belongs_to :project, inverse_of: :remote_mirrors
validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true } validates :url, presence: true, public_url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }
before_save :set_new_remote_name, if: :mirror_url_changed? before_save :set_new_remote_name, if: :mirror_url_changed?
......
---
title: Fix SSRF with import_url and remote mirror url
merge_request:
author:
type: security
...@@ -314,6 +314,13 @@ describe Project do ...@@ -314,6 +314,13 @@ describe Project do
expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed') expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed')
end end
it 'does not allow import_url pointing to the local network' do
project = build(:project, import_url: 'https://192.168.1.1')
expect(project).to be_invalid
expect(project.errors[:import_url].first).to include('Requests to the local network are not allowed')
end
it "does not allow import_url with invalid ports for new projects" do it "does not allow import_url with invalid ports for new projects" do
project = build(:project, import_url: 'http://github.com:25/t.git') project = build(:project, import_url: 'http://github.com:25/t.git')
......
...@@ -24,6 +24,20 @@ describe RemoteMirror do ...@@ -24,6 +24,20 @@ describe RemoteMirror do
expect(remote_mirror).to be_invalid expect(remote_mirror).to be_invalid
expect(remote_mirror.errors[:url].first).to include('Username needs to start with an alphanumeric character') expect(remote_mirror.errors[:url].first).to include('Username needs to start with an alphanumeric character')
end end
it 'does not allow url pointing to localhost' do
remote_mirror = build(:remote_mirror, url: 'http://127.0.0.2/t.git')
expect(remote_mirror).to be_invalid
expect(remote_mirror.errors[:url].first).to include('Requests to loopback addresses are not allowed')
end
it 'does not allow url pointing to the local network' do
remote_mirror = build(:remote_mirror, url: 'https://192.168.1.1')
expect(remote_mirror).to be_invalid
expect(remote_mirror.errors[:url].first).to include('Requests to the local network are not allowed')
end
end end
end end
......
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