Check validity of remote URLs before mirroring

For security reasons, we need to check the validity of remote URLs to
avoid users specifying blocked URLs (such as localhost)

app/services/projects/update_remote_mirror_service.rb

@@ -7,6 +7,10 @@ module Projects
     def execute(remote_mirror, tries)
       return success unless remote_mirror.enabled?
 
+      if Gitlab::UrlBlocker.blocked_url?(CGI.unescape(remote_mirror.url))
+        return error("The remote mirror URL is invalid.")
+      end
+
       update_mirror(remote_mirror)
 
       success

changelogs/unreleased/215879-check-validity-of-repository-mirror-urls.yml

+---
+title: Check validity of project's import_url before mirroring repository
+merge_request:
+author:
+type: security

ee/app/services/projects/update_mirror_service.rb

@@ -6,6 +6,10 @@ module Projects
     UpdateError = Class.new(Error)
 
     def execute
+      if project.import_url && Gitlab::UrlBlocker.blocked_url?(CGI.unescape(project.import_url))
+        return error("The import URL is invalid.")
+      end
+
       unless can?(current_user, :access_git)
         return error('The mirror user is not allowed to perform any git operations.')
       end

ee/spec/services/projects/update_mirror_service_spec.rb

@@ -62,6 +62,32 @@ RSpec.describe Projects::UpdateMirrorService do
       end
     end
 
+    context "when the URL is blocked" do
+      before do
+        allow(Gitlab::UrlBlocker).to receive(:blocked_url?).and_return(true)
+
+        stub_fetch_mirror(project)
+      end
+
+      it "fails and returns error status" do
+        expect(service.execute[:status]).to eq(:error)
+      end
+    end
+
+    context "when the URL is localhost" do
+      let(:localhost_url) { "git://localhost:1234/some-path?some-query=some-val\#@example.com/" }
+
+      before do
+        allow(project).to receive(:import_url).and_return(CGI.escape(localhost_url))
+
+        stub_fetch_mirror(project)
+      end
+
+      it "fails and returns error status" do
+        expect(service.execute[:status]).to eq(:error)
+      end
+    end
+
     context "updating tags" do
       it "creates new tags" do
         stub_fetch_mirror(project)
@@ -107,6 +133,8 @@ RSpec.describe Projects::UpdateMirrorService do
 
       before do
         allow(project).to receive(:import_url).and_return(mirror_path)
+
+        allow(Gitlab::UrlBlocker).to receive(:blocked_url?).and_return(false)
       end
 
       context 'when mirror_overwrites_diverged_branches is true' do

spec/services/projects/update_remote_mirror_service_spec.rb

@@ -56,6 +56,28 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
       expect(remote_mirror.last_error).to include('Badly broken')
     end
 
+    context 'when the URL is blocked' do
+      before do
+        allow(Gitlab::UrlBlocker).to receive(:blocked_url?).and_return(true)
+      end
+
+      it 'fails and returns error status' do
+        expect(execute!).to eq(status: :error, message: 'The remote mirror URL is invalid.')
+      end
+    end
+
+    context 'when the URL is localhost' do
+      let(:localhost_url) { "git://localhost:1234/some-path?some-query=some-val\#@example.com/" }
+
+      before do
+        allow(remote_mirror).to receive(:url).and_return(CGI.escape(localhost_url))
+      end
+
+      it 'fails and returns error status' do
+        expect(execute!).to eq(status: :error, message: 'The remote mirror URL is invalid.')
+      end
+    end
+
     context 'when the update fails because of a `Gitlab::Git::CommandError`' do
       before do
         allow(remote_mirror).to receive(:update_repository)