Merge branch 'security/fix-issue-213040' into 'master'

Fix filename bypass when uploading NuGet packages

Closes #104

See merge request gitlab-org/security/gitlab!401

ee/changelogs/unreleased/security-fix-issue-213040.yml

+---
+title: Fix filename bypass when uploading NuGet packages
+merge_request:
+author:
+type: security

ee/lib/api/nuget_packages.rb

@@ -78,13 +78,13 @@ module API
           detail 'This feature was introduced in GitLab 12.6'
         end
         params do
-          use :workhorse_upload_params
+          requires :package, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
         end
         put do
           authorize_upload!(authorized_user_project)
 
           file_params = params.merge(
-            file: uploaded_package_file(:package),
+            file: params[:package],
             file_name: PACKAGE_FILENAME
           )
 

ee/spec/requests/api/nuget_packages_spec.rb

@@ -130,14 +130,17 @@ describe API::NugetPackages do
     let(:url) { "/projects/#{project.id}/packages/nuget" }
     let(:headers) { {} }
     let(:params) { { package: temp_file(file_name) } }
+    let(:file_key) { :package }
+    let(:send_rewritten_field) { true }
 
     subject do
       workhorse_finalize(
         api(url),
         method: :put,
-        file_key: :package,
+        file_key: file_key,
         params: params,
-        headers: headers
+        headers: headers,
+        send_rewritten_field: send_rewritten_field
       )
     end
 

ee/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb

@@ -171,7 +171,7 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
       end
 
       context 'without a file from workhorse' do
-        let(:params) { { package: nil } }
+        let(:send_rewritten_field) { false }
 
         it_behaves_like 'returning response status', :bad_request
       end
@@ -212,6 +212,19 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
             it_behaves_like 'returning response status', :forbidden
           end
         end
+
+        context 'with crafted package.path param' do
+          let(:crafted_file) { Tempfile.new('nuget.crafted.package.path') }
+          let(:url) { "/projects/#{project.id}/packages/nuget?package.path=#{crafted_file.path}" }
+          let(:params) { { file: temp_file(file_name) } }
+          let(:file_key) { :file }
+
+          it 'does not create a package file' do
+            expect { subject }.to change { ::Packages::PackageFile.count }.by(0)
+          end
+
+          it_behaves_like 'returning response status', :bad_request
+        end
       end
 
       context 'and direct upload disabled' do