Merge branch 'security-fix-path-traversal-in-nuget-package-repository' into 'master'

Security fix path traversal in nuget package repository

Closes #113

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

ee/app/models/packages/package.rb

@@ -30,7 +30,7 @@ class Packages::Package < ApplicationRecord
   validate :valid_conan_package_recipe, if: :conan?
   validate :valid_npm_package_name, if: :npm?
   validate :package_already_taken, if: :npm?
-  validates :version, format: { with: Gitlab::Regex.semver_regex }, if: :npm?
+  validates :version, format: { with: Gitlab::Regex.semver_regex }, if: -> { npm? || nuget? }
   validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
   validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
 

ee/changelogs/unreleased/security-fix-path-traversal-in-nuget-package-repository.yml

+---
+title: Ensure that NuGet package versions are SemVer compliant
+merge_request:
+author:
+type: security

ee/spec/models/packages/package_spec.rb

@@ -95,17 +95,19 @@ RSpec.describe Packages::Package, type: :model do
     end
 
     describe '#version' do
-      context 'npm package' do
-        subject { create(:npm_package) }
-
-        it { is_expected.to allow_value('1.2.3').for(:version) }
-        it { is_expected.to allow_value('1.2.3-beta').for(:version) }
-        it { is_expected.to allow_value('1.2.3-alpha.3').for(:version) }
-        it { is_expected.not_to allow_value('1').for(:version) }
-        it { is_expected.not_to allow_value('1.2').for(:version) }
-        it { is_expected.not_to allow_value('1./2.3').for(:version) }
-        it { is_expected.not_to allow_value('../../../../../1.2.3').for(:version) }
-        it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
+      RSpec.shared_examples 'validating version to be SemVer compliant for' do |factory_name|
+        context "for #{factory_name}" do
+          subject { create(factory_name) }
+
+          it { is_expected.to allow_value('1.2.3').for(:version) }
+          it { is_expected.to allow_value('1.2.3-beta').for(:version) }
+          it { is_expected.to allow_value('1.2.3-alpha.3').for(:version) }
+          it { is_expected.not_to allow_value('1').for(:version) }
+          it { is_expected.not_to allow_value('1.2').for(:version) }
+          it { is_expected.not_to allow_value('1./2.3').for(:version) }
+          it { is_expected.not_to allow_value('../../../../../1.2.3').for(:version) }
+          it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
+        end
       end
 
       context 'conan package' do
@@ -123,6 +125,9 @@ RSpec.describe Packages::Package, type: :model do
         it { is_expected.not_to allow_value('+1.2.3').for(:version) }
         it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
       end
+
+      it_behaves_like 'validating version to be SemVer compliant for', :npm_package
+      it_behaves_like 'validating version to be SemVer compliant for', :nuget_package
     end
 
     describe '#package_already_taken' do
@@ -218,10 +223,12 @@ RSpec.describe Packages::Package, type: :model do
     end
 
     describe '.has_version' do
-      let!(:package4) { create(:nuget_package, version: nil) }
-
       subject { described_class.has_version }
 
+      before do
+        create(:maven_metadatum).package.update!(version: nil)
+      end
+
       it 'includes only packages with version attribute' do
         is_expected.to match_array([package1, package2, package3])
       end

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

@@ -66,5 +66,23 @@ describe Packages::Nuget::UpdatePackageFromMetadataService do
         expect { subject }.to raise_error(::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError)
       end
     end
+
+    context 'with an invalid package version' do
+      invalid_versions = [
+        '1',
+        '1.2',
+        '1./2.3',
+        '../../../../../1.2.3',
+        '%2e%2e%2f1.2.3'
+      ]
+
+      invalid_versions.each do |invalid_version|
+        it "raises an error for version #{invalid_version}" do
+          allow(service).to receive(:package_version).and_return(invalid_version)
+
+          expect { subject }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Version is invalid')
+        end
+      end
+    end
   end
 end