From 0dfa5db1dd9df8f5c4c5dee5a85ed171be2f2ec9 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 1 Sep 2020 22:56:10 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@13-1-stable-ee --- .../projects/update_remote_mirror_service.rb | 4 +++ ...79-check-validity-of-repository-mirror-urls.yml | 5 ++++ .../projects/update_remote_mirror_service_spec.rb | 34 ++++++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 changelogs/unreleased/215879-check-validity-of-repository-mirror-urls.yml diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index 5f8ef75a8d7..05eca148c97 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 418973fb0a6..b817c4b2ec0 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 @@ 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')) -- cgit v1.2.1