Merge branch 'security-validate-nuget-pkg-version-before-updating-uploaded-file' into 'master'

Update and validate nuget package before updating uploaded file path

Closes #113

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

ee/app/services/packages/nuget/update_package_from_metadata_service.rb

@@ -15,16 +15,18 @@ module Packages
         raise InvalidMetadataError.new('package name and/or package version not found in metadata') unless valid_metadata?
 
         @package_file.transaction do
-          @package_file.update!(
-            file_name: package_filename,
-            file: @package_file.file
-          )
-
           if existing_package_id
             link_to_existing_package
           else
             update_linked_package
           end
+
+          # Updating file_name updates the path where the file is stored.
+          # We must pass the file again so that CarrierWave can handle the update
+          @package_file.update!(
+            file_name: package_filename,
+            file: @package_file.file
+          )
         end
       end
 
@@ -36,7 +38,12 @@ module Packages
 
       def link_to_existing_package
         package_to_destroy = @package_file.package
-        @package_file.update!(package_id: existing_package_id)
+        # Updating package_id updates the path where the file is stored.
+        # We must pass the file again so that CarrierWave can handle the update
+        @package_file.update!(
+          package_id: existing_package_id,
+          file: @package_file.file
+        )
         package_to_destroy.destroy!
       end
 

ee/changelogs/unreleased/security-validate-nuget-pkg-version-before-updating-uploaded-file.yml

+---
+title: Ensure that NuGet package versions are validated before updating the stored
+  file path
+merge_request:
+author:
+type: security

ee/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb

@@ -69,7 +69,7 @@ describe Packages::Nuget::UpdatePackageFromMetadataService do
 
     context 'with an invalid package version' do
       invalid_versions = [
-        '1',
+        '555',
         '1.2',
         '1./2.3',
         '../../../../../1.2.3',
@@ -81,6 +81,8 @@ describe Packages::Nuget::UpdatePackageFromMetadataService do
           allow(service).to receive(:package_version).and_return(invalid_version)
 
           expect { subject }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Version is invalid')
+          expect(package_file.file_name).not_to include(invalid_version)
+          expect(package_file.file.file.path).not_to include(invalid_version)
         end
       end
     end