diff --git a/Directory.Build.props b/Directory.Build.props index 903bad4630e5..663c63a10c45 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -83,10 +83,6 @@ false - - enable - - @@ -94,6 +90,10 @@ false + + enable + + diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Workload/WorkloadElevateCommandDefinition.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Workload/WorkloadElevateCommandDefinition.cs index a606d245a458..43066914b2fd 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Workload/WorkloadElevateCommandDefinition.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Workload/WorkloadElevateCommandDefinition.cs @@ -1,13 +1,27 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.CommandLine; + namespace Microsoft.DotNet.Cli.Commands.Workload.Elevate; internal sealed class WorkloadElevateCommandDefinition : WorkloadCommandDefinitionBase { + /// + /// Optional, hidden argument supplied by the unelevated client at server launch with the value of + /// the client's . Used by the elevated server to accept + /// IPC-supplied paths that originate from the client's temp directory when it differs from the + /// server's (e.g., over-the-shoulder UAC, custom TEMP env vars). + /// + public readonly Option ClientTempOption = new("--client-temp") + { + Hidden = true + }; + public WorkloadElevateCommandDefinition() : base("elevate", CommandDefinitionStrings.WorkloadElevateCommandDescription) { Hidden = true; + Options.Add(ClientTempOption); } } diff --git a/src/Cli/dotnet/Commands/Workload/Elevate/WorkloadElevateCommand.cs b/src/Cli/dotnet/Commands/Workload/Elevate/WorkloadElevateCommand.cs index a722254cb2ca..d64c2a4c6648 100644 --- a/src/Cli/dotnet/Commands/Workload/Elevate/WorkloadElevateCommand.cs +++ b/src/Cli/dotnet/Commands/Workload/Elevate/WorkloadElevateCommand.cs @@ -3,6 +3,7 @@ using System.CommandLine; using Microsoft.DotNet.Cli.Commands.Workload.Install; +using Microsoft.DotNet.Cli.Installer.Windows; using Microsoft.DotNet.Cli.Utils; namespace Microsoft.DotNet.Cli.Commands.Workload.Elevate; @@ -14,6 +15,24 @@ public override int Execute() { if (OperatingSystem.IsWindows()) { + // Capture the unelevated client's temp directory (if supplied) so path validators + // can accept IPC-supplied paths that originate from it. Optional and ignored when null + // or unparseable; in either case validators fall back to the server's own temp. + // Use the inherited _parseResult field rather than the primary constructor parameter to avoid + // CS9107 (capturing the parameter into the type's state on top of the base ctor passthrough). + string? clientTemp = _parseResult.GetValue(Definition.ClientTempOption); + if (!string.IsNullOrWhiteSpace(clientTemp)) + { + try + { + InstallerBase.TrustedClientTempDirectory = Path.GetFullPath(clientTemp); + } + catch + { + // Ignore malformed values. + } + } + NetSdkMsiInstallerServer? server = null; try diff --git a/src/Cli/dotnet/Commands/Workload/Install/MsiInstallerBase.cs b/src/Cli/dotnet/Commands/Workload/Install/MsiInstallerBase.cs index f69bdabbd0cd..3ec133fc0ef9 100644 --- a/src/Cli/dotnet/Commands/Workload/Install/MsiInstallerBase.cs +++ b/src/Cli/dotnet/Commands/Workload/Install/MsiInstallerBase.cs @@ -146,18 +146,19 @@ internal static string GetDotNetHome() /// The path of the log file. protected void ConfigureInstall(string logFile) { + string validatedLogFile = WindowsUtils.ValidateLogFilePath(logFile); // Turn off the MSI UI. _ = WindowsInstaller.SetInternalUI(InstallUILevel.None); // The log file must be created before calling MsiEnableLog and we should avoid having active handles // against it. - FileStream logFileStream = File.Create(logFile); + FileStream logFileStream = File.Create(validatedLogFile); logFileStream.Close(); - uint error = WindowsInstaller.EnableLog(InstallLogMode.DEFAULT | InstallLogMode.VERBOSE, logFile, InstallLogAttributes.NONE); + uint error = WindowsInstaller.EnableLog(InstallLogMode.DEFAULT | InstallLogMode.VERBOSE, validatedLogFile, InstallLogAttributes.NONE); // We can report issues with the log file creation, but shouldn't fail the workload operation. - LogError(error, $"Failed to configure log file: {logFile}"); + LogError(error, $"Failed to configure log file: {validatedLogFile}"); } /// @@ -312,7 +313,7 @@ public Dictionary GetGlobalJsonWorkloadSetVersions(SdkFeatureBan protected uint InstallMsi(string packagePath, string logFile) { // Make sure the package we're going to run is coming from the cache. - if (!packagePath.StartsWith(Cache.PackageCacheRoot)) + if (!WindowsUtils.ValidatePackagePath(packagePath, Cache.PackageCacheRoot)) { return Error.INSTALL_PACKAGE_INVALID; } diff --git a/src/Cli/dotnet/Commands/Workload/Install/NetSdkMsiInstallerServer.cs b/src/Cli/dotnet/Commands/Workload/Install/NetSdkMsiInstallerServer.cs index 350a04a87347..ab825cffce36 100644 --- a/src/Cli/dotnet/Commands/Workload/Install/NetSdkMsiInstallerServer.cs +++ b/src/Cli/dotnet/Commands/Workload/Install/NetSdkMsiInstallerServer.cs @@ -174,18 +174,12 @@ public static NetSdkMsiInstallerServer Create(bool verifySignatures) throw new SecurityException(string.Format(CliCommandStrings.NoTrustWithParentPID, ParentProcess?.Id)); } - // Configure pipe DACLs - SecurityIdentifier authenticatedUserIdentifier = new(WellKnownSidType.AuthenticatedUserSid, null); + // Configure pipe DACLs. GetPipeClientIdentifier resolves the parent process's user SID + // and will throw SecurityException if the token cannot be read, preventing the server + // from starting with an insecure configuration. + SecurityIdentifier clientIdentifier = WindowsUtils.GetPipeClientIdentifier(); SecurityIdentifier currentOwnerIdentifier = WindowsIdentity.GetCurrent().Owner; - PipeSecurity pipeSecurity = new(); - - // The current user has full control and should be running as Administrator. - pipeSecurity.SetOwner(currentOwnerIdentifier); - pipeSecurity.AddAccessRule(new PipeAccessRule(currentOwnerIdentifier, PipeAccessRights.FullControl, AccessControlType.Allow)); - - // Restrict read/write access to authenticated users - pipeSecurity.AddAccessRule(new PipeAccessRule(authenticatedUserIdentifier, - PipeAccessRights.Read | PipeAccessRights.Write | PipeAccessRights.Synchronize, AccessControlType.Allow)); + PipeSecurity pipeSecurity = WindowsUtils.CreatePipeSecurity(currentOwnerIdentifier, clientIdentifier); // Initialize the named pipe for dispatching commands. The name of the pipe is based off the server PID since // the client knows this value and ensures both processes can generate the same name. diff --git a/src/Cli/dotnet/Installer/Windows/InstallClientElevationContext.cs b/src/Cli/dotnet/Installer/Windows/InstallClientElevationContext.cs index 53c7710af59b..86046ea3428c 100644 --- a/src/Cli/dotnet/Installer/Windows/InstallClientElevationContext.cs +++ b/src/Cli/dotnet/Installer/Windows/InstallClientElevationContext.cs @@ -29,10 +29,16 @@ public override void Elevate() { if (!IsElevated && !HasElevated) { + // Pass the unelevated client's temp directory to the elevated server so it can validate + // IPC-supplied paths (e.g., the workload pack manifest extracted by the client) against it. + // Quoted to handle profile paths that contain spaces. Optional on the server side; if + // omitted or unparseable, the server falls back to its own Path.GetTempPath(). + string clientTemp = Path.GetFullPath(Path.GetTempPath()).TrimEnd(Path.DirectorySeparatorChar); + // Use the path of the current host, otherwise we risk resolving against the wrong SDK version. // To trigger UAC, UseShellExecute must be true and Verb must be "runas". ProcessStartInfo startInfo = new($@"""{Environment.ProcessPath}""", - $@"""{Assembly.GetExecutingAssembly().Location}"" workload elevate") + $@"""{Assembly.GetExecutingAssembly().Location}"" workload elevate --client-temp ""{clientTemp}""") { Verb = "runas", UseShellExecute = true, diff --git a/src/Cli/dotnet/Installer/Windows/InstallerBase.cs b/src/Cli/dotnet/Installer/Windows/InstallerBase.cs index 835a9c9382a7..186cdd193ba4 100644 --- a/src/Cli/dotnet/Installer/Windows/InstallerBase.cs +++ b/src/Cli/dotnet/Installer/Windows/InstallerBase.cs @@ -74,6 +74,15 @@ protected InstallElevationContextBase ElevationContext /// public static readonly Process ParentProcess; + /// + /// The fully-qualified path of the unelevated client's temp directory, as supplied at server launch + /// via the --client-temp argument. Used by path validators to accept manifest/log paths that + /// originate from the client when the client and server resolve different values for + /// (e.g., over-the-shoulder UAC, custom TEMP env vars). + /// if not supplied. + /// + public static string TrustedClientTempDirectory { get; set; } + /// /// Gets the processor architecture. /// diff --git a/src/Cli/dotnet/Installer/Windows/MsiPackageCache.cs b/src/Cli/dotnet/Installer/Windows/MsiPackageCache.cs index 3d7bf395fd17..9f42a8879639 100644 --- a/src/Cli/dotnet/Installer/Windows/MsiPackageCache.cs +++ b/src/Cli/dotnet/Installer/Windows/MsiPackageCache.cs @@ -41,11 +41,34 @@ internal class MsiPackageCache(InstallElevationContextBase elevationContext, ISe /// The JSON manifest associated with the workload pack MSI. public void CachePayload(string packageId, string packageVersion, string manifestPath) { - if (!File.Exists(manifestPath)) + // Validate that packageId and packageVersion do not contain path traversal characters + // to prevent an IPC client from constructing paths outside the package cache. + if (!WindowsUtils.ValidatePathComponent(packageId)) + { + throw new ArgumentException($"Invalid package ID: {packageId}"); + } + + if (!WindowsUtils.ValidatePathComponent(packageVersion)) { - throw new FileNotFoundException($"CachePayload: Manifest file not found: {manifestPath}"); + throw new ArgumentException($"Invalid package version: {packageVersion}"); } + // Validate that the manifest path resolves to a location under the elevated server's temp + // directory or the unelevated client's temp directory (when supplied at server launch via + // --client-temp). This prevents an IPC client from coercing the elevated server into reading + // or moving arbitrary files. + string fullManifestPath = Path.GetFullPath(manifestPath); + + if (!WindowsUtils.ValidateManifestPath(fullManifestPath)) + { + throw new ArgumentException($"CachePayload: Manifest path is not under an allowed temp directory: {manifestPath}"); + } + + if (!File.Exists(fullManifestPath)) + { + throw new FileNotFoundException($"CachePayload: Manifest file not found: {fullManifestPath}"); + } + Elevate(); if (IsElevated) @@ -63,14 +86,14 @@ public void CachePayload(string packageId, string packageVersion, string manifes // We cannot assume that the MSI adjacent to the manifest is the one to cache. We'll trust // the manifest to provide the MSI filename. - MsiManifest msiManifest = JsonConvert.DeserializeObject(File.ReadAllText(manifestPath)); + MsiManifest msiManifest = JsonConvert.DeserializeObject(File.ReadAllText(fullManifestPath)); // Only use the filename+extension of the payload property in case the manifest has been altered. - string msiPath = Path.Combine(Path.GetDirectoryName(manifestPath), Path.GetFileName(msiManifest.Payload)); + string msiPath = Path.Combine(Path.GetDirectoryName(fullManifestPath), Path.GetFileName(msiManifest.Payload)); string cachedMsiPath = Path.Combine(packageDirectory, Path.GetFileName(msiPath)); - string cachedManifestPath = Path.Combine(packageDirectory, Path.GetFileName(manifestPath)); + string cachedManifestPath = Path.Combine(packageDirectory, Path.GetFileName(fullManifestPath)); - SecurityUtils.MoveAndSecureFile(manifestPath, cachedManifestPath, Log); + SecurityUtils.MoveAndSecureFile(fullManifestPath, cachedManifestPath, Log); SecurityUtils.MoveAndSecureFile(msiPath, cachedMsiPath, Log); } else if (IsClient) diff --git a/src/Cli/dotnet/Installer/Windows/NativeMethods.cs b/src/Cli/dotnet/Installer/Windows/NativeMethods.cs index abd337c9cfad..c0a9d242d5a6 100644 --- a/src/Cli/dotnet/Installer/Windows/NativeMethods.cs +++ b/src/Cli/dotnet/Installer/Windows/NativeMethods.cs @@ -4,6 +4,7 @@ #nullable disable using System.Runtime.Versioning; +using Microsoft.Win32.SafeHandles; namespace Microsoft.DotNet.Cli.Installer.Windows; @@ -13,4 +14,9 @@ internal class NativeMethods [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] public static extern uint FormatMessage(uint dwFlags, nint lpSource, uint dwMessageId, uint dwLanguageId, StringBuilder lpBuffer, uint nSize, nint Arguments); + + [DllImport("advapi32.dll", SetLastError = true)] + [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] + [return: MarshalAs(UnmanagedType.Bool)] + public static extern bool OpenProcessToken(nint processHandle, uint desiredAccess, out SafeAccessTokenHandle tokenHandle); } diff --git a/src/Cli/dotnet/Installer/Windows/WindowsUtils.cs b/src/Cli/dotnet/Installer/Windows/WindowsUtils.cs index 5a471cc8b1ef..6ecf96bada5a 100644 --- a/src/Cli/dotnet/Installer/Windows/WindowsUtils.cs +++ b/src/Cli/dotnet/Installer/Windows/WindowsUtils.cs @@ -3,12 +3,17 @@ #nullable disable +using System.Diagnostics; +using System.IO.Pipes; using System.Runtime.Versioning; +using System.Security; +using System.Security.AccessControl; using System.Security.Principal; using Microsoft.DotNet.Cli.Telemetry; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Utilities; using Microsoft.Win32; +using Microsoft.Win32.SafeHandles; namespace Microsoft.DotNet.Cli.Installer.Windows; @@ -72,4 +77,214 @@ public static bool RebootRequired() return auKey != null || cbsKey != null || hasPendingFileRenames; } + + /// + /// Returns the of the user associated with the specified process. + /// + /// The process whose user SID to retrieve. + /// The of the process owner. + /// Thrown when the process token cannot be opened. + public static SecurityIdentifier GetProcessUserSid(Process process) + { + if (!NativeMethods.OpenProcessToken(process.Handle, (uint)TokenAccessLevels.Query, out SafeAccessTokenHandle tokenHandle)) + { + throw new SecurityException($"Failed to open process token for PID {process.Id}: {Marshal.GetLastPInvokeErrorMessage()}"); + } + + using (tokenHandle) + using (WindowsIdentity identity = new(tokenHandle.DangerousGetHandle())) + { + return identity.User + ?? throw new SecurityException($"Unable to determine user SID for PID {process.Id}."); + } + } + + /// + /// Returns the that should be granted client access to the IPC pipe. + /// Resolves the parent process's user SID to restrict pipe access to only the invoking user. + /// + /// The SID of the client allowed to connect to the pipe. + /// Thrown when the parent process user SID cannot be determined. + public static SecurityIdentifier GetPipeClientIdentifier() + { + return GetProcessUserSid(InstallerBase.ParentProcess); + } + + /// + /// Creates a instance that grants the owner full control and + /// the specified client identity read/write access. + /// + /// The SID of the pipe owner (typically the current elevated user). + /// The SID of the client allowed to connect to the pipe. + /// A configured instance. + public static PipeSecurity CreatePipeSecurity(SecurityIdentifier ownerSid, SecurityIdentifier clientSid) + { + PipeSecurity pipeSecurity = new(); + + // The current user has full control and should be running as Administrator. + pipeSecurity.SetOwner(ownerSid); + pipeSecurity.AddAccessRule(new PipeAccessRule(ownerSid, PipeAccessRights.FullControl, AccessControlType.Allow)); + + // Restrict read/write access to the allowed client (typically in workloads the unelevated process parent talking to the elevated 'server') + pipeSecurity.AddAccessRule(new PipeAccessRule(clientSid, + PipeAccessRights.Read | PipeAccessRights.Write | PipeAccessRights.Synchronize, AccessControlType.Allow)); + + return pipeSecurity; + } + + /// + /// Validates and returns the log file path to use for MSI operations. + /// Ensures the path is under the server's temp directory, the trusted client temp directory + /// (if supplied at server launch), or the parent user's profile temp directory. + /// If the path is not in an allowed location, it is redirected to the server's temp directory. + /// + /// The requested log file path. + /// The server's temp directory. If null, defaults to . + /// The validated log file path. + public static string ValidateLogFilePath(string logFile, string serverTempPath = null) + { + string fullLogPath = Path.GetFullPath(logFile); + string serverTemp = Path.GetFullPath(serverTempPath ?? Path.GetTempPath()); + + if (IsPathUnder(fullLogPath, serverTemp)) + { + return fullLogPath; + } + + string clientTemp = InstallerBase.TrustedClientTempDirectory; + if (!string.IsNullOrEmpty(clientTemp) && IsPathUnder(fullLogPath, clientTemp)) + { + return fullLogPath; + } + + if (InstallerBase.ParentProcess != null) + { + try + { + SecurityIdentifier parentUserSid = GetProcessUserSid(InstallerBase.ParentProcess); + string profilePath = GetUserProfilePath(parentUserSid); + + if (profilePath != null) + { + string profileTemp = Path.GetFullPath(Path.Combine(profilePath, "AppData", "Local", "Temp")); + if (IsPathUnder(fullLogPath, profileTemp)) + { + return fullLogPath; + } + } + } + catch + { + } + } + + return Path.Combine(serverTemp, Path.GetFileName(fullLogPath)); + } + + /// + /// Validates that an IPC-supplied workload manifest path lives under an allowed root. + /// + public static bool ValidateManifestPath(string manifestPath, string serverTempPath = null) + { + if (string.IsNullOrWhiteSpace(manifestPath)) + { + return false; + } + + string fullManifestPath; + try + { + fullManifestPath = Path.GetFullPath(manifestPath); + } + catch + { + return false; + } + + string serverTemp = Path.GetFullPath(serverTempPath ?? Path.GetTempPath()); + if (IsPathUnder(fullManifestPath, serverTemp)) + { + return true; + } + + string clientTemp = InstallerBase.TrustedClientTempDirectory; + if (!string.IsNullOrEmpty(clientTemp) && IsPathUnder(fullManifestPath, clientTemp)) + { + return true; + } + + return false; + } + + private static bool IsPathUnder(string fullPath, string root) + { + string normalizedPath = fullPath.TrimEnd(Path.DirectorySeparatorChar); + string normalizedRoot = root.TrimEnd(Path.DirectorySeparatorChar); + string rootWithSep = normalizedRoot + Path.DirectorySeparatorChar; + + return normalizedPath.Equals(normalizedRoot, StringComparison.OrdinalIgnoreCase) + || normalizedPath.StartsWith(rootWithSep, StringComparison.OrdinalIgnoreCase); + } + + /// + /// Returns the profile path for the user identified by the specified . + /// Reads the ProfileImagePath value from the registry ProfileList key. + /// + /// The SID of the user whose profile path to retrieve. + /// The profile path, or if the profile is not found. + public static string GetUserProfilePath(SecurityIdentifier sid) + { + // RegistryKey.GetValue expands REG_EXPAND_SZ values by default, but call ExpandEnvironmentVariables + // explicitly to also handle the rare case where the value was stored as REG_SZ with literal %vars%. + using RegistryKey profileListKey = Registry.LocalMachine.OpenSubKey( + $@"SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\{sid.Value}"); + + string profileImagePath = profileListKey?.GetValue("ProfileImagePath") as string; + return profileImagePath != null ? Environment.ExpandEnvironmentVariables(profileImagePath) : null; + } + + /// + /// Validates that the specified package path is under the expected cache root directory. + /// Canonicalizes paths to prevent directory traversal and sibling-prefix attacks. + /// + /// The package path to validate. + /// The expected cache root directory. + /// if the path is under the cache root; otherwise . + public static bool ValidatePackagePath(string packagePath, string cacheRoot) + { + return ValidatePathUnderRoot(packagePath, cacheRoot); + } + + /// + /// Validates that a path component (such as a package ID or version) does not contain + /// directory separator characters or parent-directory traversal sequences. + /// + /// The path component to validate. + /// if the component is safe to use in ; otherwise . + public static bool ValidatePathComponent(string component) + { + if (string.IsNullOrWhiteSpace(component)) + { + return false; + } + + return !component.Contains(Path.DirectorySeparatorChar) + && !component.Contains(Path.AltDirectorySeparatorChar) + && !component.Contains(".."); + } + + /// + /// Validates that the specified path, after canonicalization, is under the expected root directory. + /// Prevents directory traversal and sibling-prefix attacks. + /// + /// The path to validate. + /// The expected root directory. + /// if the canonicalized path is under the root; otherwise . + public static bool ValidatePathUnderRoot(string path, string expectedRoot) + { + string fullPath = Path.GetFullPath(path); + string fullRoot = Path.GetFullPath(expectedRoot); + + return IsPathUnder(fullPath, fullRoot); + } } diff --git a/test/dotnet.Tests/WindowsInstallerTests.cs b/test/dotnet.Tests/WindowsInstallerTests.cs index 40e41cbe6730..7fa70b6a0fc0 100644 --- a/test/dotnet.Tests/WindowsInstallerTests.cs +++ b/test/dotnet.Tests/WindowsInstallerTests.cs @@ -6,6 +6,8 @@ using System.IO.Pipes; using System.Reflection; using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; using Microsoft.DotNet.Cli.Installer.Windows; using Microsoft.DotNet.Cli.Installer.Windows.Security; @@ -170,6 +172,244 @@ private NamedPipeServerStream CreateServerPipe(string name) { return new NamedPipeServerStream(name, PipeDirection.InOut, 1, PipeTransmissionMode.Message); } + + [WindowsOnlyFact] + public void CreatePipeSecurity_ShouldNotGrantAccessToAuthenticatedUsers() + { + SecurityIdentifier ownerSid = WindowsIdentity.GetCurrent().Owner; + SecurityIdentifier clientSid = WindowsUtils.GetPipeClientIdentifier(); + + PipeSecurity pipeSecurity = WindowsUtils.CreatePipeSecurity(ownerSid, clientSid); + + var rules = pipeSecurity.GetAccessRules(true, false, typeof(SecurityIdentifier)); + SecurityIdentifier authenticatedUserSid = new(WellKnownSidType.AuthenticatedUserSid, null); + + Assert.DoesNotContain(rules.Cast(), + r => r.IdentityReference.Equals(authenticatedUserSid) && r.AccessControlType == AccessControlType.Allow); + } + + [WindowsOnlyFact] + public void ValidateLogFilePath_ShouldRejectSystemPaths() + { + string maliciousPath = @"C:\Windows\System32\evil.log"; + + string result = WindowsUtils.ValidateLogFilePath(maliciousPath); + Assert.NotEqual(maliciousPath, result); + } + + [WindowsOnlyFact] + public void ValidateLogFilePath_ShouldAcceptUserProfileTempPath() + { + // Use a fake server temp that differs from the user's profile temp, + // forcing the validation to exercise the profile-based lookup path. + string fakeServerTemp = @"C:\Windows\Temp"; + string userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + string userTempPath = Path.Combine(userProfile, "AppData", "Local", "Temp", "Microsoft.NET.Workload_test.log"); + + string result = WindowsUtils.ValidateLogFilePath(userTempPath, fakeServerTemp); + Assert.Equal(Path.GetFullPath(userTempPath), result); + } + + [WindowsOnlyFact] + public void ValidateLogFilePath_ShouldRejectTraversalAttack() + { + string traversalPath = Path.Combine(Path.GetTempPath(), @"..\..\Windows\System32\evil.log"); + + string result = WindowsUtils.ValidateLogFilePath(traversalPath); + string canonicalized = Path.GetFullPath(traversalPath); + + // The traversal resolves to a system path, so it should be redirected + Assert.NotEqual(canonicalized, result); + Assert.StartsWith(Path.GetFullPath(Path.GetTempPath()), result, StringComparison.OrdinalIgnoreCase); + } + + [WindowsOnlyFact] + public void ValidatePackagePath_ShouldRejectTraversalAttack() + { + string cacheRoot = @"C:\ProgramData\dotnet\workloads"; + string traversalPath = cacheRoot + @"\..\..\..\..\Users\Public\evil.msi"; + + Assert.False(WindowsUtils.ValidatePackagePath(traversalPath, cacheRoot)); + } + + [WindowsOnlyFact] + public void ValidatePackagePath_ShouldRejectSiblingPrefixAttack() + { + string cacheRoot = @"C:\ProgramData\dotnet\workloads"; + string siblingPath = @"C:\ProgramData\dotnet\workloadsEvil\evil.msi"; + + Assert.False(WindowsUtils.ValidatePackagePath(siblingPath, cacheRoot)); + } + + [WindowsOnlyFact] + public void ValidatePackagePath_ShouldAcceptValidCachePath() + { + string cacheRoot = @"C:\ProgramData\dotnet\workloads"; + string validPath = @"C:\ProgramData\dotnet\workloads\pack\1.0\pack.msi"; + + Assert.True(WindowsUtils.ValidatePackagePath(validPath, cacheRoot)); + } + + [WindowsOnlyTheory] + [InlineData(@"..\..\evil")] + [InlineData(@"good\evil")] + [InlineData("good/evil")] + [InlineData("")] + [InlineData(null)] + public void ValidatePathComponent_ShouldRejectInvalidInput(string input) + { + Assert.False(WindowsUtils.ValidatePathComponent(input)); + } + + [WindowsOnlyTheory] + [InlineData("Microsoft.NET.Workload.Mono.ToolChain")] + [InlineData("8.0.100")] + public void ValidatePathComponent_ShouldAcceptValidComponent(string input) + { + Assert.True(WindowsUtils.ValidatePathComponent(input)); + } + + [WindowsOnlyTheory] + [InlineData(@"C:\ProgramData\dotnet\workloads\..\..\..\..\Windows\System32\evil.msi", @"C:\ProgramData\dotnet\workloads", false)] + [InlineData(@"C:\ProgramData\dotnet\workloadsEvil\evil.msi", @"C:\ProgramData\dotnet\workloads", false)] + [InlineData(@"C:\ProgramData\dotnet\workloads\pack\1.0\manifest.json", @"C:\ProgramData\dotnet\workloads", true)] + [InlineData(@"C:\ProgramData\dotnet\workloads", @"C:\ProgramData\dotnet\workloads", true)] + [InlineData(@"C:\ProgramData\dotnet\workloads\", @"C:\ProgramData\dotnet\workloads", true)] + [InlineData(@"C:\ProgramData\dotnet\workloads", @"C:\ProgramData\dotnet\workloads\", true)] + public void ValidatePathUnderRoot_ReturnsExpectedResult(string path, string root, bool expected) + { + Assert.Equal(expected, WindowsUtils.ValidatePathUnderRoot(path, root)); + } + + [WindowsOnlyFact] + public void ValidateManifestPath_ShouldAcceptPathUnderServerTemp() + { + string serverTemp = Path.GetFullPath(Path.GetTempPath()).TrimEnd(Path.DirectorySeparatorChar); + string manifest = Path.Combine(serverTemp, Guid.NewGuid().ToString(), "data", "msi.json"); + + string priorClientTemp = InstallerBase.TrustedClientTempDirectory; + try + { + InstallerBase.TrustedClientTempDirectory = null; + Assert.True(WindowsUtils.ValidateManifestPath(manifest)); + } + finally + { + InstallerBase.TrustedClientTempDirectory = priorClientTemp; + } + } + + [WindowsOnlyFact] + public void ValidateManifestPath_ShouldAcceptPathUnderTrustedClientTemp() + { + string fakeServerTemp = @"C:\fake-server-temp"; + string fakeClientTemp = @"C:\fake-client-temp"; + string manifest = Path.Combine(fakeClientTemp, Guid.NewGuid().ToString(), "data", "msi.json"); + + string priorClientTemp = InstallerBase.TrustedClientTempDirectory; + try + { + InstallerBase.TrustedClientTempDirectory = fakeClientTemp; + Assert.True(WindowsUtils.ValidateManifestPath(manifest, fakeServerTemp)); + } + finally + { + InstallerBase.TrustedClientTempDirectory = priorClientTemp; + } + } + + [WindowsOnlyFact] + public void ValidateManifestPath_ShouldRejectPathOutsideAllowedRoots() + { + string fakeServerTemp = @"C:\fake-server-temp"; + string maliciousPath = @"C:\Users\OtherUser\Desktop\evil.json"; + + string priorClientTemp = InstallerBase.TrustedClientTempDirectory; + try + { + InstallerBase.TrustedClientTempDirectory = null; + Assert.False(WindowsUtils.ValidateManifestPath(maliciousPath, fakeServerTemp)); + } + finally + { + InstallerBase.TrustedClientTempDirectory = priorClientTemp; + } + } + + [WindowsOnlyFact] + public void ValidateManifestPath_ShouldRejectTraversalAttack() + { + string serverTemp = Path.GetFullPath(Path.GetTempPath()).TrimEnd(Path.DirectorySeparatorChar); + string traversal = Path.Combine(serverTemp, "..", "..", "..", "Windows", "System32", "evil.json"); + + string priorClientTemp = InstallerBase.TrustedClientTempDirectory; + try + { + InstallerBase.TrustedClientTempDirectory = null; + Assert.False(WindowsUtils.ValidateManifestPath(traversal)); + } + finally + { + InstallerBase.TrustedClientTempDirectory = priorClientTemp; + } + } + + [WindowsOnlyFact] + public void ValidateManifestPath_ShouldRejectSiblingPrefix() + { + string fakeServerTemp = @"C:\fake-server-temp"; + string sibling = @"C:\fake-server-temp_evil\msi.json"; + + string priorClientTemp = InstallerBase.TrustedClientTempDirectory; + try + { + InstallerBase.TrustedClientTempDirectory = null; + Assert.False(WindowsUtils.ValidateManifestPath(sibling, fakeServerTemp)); + } + finally + { + InstallerBase.TrustedClientTempDirectory = priorClientTemp; + } + } + + [WindowsOnlyFact] + public void ValidateManifestPath_ShouldRejectNullOrEmpty() + { + Assert.False(WindowsUtils.ValidateManifestPath(null)); + Assert.False(WindowsUtils.ValidateManifestPath("")); + Assert.False(WindowsUtils.ValidateManifestPath(" ")); + } + + [WindowsOnlyFact] + public void ValidateLogFilePath_ShouldRejectSiblingPrefixAttack() + { + string serverTemp = @"C:\Temp"; + string maliciousPath = @"C:\TempEvil\evil.log"; + + string result = WindowsUtils.ValidateLogFilePath(maliciousPath, serverTemp); + Assert.NotEqual(Path.GetFullPath(maliciousPath), result); + Assert.StartsWith(Path.GetFullPath(serverTemp), result, StringComparison.OrdinalIgnoreCase); + } + + [WindowsOnlyFact] + public void ValidateLogFilePath_ShouldAcceptTrustedClientTemp() + { + string fakeServerTemp = @"C:\fake-server-temp"; + string fakeClientTemp = @"C:\fake-client-temp"; + string clientLogPath = Path.Combine(fakeClientTemp, "Microsoft.NET.Workload_42.log"); + + string priorClientTemp = InstallerBase.TrustedClientTempDirectory; + try + { + InstallerBase.TrustedClientTempDirectory = fakeClientTemp; + string result = WindowsUtils.ValidateLogFilePath(clientLogPath, fakeServerTemp); + Assert.Equal(Path.GetFullPath(clientLogPath), result); + } + finally + { + InstallerBase.TrustedClientTempDirectory = priorClientTemp; + } + } } [SupportedOSPlatform("windows")]