Commit de9b380e authored by Alex Hanselka's avatar Alex Hanselka

Merge remote-tracking branch 'dev/master'

* dev/master:
  Update CHANGELOG.md for 11.3.13
  Validate LFS hrefs before downloading them
parents 07e079e8 0b7e870f
...@@ -628,6 +628,13 @@ entry. ...@@ -628,6 +628,13 @@ entry.
- Check frozen string in style builds. (gfyoung) - 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) ## 11.3.12 (2018-12-06)
### Security (1 change) ### Security (1 change)
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
module Projects module Projects
module LfsPointers module LfsPointers
class LfsDownloadService < BaseService class LfsDownloadService < BaseService
VALID_PROTOCOLS = %w[http https].freeze
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def execute(oid, url) def execute(oid, url)
return unless project&.lfs_enabled? && oid.present? && url.present? return unless project&.lfs_enabled? && oid.present? && url.present?
...@@ -11,6 +13,7 @@ module Projects ...@@ -11,6 +13,7 @@ module Projects
return if LfsObject.exists?(oid: oid) return if LfsObject.exists?(oid: oid)
sanitized_uri = Gitlab::UrlSanitizer.new(url) sanitized_uri = Gitlab::UrlSanitizer.new(url)
Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS)
with_tmp_file(oid) do |file| with_tmp_file(oid) do |file|
size = download_and_save_file(file, sanitized_uri) size = download_and_save_file(file, sanitized_uri)
......
---
title: Validate LFS hrefs before downloading them
merge_request:
author:
type: security
...@@ -54,6 +54,18 @@ describe Projects::LfsPointers::LfsDownloadService do ...@@ -54,6 +54,18 @@ describe Projects::LfsPointers::LfsDownloadService do
end end
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 context 'when an lfs object with the same oid already exists' do
before do before do
create(:lfs_object, oid: 'oid') create(:lfs_object, oid: 'oid')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment