Harden validation of package metadata (#10778)

Author: joelverhagen

src/Catalog/CatalogCommitItem.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.
 
 using System;
@@ -40,6 +40,12 @@ public CatalogCommitItem(
             Types = types;
             TypeUris = typeUris;
 
+            Utils.AssertValidPackageId(packageIdentity.Id);
+            if (packageIdentity.Version is null)
+            {
+                throw new ArgumentNullException("The version in the package identity must not be null.");
+            }
+
             IsPackageDetails = HasTypeUri(Schema.DataTypes.PackageDetails);
             IsPackageDelete = HasTypeUri(Schema.DataTypes.PackageDelete);
         }
@@ -123,4 +129,4 @@ private static IEnumerable<string> GetTypes(JObject commitItem)
             }
         }
     }
-}
+}

src/Catalog/CatalogIndexEntry.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.
 
 using System;
@@ -136,8 +136,15 @@ private void Initialize(
                 throw new ArgumentNullException(nameof(packageIdentity));
             }
 
+            if (packageIdentity.Version is null)
+            {
+                throw new ArgumentNullException("The ID and version of the package identity must not be null.");
+            }
+
+            Utils.AssertValidPackageId(packageIdentity.Id);
+
             Id = packageIdentity.Id;
             Version = packageIdentity.Version;
         }
     }
-}
+}

src/Catalog/Dnx/DnxCatalogCollector.cs

@@ -13,6 +13,7 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Newtonsoft.Json.Linq;
+using NuGet.Packaging;
 using NuGet.Protocol.Catalog;
 using NuGet.Services.Metadata.Catalog.Helpers;
 using NuGet.Services.Metadata.Catalog.Persistence;
@@ -456,35 +457,18 @@ private async Task<string> GetNuspecAsync(
 
         private static string GetNuspec(Stream stream, string id)
         {
-            string name = $"{id}.nuspec";
+            using var archive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: true);
+            using var packageArchiveReader = new PackageArchiveReader(archive);
 
-            using (var archive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: true))
+            var actualId = packageArchiveReader.GetIdentity().Id;
+            if (!StringComparer.OrdinalIgnoreCase.Equals(id, actualId))
             {
-                // first look for a nuspec file named as the package id
-                foreach (ZipArchiveEntry entry in archive.Entries)
-                {
-                    if (entry.FullName.Equals(name, StringComparison.InvariantCultureIgnoreCase))
-                    {
-                        using (TextReader reader = new StreamReader(entry.Open()))
-                        {
-                            return reader.ReadToEnd();
-                        }
-                    }
-                }
-                // failing that, just return the first file that appears to be a nuspec
-                foreach (ZipArchiveEntry entry in archive.Entries)
-                {
-                    if (entry.FullName.EndsWith(".nuspec", StringComparison.InvariantCultureIgnoreCase))
-                    {
-                        using (TextReader reader = new StreamReader(entry.Open()))
-                        {
-                            return reader.ReadToEnd();
-                        }
-                    }
-                }
+                throw new InvalidOperationException($"The package ID in the catalog leaf '{id}' does not match the package ID in the .nupkg '{actualId}'."); 
             }
 
-            return null;
+            using var nuspecStream = packageArchiveReader.GetNuspec();
+            using var nuspecReader = new StreamReader(nuspecStream);
+            return nuspecReader.ReadToEnd();
         }
 
         private static Dictionary<string, string> GetTelemetryProperties(CatalogEntry catalogEntry)

src/Catalog/FlatContainerPackagePathProvider.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.
 
 using System;
@@ -27,6 +27,9 @@ public string GetPackagePath(string id, string version)
                 throw new ArgumentNullException(nameof(version));
             }
 
+            Utils.AssertValidPackageId(id);
+            Utils.AssertValidPackageVersion(version);
+
             var idLowerCase = id.ToLowerInvariant();
             var versionLowerCase = NuGetVersionUtility.NormalizeVersion(version).ToLowerInvariant();
             var packageFileName = PackageUtility.GetPackageFileName(idLowerCase, versionLowerCase);
@@ -51,6 +54,9 @@ public string GetIconPath(string id, string version, bool normalize)
                 throw new ArgumentNullException(nameof(version));
             }
 
+            Utils.AssertValidPackageId(id);
+            Utils.AssertValidPackageVersion(version);
+
             var idLowerCase = id.ToLowerInvariant();
 
             string versionLowerCase;

src/Catalog/Helpers/NuGetVersionUtility.cs

@@ -1,6 +1,7 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.
 
+using System;
 using NuGet.Versioning;
 
 namespace NuGet.Services.Metadata.Catalog.Helpers
@@ -12,7 +13,7 @@ public static string NormalizeVersion(string version)
             NuGetVersion parsedVersion;
             if (!NuGetVersion.TryParse(version, out parsedVersion))
             {
-                return version;
+                throw new InvalidOperationException($"Invalid version found when normalized: '{version}'");
             }
 
             return parsedVersion.ToNormalizedString();

src/Catalog/Helpers/PackageUtility.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.
 
 using System;
@@ -19,7 +19,10 @@ public static string GetPackageFileName(string packageId, string packageVersion)
                 throw new ArgumentException(Strings.ArgumentMustNotBeNullOrEmpty, nameof(packageVersion));
             }
 
+            Utils.AssertValidPackageId(packageId);
+            Utils.AssertValidPackageVersion(packageVersion);
+
             return $"{packageId}.{packageVersion}.nupkg";
         }
     }
-}
+}

src/Catalog/Helpers/Utils.cs

@@ -14,12 +14,15 @@
 using System.Xml;
 using System.Xml.Linq;
 using System.Xml.Xsl;
-using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
-using NuGet.Services.Metadata.Catalog.Helpers;
+using NuGet.Packaging;
+using NuGet.Versioning;
+
 #if NETFRAMEWORK
 using JsonLD.Core;
+using Newtonsoft.Json;
 using NuGet.Services.Metadata.Catalog.JsonLDIntegration;
+using NuGet.Services.Metadata.Catalog.Helpers;
 using VDS.RDF;
 using VDS.RDF.Parsing;
 #endif
@@ -28,16 +31,16 @@ namespace NuGet.Services.Metadata.Catalog
 {
     public static class Utils
     {
+#if NETFRAMEWORK
         private const string XslTransformNuSpec = "xslt.nuspec.xslt";
         private const string XslTransformNormalizeNuSpecNamespace = "xslt.normalizeNuspecNamespace.xslt";
 
         private static readonly Lazy<XslCompiledTransform> XslTransformNuSpecCache = new Lazy<XslCompiledTransform>(() => SafeLoadXslTransform(XslTransformNuSpec));
         private static readonly Lazy<XslCompiledTransform> XslTransformNormalizeNuSpecNamespaceCache = new Lazy<XslCompiledTransform>(() => SafeLoadXslTransform(XslTransformNormalizeNuSpecNamespace));
+#endif
 
         private static readonly char[] TagTrimChars = { ',', ' ', '\t', '|', ';' };
 
-        private static readonly char[] Slashes = { '/', '\\' };
-
         public static string[] SplitTags(string original)
         {
             var fields = original
@@ -49,6 +52,22 @@ public static string[] SplitTags(string original)
             return fields;
         }
 
+        public static void AssertValidPackageId(string packageId)
+        {
+            if (packageId is null || !PackageIdValidator.IsValidPackageId(packageId))
+            {
+                throw new InvalidOperationException($"The package ID {(packageId is null ? "<null>" : $"'{packageId}'")} is not valid.");
+            }
+        }
+
+        public static void AssertValidPackageVersion(string packageVersion)
+        {
+            if (packageVersion is null || !NuGetVersion.TryParse(packageVersion, out _))
+            {
+                throw new InvalidOperationException($"The package version {(packageVersion is null ? "<null>" : $"'{packageVersion}'")} is not valid.");
+            }
+        }
+
         public static Stream GetResourceStream(string resourceName)
         {
             if (string.IsNullOrEmpty(resourceName))
@@ -114,20 +133,10 @@ private static XslCompiledTransform SafeLoadXslTransform(string resourceName)
             return transform;
         }
 
-        public static XDocument GetNuspec(ZipArchive package)
+        public static XDocument GetNuspecXDocument(PackageArchiveReader packageArchiveReader)
         {
-            if (package == null) { return null; }
-
-            foreach (ZipArchiveEntry part in package.Entries)
-            {
-                if (part.FullName.EndsWith(".nuspec") && part.FullName.IndexOfAny(Slashes) == -1)
-                {
-                    XDocument nuspec = XDocument.Load(part.Open());
-                    return nuspec;
-                }
-            }
-
-            return null;
+            using var nuspecStream = packageArchiveReader.GetNuspec();
+            return XDocument.Load(nuspecStream);
         }
 
         public static Uri Expand(JToken context, string term)
@@ -203,14 +212,17 @@ public static NupkgMetadata GetNupkgMetadata(Stream stream, string packageHash)
             stream.Seek(0, SeekOrigin.Begin);
 
             using (var package = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: true))
+            using (var packageArchiveReader = new PackageArchiveReader(package))
             {
-                var nuspec = GetNuspec(package);
-
-                if (nuspec == null)
+                var identity = packageArchiveReader.GetIdentity();
+                AssertValidPackageId(identity.Id);
+                if (identity.Version is null)
                 {
-                    throw new InvalidDataException("Unable to find nuspec");
+                    throw new InvalidOperationException("The version from the package identity must not be null.");
                 }
 
+                var nuspec = GetNuspecXDocument(packageArchiveReader);
+
                 var entries = GetEntries(package);
 
                 return new NupkgMetadata(nuspec, entries, packageSize, packageHash);

src/Catalog/Helpers/XsltHelper.cs

@@ -1,8 +1,9 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.
 
 using System.Xml;
 using System.Xml.XPath;
+using NuGet.Packaging;
 using NuGet.Services.Metadata.Catalog.Helpers;
 using NuGet.Versioning;
 
@@ -40,6 +41,16 @@ public string LowerCase(string original)
             return original.ToLowerInvariant();
         }
 
+        public bool IsValidPackageId(string original)
+        {
+            return PackageIdValidator.IsValidPackageId(original);
+        }
+
+        public bool IsValidVersion(string original)
+        {
+            return NuGetVersion.TryParse(original, out _);
+        }
+
         public string NormalizeVersion(string original)
         {
             return NuGetVersionUtility.NormalizeVersion(original);

src/Catalog/Persistence/AzureStorage.cs

@@ -438,13 +438,7 @@ protected override async Task OnDeleteAsync(Uri resourceUri, DeleteRequestOption
         public override Uri GetUri(string name)
         {
             var baseUri = RemoveQueryString(_directory.Uri);
-
-            if (baseUri.EndsWith("/"))
-            {
-                return new Uri($"{baseUri}{name}", UriKind.Absolute);
-            }
-
-            return new Uri($"{baseUri}/{name}", UriKind.Absolute);
+            return new Uri(baseUri.TrimEnd('/') + '/' + name.TrimStart('/'), UriKind.Absolute);
         }
 
         public override async Task<bool> AreSynchronized(Uri firstResourceUri, Uri secondResourceUri)

src/Catalog/Persistence/Storage.cs

@@ -181,7 +181,7 @@ public virtual Task<bool> UpdateCacheControlAsync(Uri resourceUri, string cacheC
 
         public Uri ResolveUri(string relativeUri)
         {
-            return new Uri(BaseAddress, relativeUri);
+            return Services.Storage.Storage.ResolveUri(BaseAddress, relativeUri);
         }
 
         /// <summary>
@@ -197,39 +197,7 @@ public virtual Task<bool> AreSynchronized(Uri firstResourceUri, Uri secondResour
 
         public string GetName(Uri uri)
         {
-            if (uri is null)
-            { 
-                throw new ArgumentNullException(nameof(uri));
-            }
-
-            if (BaseAddress is null)
-            {
-                throw new InvalidOperationException("BaseAddress must be set.");
-            }
-
-            // The GetLeftPart method performs encoding under the hood, which could be problematic if it contains Unicode characters.
-            // It doesn't perform double encoding; the Uri object knows if it's already encoded and skips encoding it again.
-            // Decoding the base address to remove any encoded characters.
-            string address = Uri.UnescapeDataString(BaseAddress.GetLeftPart(UriPartial.Path)); // Remove potential query or SAS from the URI
-            if (!address.EndsWith("/"))
-            {
-                address += "/";
-            }
-
-            // Do the same with the above to get it decoded.
-            string fullPath = Uri.UnescapeDataString(uri.GetLeftPart(UriPartial.Path)); // Remove potential query or SAS from the URI
-
-            // handle mismatched scheme (http vs https)
-            int schemeLengthDifference = uri.Scheme.Length - BaseAddress.Scheme.Length;
-
-            string name = fullPath.Substring(address.Length + schemeLengthDifference);
-
-            if (name.Contains("#"))
-            {
-                name = name.Substring(0, name.IndexOf("#"));
-            }
-
-            return name;
+            return Services.Storage.Storage.GetName(BaseAddress, uri);
         }
 
         public virtual Uri GetUri(string name)