diff options
author | John Jarvis <jarv@gitlab.com> | 2019-01-01 23:38:18 +0300 |
---|---|---|
committer | John Jarvis <jarv@gitlab.com> | 2019-01-01 23:38:18 +0300 |
commit | 1f7b0572d08f726dc224d51c85f7d91bd29a41fa (patch) | |
tree | 643eca484f7711711b08c477e76d1bc6dc52f8ab /app | |
parent | 082a65670290cd4d6064ecc0fe1a3a947bf39d8e (diff) | |
parent | b7a9c26a5bd9b33de9193ba3fd88e3c5d4a2d996 (diff) |
Merge branch 'security-fix-ssrf-lfs-project-import' into 'master'
[master] SSRF in project imports with LFS
See merge request gitlab/gitlabhq!2720
Diffstat (limited to 'app')
-rw-r--r-- | app/services/projects/lfs_pointers/lfs_download_service.rb | 35 |
1 files changed, 27 insertions, 8 deletions
diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index f9b9781ad5f..b5128443435 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -12,28 +12,43 @@ module Projects return if LfsObject.exists?(oid: oid) - sanitized_uri = Gitlab::UrlSanitizer.new(url) - Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS) + sanitized_uri = sanitize_url!(url) with_tmp_file(oid) do |file| - size = download_and_save_file(file, sanitized_uri) - lfs_object = LfsObject.new(oid: oid, size: size, file: file) + download_and_save_file(file, sanitized_uri) + lfs_object = LfsObject.new(oid: oid, size: file.size, file: file) project.all_lfs_objects << lfs_object end + rescue Gitlab::UrlBlocker::BlockedUrlError => e + Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded: #{e.message}") rescue StandardError => e - Rails.logger.error("LFS file with oid #{oid} could't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}") + Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}") end # rubocop: enable CodeReuse/ActiveRecord private + def sanitize_url!(url) + Gitlab::UrlSanitizer.new(url).tap do |sanitized_uri| + # Just validate that HTTP/HTTPS protocols are used. The + # subsequent Gitlab::HTTP.get call will do network checks + # based on the settings. + Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, + protocols: VALID_PROTOCOLS) + end + end + def download_and_save_file(file, sanitized_uri) - IO.copy_stream(open(sanitized_uri.sanitized_url, headers(sanitized_uri)), file) # rubocop:disable Security/Open + response = Gitlab::HTTP.get(sanitized_uri.sanitized_url, headers(sanitized_uri)) do |fragment| + file.write(fragment) + end + + raise StandardError, "Received error code #{response.code}" unless response.success? end def headers(sanitized_uri) - {}.tap do |headers| + query_options.tap do |headers| credentials = sanitized_uri.credentials if credentials[:user].present? || credentials[:password].present? @@ -43,10 +58,14 @@ module Projects end end + def query_options + { stream_body: true } + end + def with_tmp_file(oid) create_tmp_storage_dir - File.open(File.join(tmp_storage_dir, oid), 'w') { |file| yield file } + File.open(File.join(tmp_storage_dir, oid), 'wb') { |file| yield file } end def create_tmp_storage_dir |