Prevent nupkg validation failure for malformed nupkg (#9739)

src/NuGetGallery/Controllers/ApiController.cs

@@ -553,12 +553,33 @@ private async Task<ActionResult> CreatePackageInternal()
                 {
                     try
                     {
-                        if (ZipArchiveHelpers.FoundEntryInFuture(packageStream, out ZipArchiveEntry entryInTheFuture))
+                        InvalidZipEntry anyInvalidZipEntry = ZipArchiveHelpers.ValidateArchiveEntries(packageStream, out ZipArchiveEntry invalidZipEntry);
+
+                        switch (anyInvalidZipEntry)
                         {
-                            return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, string.Format(
-                                CultureInfo.CurrentCulture,
-                                Strings.PackageEntryFromTheFuture,
-                                entryInTheFuture.Name));
+                            case InvalidZipEntry.None:
+                                break;
+                            case InvalidZipEntry.InFuture:
+                                return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, string.Format(
+                                    CultureInfo.CurrentCulture,
+                                    Strings.PackageEntryFromTheFuture,
+                                    invalidZipEntry.Name));
+                            case InvalidZipEntry.DoubleForwardSlashesInPath:
+                                return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, string.Format(
+                                    CultureInfo.CurrentCulture,
+                                    Strings.PackageEntryWithDoubleForwardSlash,
+                                    invalidZipEntry.Name));
+                            case InvalidZipEntry.DoubleBackwardSlashesInPath:
+                                return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, string.Format(
+                                    CultureInfo.CurrentCulture,
+                                    Strings.PackageEntryWithDoubleBackSlash,
+                                    invalidZipEntry.Name));
+                            default:
+                                // Generic error message for unknown invalid zip entry
+                                return new HttpStatusCodeWithBodyResult(HttpStatusCode.BadRequest, string.Format(
+                                    CultureInfo.CurrentCulture,
+                                    Strings.InvalidPackageEntry,
+                                    invalidZipEntry.Name));
                         }
                     }
                     catch (Exception ex)

src/NuGetGallery/Controllers/PackagesController.cs

@@ -8,6 +8,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.IO;
+using System.IO.Compression;
 using System.Linq;
 using System.Net;
 using System.Net.Mail;
@@ -508,11 +509,33 @@ private async Task<JsonResult> UploadPackageInternal(PackageArchiveReader packag
             // If the current user doesn't have the rights to upload the package, the package upload will be rejected by submitting the form.
             // Related: https://github.com/NuGet/NuGetGallery/issues/5043
             IEnumerable<User> accountsAllowedOnBehalfOf = new[] { currentUser };
-            var foundEntryInFuture = ZipArchiveHelpers.FoundEntryInFuture(uploadStream, out var entryInTheFuture);
-            if (foundEntryInFuture)
+            InvalidZipEntry anyInvalidZipEntry = ZipArchiveHelpers.ValidateArchiveEntries(uploadStream, out ZipArchiveEntry invalidZipEntry);
+
+            switch (anyInvalidZipEntry)
             {
-                return Json(HttpStatusCode.BadRequest, new[] {
-                    new JsonValidationMessage(string.Format(CultureInfo.CurrentCulture, Strings.PackageEntryFromTheFuture, entryInTheFuture.Name)) });
+                case InvalidZipEntry.None:
+                    break;
+                case InvalidZipEntry.InFuture:
+                    return Json(HttpStatusCode.BadRequest, new[]
+                    {
+                        new JsonValidationMessage(string.Format(CultureInfo.CurrentCulture, Strings.PackageEntryFromTheFuture, invalidZipEntry.Name))
+                    });
+                case InvalidZipEntry.DoubleForwardSlashesInPath:
+                    return Json(HttpStatusCode.BadRequest, new[]
+                    {
+                        new JsonValidationMessage(string.Format(CultureInfo.CurrentCulture, Strings.PackageEntryWithDoubleForwardSlash, invalidZipEntry.Name))
+                    });
+                case InvalidZipEntry.DoubleBackwardSlashesInPath:
+                    return Json(HttpStatusCode.BadRequest, new[]
+                    {
+                        new JsonValidationMessage(string.Format(CultureInfo.CurrentCulture, Strings.PackageEntryWithDoubleBackSlash, invalidZipEntry.Name))
+                    });
+                default:
+                    return Json(HttpStatusCode.BadRequest, new[]
+                    {
+                        // Generic error message for unknown invalid zip entry
+                        new JsonValidationMessage(string.Format(CultureInfo.CurrentCulture, Strings.InvalidPackageEntry, invalidZipEntry.Name))
+                    });
             }
 
             try

src/NuGetGallery/Helpers/InvalidZipEntry.cs

@@ -0,0 +1,13 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+namespace NuGetGallery
+{
+    public enum InvalidZipEntry
+    {
+        None,
+        InFuture,
+        DoubleForwardSlashesInPath,
+        DoubleBackwardSlashesInPath
+    }
+}

src/NuGetGallery/Helpers/ZipArchiveHelpers.cs

@@ -5,38 +5,93 @@
 using System.IO;
 using System.IO.Compression;
 using System.Linq;
+using System.Text;
 
 namespace NuGetGallery
 {
     public static class ZipArchiveHelpers
     {
         /// <summary>
         /// This method checks all the <see cref="ZipArchiveEntry"/> in a given 
-        /// <see cref="Stream"/> if it has the entry in the future. It will return
-        /// the first entry found in the future.
+        /// <see cref="Stream"/> if it has an entry with a future datetime or a double slash in the path, 
+        /// it will return the first entry found in the future or with a double slash in the path.
         /// </summary>
         /// <param name="stream"><see cref="Stream"/> object to verify</param>
         /// <param name="entry"><see cref="ZipArchiveEntry"/> found with future entry.</param>
         /// <returns>True if <see cref="Stream"/> contains an entry in future, false otherwise.</returns>
-        public static bool FoundEntryInFuture(Stream stream, out ZipArchiveEntry entry)
+        public static InvalidZipEntry ValidateArchiveEntries(Stream stream, out ZipArchiveEntry entry)
         {
             entry = null;
 
             using (var archive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: true))
             {
                 var reference = DateTime.UtcNow.AddDays(1); // allow "some" clock skew
 
-                var entryInTheFuture = archive.Entries.FirstOrDefault(
+                ZipArchiveEntry entryInTheFuture = archive.Entries.FirstOrDefault(
                     e => e.LastWriteTime.UtcDateTime > reference);
 
                 if (entryInTheFuture != null)
                 {
                     entry = entryInTheFuture;
-                    return true;
+                    return InvalidZipEntry.InFuture;
+                }
+
+                ZipArchiveEntry entryWithDoubleForwardSlash = archive.Entries.FirstOrDefault(
+                    e => e.FullName.Contains("//"));
+
+                if (entryWithDoubleForwardSlash != null)
+                {
+                    entry = entryWithDoubleForwardSlash;
+                    string entryFullName = NormalizeForwardSlashesInPath(entry.FullName);
+                    bool duplicateExist = archive.Entries.Select(e => NormalizeForwardSlashesInPath(e.FullName))
+                        .Count(f => string.Equals(f, entryFullName, StringComparison.OrdinalIgnoreCase)) > 1;
+
+                    if (duplicateExist)
+                        return InvalidZipEntry.DoubleForwardSlashesInPath;
+                }
+
+                ZipArchiveEntry entryWithDoubleBackSlash = archive.Entries.FirstOrDefault(
+                    e => e.FullName.Contains("\\\\"));
+
+                if (entryWithDoubleBackSlash != null)
+                {
+                    entry = entryWithDoubleBackSlash;
+                    return InvalidZipEntry.DoubleBackwardSlashesInPath;
+                }
+            }
+
+            return InvalidZipEntry.None;
+        }
+
+        internal static string NormalizeForwardSlashesInPath(string path)
+        {
+            StringBuilder sb = new StringBuilder();
+            bool lastWasSlash = false;
+
+            foreach (char c in path)
+            {
+                if (c == '/')
+                {
+                    if (!lastWasSlash)
+                    {
+                        sb.Append(c);
+                        lastWasSlash = true;
+                    }
+                }
+                else
+                {
+                    sb.Append(c);
+                    lastWasSlash = false;
+                }
+
+                // Standard ZIP format specification has a limitation for file path lengths of 260 characters
+                if (sb.Length >= 260)
+                {
+                    break;
                 }
             }
 
-            return false;
+            return sb.ToString();
         }
     }
 }

src/NuGetGallery/NuGetGallery.csproj

@@ -240,6 +240,7 @@
     <Compile Include="Filters\AdminActionAttribute.cs" />
     <Compile Include="Filters\RequiresUserAgentAttribute.cs" />
     <Compile Include="Helpers\AdminHelper.cs" />
+    <Compile Include="Helpers\InvalidZipEntry.cs" />
     <Compile Include="Helpers\SearchResponseHelper.cs" />
     <Compile Include="Helpers\ViewModelExtensions\DeleteAccountListPackageItemViewModelFactory.cs" />
     <Compile Include="Helpers\ViewModelExtensions\DeletePackageViewModelFactory.cs" />

src/NuGetGallery/Services/SymbolPackageUploadService.cs

@@ -6,6 +6,7 @@
 using System.IO;
 using System.IO.Compression;
 using System.Linq;
+using System.Net;
 using System.Threading.Tasks;
 using NuGet.Frameworks;
 using NuGet.Packaging;
@@ -63,12 +64,33 @@ public async Task<SymbolPackageValidationResult> ValidateUploadedSymbolsPackage(
 
             try
             {
-                if (ZipArchiveHelpers.FoundEntryInFuture(symbolPackageStream, out ZipArchiveEntry entryInTheFuture))
+                InvalidZipEntry anyInvalidZipEntry = ZipArchiveHelpers.ValidateArchiveEntries(symbolPackageStream, out ZipArchiveEntry invalidZipEntry);
+
+                switch (anyInvalidZipEntry)
                 {
-                    return SymbolPackageValidationResult.Invalid(string.Format(
-                        CultureInfo.CurrentCulture,
-                        Strings.PackageEntryFromTheFuture,
-                        entryInTheFuture.Name));
+                    case InvalidZipEntry.None:
+                        break;
+                    case InvalidZipEntry.InFuture:
+                        return SymbolPackageValidationResult.Invalid(string.Format(
+                            CultureInfo.CurrentCulture,
+                            Strings.PackageEntryFromTheFuture,
+                            invalidZipEntry.Name));
+                    case InvalidZipEntry.DoubleForwardSlashesInPath:
+                        return SymbolPackageValidationResult.Invalid(string.Format(
+                            CultureInfo.CurrentCulture,
+                            Strings.PackageEntryWithDoubleForwardSlash,
+                            invalidZipEntry.Name));
+                    case InvalidZipEntry.DoubleBackwardSlashesInPath:
+                        return SymbolPackageValidationResult.Invalid(string.Format(
+                            CultureInfo.CurrentCulture,
+                            Strings.PackageEntryWithDoubleBackSlash,
+                            invalidZipEntry.Name));
+                    default:
+                        // Generic error message for unknown invalid zip entry
+                        return SymbolPackageValidationResult.Invalid(string.Format(
+                            CultureInfo.CurrentCulture,
+                            Strings.InvalidPackageEntry,
+                            invalidZipEntry.Name));
                 }
 
                 using (var packageToPush = new PackageArchiveReader(symbolPackageStream, leaveStreamOpen: true))

src/NuGetGallery/Strings.resx

@@ -317,7 +317,7 @@
     <value>The package framework '{0}' is not supported. Frameworks within the portable profile are not allowed to have profiles themselves.</value>
   </data>
   <data name="PackageEntryFromTheFuture" xml:space="preserve">
-    <value>The package is invalid and cannot be uploaded. One or more files, such as '{0}' have a date in the future.</value>
+    <value>The package is invalid and cannot be published. One or more files, such as '{0}' have a date in the future.</value>
   </data>
   <data name="AzureActiveDirectory_SignInMessage" xml:space="preserve">
     <value>Sign in with Microsoft Entra ID</value>
@@ -1260,4 +1260,13 @@ The {1} Team</value>
   <data name="Emails_ApiKeyRevoked_Subject" xml:space="preserve">
     <value>[{0}] API Key '{1}' Revoked</value>
   </data>
+  <data name="PackageEntryWithDoubleBackSlash" xml:space="preserve">
+    <value>The package is invalid and cannot be published. The package entry for '{0}' has a  double back slashes in path.</value>
+  </data>
+  <data name="PackageEntryWithDoubleForwardSlash" xml:space="preserve">
+    <value>The package is invalid and cannot be published. The package entry for '{0}' has a  double forward slashes in path, which is causing a file name collision during extraction.</value>
+  </data>
+  <data name="InvalidPackageEntry" xml:space="preserve">
+    <value>The package is invalid and cannot be uploaded. The package entry for '{0}' is not valid.</value>
+  </data>
 </root>

tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs

@@ -829,6 +829,43 @@ public async Task WillRejectBrokenZipFiles()
                 ResultAssert.IsStatusCode(result, HttpStatusCode.BadRequest);
                 Assert.Equal(Strings.FailedToReadUploadFile, (result as HttpStatusCodeWithBodyResult).StatusDescription);
             }
+
+            [Theory]
+            [InlineData("PackageWithDoubleForwardSlash.1.0.0.nupkg")]
+            [InlineData("PackageWithDoubleBackwardSlash.1.0.0.nupkg")]
+            [InlineData("PackageWithVeryLongZipFileEntry.1.0.0.nupkg")]
+            public async Task WillRejectMalformedZipWithEntryDoubleSlashInPath(string zipPath)
+            {
+                // Arrange
+                var package = new MemoryStream(TestDataResourceUtility.GetResourceBytes(zipPath));
+
+                var user = new User() { EmailAddress = "[email protected]" };
+                var controller = new TestableApiController(GetConfigurationService());
+                controller.SetCurrentUser(user);
+                controller.SetupPackageFromInputStream(package);
+
+                // Act
+                ActionResult result = await controller.CreatePackagePut();
+
+                // Assert
+                ResultAssert.IsStatusCode(result, HttpStatusCode.BadRequest);
+
+                if(zipPath.Contains("Forward"))
+                {
+                    Assert.Equal(String.Format(Strings.PackageEntryWithDoubleForwardSlash, "malformedfile.txt"), (result as HttpStatusCodeWithBodyResult).StatusDescription);
+                }
+                else if (zipPath.Contains("Backward"))
+                {
+                    Assert.Equal(String.Format(Strings.PackageEntryWithDoubleBackSlash, "malformedfile.txt"), (result as HttpStatusCodeWithBodyResult).StatusDescription);
+                }
+                else
+                {
+                    string longFileName = "a".PadRight(270, 'a') + ".txt";
+                    Assert.Equal(String.Format(Strings.PackageEntryWithDoubleForwardSlash, longFileName), (result as HttpStatusCodeWithBodyResult).StatusDescription);
+                    string normalizedZipEntry = ZipArchiveHelpers.NormalizeForwardSlashesInPath(longFileName);
+                    Assert.Equal(260, normalizedZipEntry.Length);
+                }
+            }
 
             [Fact]
             public async Task CreatePackageReturns400IfMinClientVersionIsTooHigh()