Merge branch 'security-2754-fix-lfs-import' into 'master'

[MASTER]: Validate LFS hrefs before downloading them

Closes #2754

See merge request gitlab/gitlabhq!2696

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)

changelogs/unreleased/security-2754-fix-lfs-import.yml

+---
+title: Validate LFS hrefs before downloading them
+merge_request:
+author:
+type: security

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')