diff options
author | Alex Hanselka <alex@gitlab.com> | 2018-12-14 02:58:13 +0300 |
---|---|---|
committer | Alex Hanselka <alex@gitlab.com> | 2018-12-14 02:58:13 +0300 |
commit | de9b380e73992de3c76d965f3843d269a03f97f8 (patch) | |
tree | 978f7ccbbae566c8f4bb66d064b4ca5ce21c1ddc | |
parent | 07e079e8dd336e76986ad001fce79ab9babb00b0 (diff) | |
parent | 0b7e870f9e938f3776d91866f491302945d604ba (diff) |
Merge remote-tracking branch 'dev/master'
* dev/master:
Update CHANGELOG.md for 11.3.13
Validate LFS hrefs before downloading them
-rw-r--r-- | CHANGELOG.md | 7 | ||||
-rw-r--r-- | app/services/projects/lfs_pointers/lfs_download_service.rb | 3 | ||||
-rw-r--r-- | changelogs/unreleased/security-2754-fix-lfs-import.yml | 5 | ||||
-rw-r--r-- | spec/services/projects/lfs_pointers/lfs_download_service_spec.rb | 12 |
4 files changed, 27 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index d1e324c5518..a51ac887aed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -628,6 +628,13 @@ entry. - Check frozen string in style builds. (gfyoung) +## 11.3.13 (2018-12-13) + +### Security (1 change) + +- Validate LFS hrefs before downloading them. + + ## 11.3.12 (2018-12-06) ### Security (1 change) 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') |