Add latest changes from gitlab-org/security/gitlab@13-2-stable-ee

3 files changed, 43 insertions, 0 deletions

diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb
index d6c0d647468..769c65583c0 100644
--- a/app/services/projects/update_remote_mirror_service.rb
+++ b/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(Gitlab::UrlSanitizer.sanitize(remote_mirror.url)))
+        return error("The remote mirror URL is invalid.")
+      end
+
       update_mirror(remote_mirror)
 
       success
diff --git a/changelogs/unreleased/215879-check-validity-of-repository-mirror-urls.yml b/changelogs/unreleased/215879-check-validity-of-repository-mirror-urls.yml
new file mode 100644
index 00000000000..0117d6a3ccf
--- /dev/null
+++ b/changelogs/unreleased/215879-check-validity-of-repository-mirror-urls.yml
@@ -0,0 +1,5 @@
+---
+title: Check validity of project's import_url before mirroring repository
+merge_request:
+author:
+type: security
diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb
index f0a8074f46c..c2d6d1cc6e3 100644
--- a/spec/services/projects/update_remote_mirror_service_spec.rb
+++ b/spec/services/projects/update_remote_mirror_service_spec.rb
@@ -65,6 +65,40 @@ 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 given URLs containing escaped elements" do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:url, :result_status) do
+        "https://user:0a%23@test.example.com/project.git"                               | :success
+        "https://git.example.com:1%2F%2F@source.developers.google.com/project.git"      | :success
+        CGI.escape("git://localhost:1234/some-path?some-query=some-val\#@example.com/") | :error
+        CGI.escape(CGI.escape("https://user:0a%23@test.example.com/project.git"))       | :error
+      end
+
+      with_them do
+        before do
+          allow(remote_mirror).to receive(:url).and_return(url)
+          allow(service).to receive(:update_mirror).with(remote_mirror).and_return(true)
+        end
+
+        it "returns expected status" do
+          result = execute!
+
+          expect(result[:status]).to eq(result_status)
+        end
+      end
+    end
+
     context 'when the update fails because of a `Gitlab::Git::CommandError`' do
       before do
         allow(project.repository).to receive(:fetch_remote).and_raise(Gitlab::Git::CommandError.new('fetch failed'))