diff options
| author | Alex Hanselka <alex@gitlab.com> | 2018-12-13 23:57:41 +0000 |
|---|---|---|
| committer | Alex Hanselka <alex@gitlab.com> | 2018-12-13 23:57:41 +0000 |
| commit | 0b7e870f9e938f3776d91866f491302945d604ba (patch) | |
| tree | 5415d5de22c3ea389eefd6cf4167a20eca2e2b11 | |
| parent | 6925165f8b4404894d8bca2613c22cb943d3f774 (diff) | |
| parent | 3ee0710d1d47bec895568563aeca2d3b53bfa8ce (diff) | |
| download | gitlab-ce-0b7e870f9e938f3776d91866f491302945d604ba.tar.gz | |
Merge branch 'security-2754-fix-lfs-import' into 'master'
[MASTER]: Validate LFS hrefs before downloading them
Closes #2754
See merge request gitlab/gitlabhq!2696
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') |