Automatic merge of gitlab-org/gitlab-ce master

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)

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