diff options
author | John Jarvis <jarv@gitlab.com> | 2018-12-12 16:06:43 +0000 |
---|---|---|
committer | John Jarvis <jarv@gitlab.com> | 2018-12-12 16:06:43 +0000 |
commit | 3027b96b467b7e4ca286d3551a0f191a48b3451f (patch) | |
tree | 94969e730f603faf9bbd5efc51bd61a6668b894d | |
parent | 8f4ec2509d9fd6f824cfd0a3ace90088df0153ed (diff) | |
parent | c9d74cf24a39ad4390beced305ff9247beb5bf3e (diff) | |
download | gitlab-ce-3027b96b467b7e4ca286d3551a0f191a48b3451f.tar.gz |
Merge branch 'security-2754-fix-lfs-import-11-6-stable' into '11-6-stable'
[11.6] Validate LFS hrefs before downloading them
See merge request gitlab/gitlabhq!2701
3 files changed, 20 insertions, 0 deletions
diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index 1c4a8d05be6..f9b9781ad5f 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -4,6 +4,8 @@ module Projects module LfsPointers class LfsDownloadService < BaseService + VALID_PROTOCOLS = %w[http https].freeze + # rubocop: disable CodeReuse/ActiveRecord def execute(oid, url) return unless project&.lfs_enabled? && oid.present? && url.present? @@ -11,6 +13,7 @@ module Projects return if LfsObject.exists?(oid: oid) sanitized_uri = Gitlab::UrlSanitizer.new(url) + Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS) with_tmp_file(oid) do |file| size = download_and_save_file(file, sanitized_uri) diff --git a/changelogs/unreleased/security-2754-fix-lfs-import.yml b/changelogs/unreleased/security-2754-fix-lfs-import.yml new file mode 100644 index 00000000000..e8e74c9c3f6 --- /dev/null +++ b/changelogs/unreleased/security-2754-fix-lfs-import.yml @@ -0,0 +1,5 @@ +--- +title: Validate LFS hrefs before downloading them +merge_request: +author: +type: security diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index 6af5bfc7689..d7d7f1874eb 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -54,6 +54,18 @@ describe Projects::LfsPointers::LfsDownloadService do end end + context 'when a bad URL is used' do + where(download_link: ['/etc/passwd', 'ftp://example.com', 'http://127.0.0.2']) + + with_them do + it 'does not download the file' do + expect(subject).not_to receive(:download_and_save_file) + + expect { subject.execute(oid, download_link) }.not_to change { LfsObject.count } + end + end + end + context 'when an lfs object with the same oid already exists' do before do create(:lfs_object, oid: 'oid') |