Improve validation of X-Gitlab-Lfs-Tmp header

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