diff options
author | Jacob Vosmaer <jacob@gitlab.com> | 2016-08-10 17:40:20 +0200 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2016-08-10 17:40:20 +0200 |
commit | 26b98bfff8c5bb7048bcbec46e028e30c46bccc5 (patch) | |
tree | 59efd9360829381150e96d5e7230b61e107777fc | |
parent | f817eecb22517ece0344977d00ecc7ddfff30594 (diff) | |
download | gitlab-ce-26b98bfff8c5bb7048bcbec46e028e30c46bccc5.tar.gz |
Improve validation of X-Gitlab-Lfs-Tmp header
-rw-r--r-- | app/controllers/projects/lfs_storage_controller.rb | 10 | ||||
-rw-r--r-- | spec/requests/lfs_http_spec.rb | 16 |
2 files changed, 17 insertions, 9 deletions
diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index a80fa525631..69066cb40e6 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -58,13 +58,9 @@ class Projects::LfsStorageController < Projects::GitHttpClientController def tmp_filename name = request.headers['X-Gitlab-Lfs-Tmp'] - if name.present? - name.gsub!(/^.*(\\|\/)/, '') - name = name.match(/[0-9a-f]{73}/) - name[0] if name - else - nil - end + return if name.include?('/') + return unless oid.present? && name.start_with?(oid) + name end def store_file(oid, size, tmp_file) diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 26572699bf8..9db282a9f3d 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -556,7 +556,7 @@ describe 'Git LFS API and storage' do context 'and request is sent with a malformed headers' do before do - put_finalize('cat /etc/passwd') + put_finalize('/etc/passwd') end it 'does not recognize it as a valid lfs command' do @@ -588,7 +588,7 @@ describe 'Git LFS API and storage' do context 'and request is sent with a malformed headers' do before do - put_finalize('cat /etc/passwd') + put_finalize('/etc/passwd') end it 'does not recognize it as a valid lfs command' do @@ -636,6 +636,18 @@ describe 'Git LFS API and storage' do it 'lfs object is linked to the project' do expect(lfs_object.projects.pluck(:id)).to include(project.id) end + en + + context 'invalid tempfiles' do + it 'rejects slashes in the tempfile name (path traversal' do + put_finalize('foo/bar') + expect(response).to have_http_status(403) + end + + it 'rejects tempfile names that do not start with the oid' do + put_finalize("foo#{sample_oid}") + expect(response).to have_http_status(403) + end end end |