From e8b6bb8d79a2b58ad98381f9d2eeebb5b37ae6b9 Mon Sep 17 00:00:00 2001 From: ProductConstructionServiceProd Date: Wed, 19 Aug 2026 23:22:09 +0000 Subject: [PATCH 1/5] [internal/release/stable] Update dependencies from dnceng/internal/microsoft-clrmd - Add internal feed setup to PRs in internal branches --- eng/Version.Details.props | 8 ++++---- eng/Version.Details.xml | 12 ++++++------ eng/pipelines/build.yml | 2 ++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/eng/Version.Details.props b/eng/Version.Details.props index 73c585b185..e036d06fb6 100644 --- a/eng/Version.Details.props +++ b/eng/Version.Details.props @@ -29,9 +29,9 @@ This file should be imported by eng/Versions.props 11.0.0-preview.6.26305.101 11.0.0-preview.6.26305.101 11.0.0-preview.6.26305.101 - - 4.0.727102 - 4.0.727102 + + 4.0.741701 + 4.0.741701 @@ -59,7 +59,7 @@ This file should be imported by eng/Versions.props $(runtimeosxx64MicrosoftDotNetCdacTransportPackageVersion) $(runtimewinarm64MicrosoftDotNetCdacTransportPackageVersion) $(runtimewinx64MicrosoftDotNetCdacTransportPackageVersion) - + $(MicrosoftDiagnosticsRuntimePackageVersion) $(MicrosoftDiagnosticsRuntimeUtilitiesPackageVersion) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 0adb204a11..8a40122e1d 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -1,13 +1,13 @@ - - https://github.com/microsoft/clrmd - 86762cb7283a30fbb99e87008114be6be4a4f4dd + + https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd + f427643b7085e311f1e0bc503b1692143e13cc46 - - https://github.com/microsoft/clrmd - 86762cb7283a30fbb99e87008114be6be4a4f4dd + + https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd + f427643b7085e311f1e0bc503b1692143e13cc46 https://github.com/dotnet/command-line-api diff --git a/eng/pipelines/build.yml b/eng/pipelines/build.yml index d6b95a98ad..19615b8b2b 100644 --- a/eng/pipelines/build.yml +++ b/eng/pipelines/build.yml @@ -184,6 +184,8 @@ jobs: targetPath: '$(Build.SourcesDirectory)/artifacts' checkDownloadedFiles: true + - template: /eng/common/core-templates/steps/enable-internal-sources.yml@self + - script: $(_buildScript) -ci -binaryLog From 8091c66b7e563ae156df8abfc485bc65e6cdff4c Mon Sep 17 00:00:00 2001 From: ProductConstructionServiceProd Date: Wed, 2 Sep 2026 11:23:13 +0000 Subject: [PATCH 2/5] Merged PR 64767: [internal/release/stable] Update dependencies from dnceng/internal/microsoft-clrmd This pull request updates the following dependencies [marker]: <> (Begin:eb6d769e-f5ac-4126-8aa9-724753c581ee) ## From https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd - **Subscription**: [eb6d769e-f5ac-4126-8aa9-724753c581ee](https://maestro.dot.net/subscriptions?search=eb6d769e-f5ac-4126-8aa9-724753c581ee) - **Build**: [20260831.1](https://dev.azure.com/dnceng/internal/_build/results?buildId=3061998) ([329474](https://maestro.dot.net/channel/551/azdo:dnceng:internal:Microsoft-clrmd/build/329474)) - **Date Produced**: August 31, 2026 2:53:33 PM UTC - **Commit**: [04c417f99d7724630eef6c3f737f7b7356b40a7a](https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd?_a=history&version=GC04c417f99d7724630eef6c3f737f7b7356b40a7a) - **Branch**: [refs/heads/msrc-sos](https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd?version=GBrefs/heads/msrc-sos) [DependencyUpdate]: <> (Begin) - **Dependency Updates**: - From [4.0.741701 to 4.0.743101][1] - Microsoft.Diagnostics.Runtime - Microsoft.Diagnostics.Runtime.Utilities [1]: https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd/branches?baseVersion=GCf427643b7085e311f1e0bc503b1692143e13cc46&targetVersion=GC04c417f99d7724630eef6c3f737f7b7356b40a7a&_a=files [DependencyUpdate]: <> (End) [marker]: <> (End:eb6d769e-f5ac-4126-8aa9-724753c581ee) --- eng/Version.Details.props | 4 ++-- eng/Version.Details.xml | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/eng/Version.Details.props b/eng/Version.Details.props index e036d06fb6..d0650e0108 100644 --- a/eng/Version.Details.props +++ b/eng/Version.Details.props @@ -30,8 +30,8 @@ This file should be imported by eng/Versions.props 11.0.0-preview.6.26305.101 11.0.0-preview.6.26305.101 - 4.0.741701 - 4.0.741701 + 4.0.743101 + 4.0.743101 diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 8a40122e1d..74cd6a4509 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -1,13 +1,13 @@ - + https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd - f427643b7085e311f1e0bc503b1692143e13cc46 + 04c417f99d7724630eef6c3f737f7b7356b40a7a - + https://dev.azure.com/dnceng/internal/_git/Microsoft-clrmd - f427643b7085e311f1e0bc503b1692143e13cc46 + 04c417f99d7724630eef6c3f737f7b7356b40a7a https://github.com/dotnet/command-line-api From 03a6ea99c7d0d644baa288dacb65fa34dfb45b64 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Wed, 2 Sep 2026 17:25:25 +0000 Subject: [PATCH 3/5] Merged PR 64786: Update SDK and enable private feeds everywhere --- diagnostics.yml | 2 ++ global.json | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/diagnostics.yml b/diagnostics.yml index ded5f372e1..a47615edcc 100644 --- a/diagnostics.yml +++ b/diagnostics.yml @@ -371,6 +371,8 @@ extends: } displayName: 'Setup layout for packaging' + - template: /eng/common/core-templates/steps/enable-internal-sources.yml@self + # Create nuget packages, sign binaries and publish to blob feed - script: $(Build.SourcesDirectory)\eng\ci-prepare-artifacts.cmd /p:DotNetSignType=$(_SignType) /p:TeamName=$(_TeamName) /p:DotNetPublishUsingPipelines=true /p:OfficialBuildId=$(BUILD.BUILDNUMBER) displayName: Package, Sign, and Publish diff --git a/global.json b/global.json index 75792e25d1..d354c10a72 100644 --- a/global.json +++ b/global.json @@ -1,11 +1,11 @@ { "sdk": { - "version": "10.0.108", + "version": "10.0.111", "allowPrerelease": true, "rollForward": "major" }, "tools": { - "dotnet": "10.0.108", + "dotnet": "10.0.111", "runtimes": { "dotnet": [ "$(MicrosoftNETCoreApp80Version)" From c58476bd416927f0962df4afd45a98be3506a521 Mon Sep 17 00:00:00 2001 From: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:36:48 -0700 Subject: [PATCH 4/5] Share diagnostics release tool across repositories (#5988) ## Summary - package DiagnosticsReleaseTool as Microsoft.Diagnostics.ReleaseTool - support public and internal-only release assets through IsAssetForPublicRelease - move repository-specific release constants into tool-list.json - gather non-shipping assets and skip MergedManifest.xml during release preparation ## Validation - built and packed DiagnosticsReleaseTool in Release configuration - installed the generated .NET tool package and exercised its command-line help - completed independent correctness review --- build.proj | 1 + diagnostics.yml | 1 + eng/pipelines/prepare-release.yml | 7 +- eng/release/DiagnosticsReleaseTool/Config.cs | 7 +- .../Core/FileReleaseData.cs | 8 +- .../DiagnosticsReleaseTool/Core/Release.cs | 71 +++++-- .../DiagnosticsReleaseTool/DarcHelpers.cs | 16 +- .../DiagnosticsManifestGenerator.cs | 2 + .../DiagnosticsReleaseCommandLine.cs | 13 +- .../DiagnosticsReleaseRunner.cs | 9 +- .../DiagnosticsReleaseTool.csproj | 18 ++ eng/release/DiagnosticsReleaseTool/README.md | 6 +- eng/release/Scripts/AcquireBuild.ps1 | 35 ++-- eng/release/Scripts/FixupManifestToLocal.ps1 | 86 -------- eng/release/Scripts/GenerateGithubRelease.ps1 | 188 ------------------ eng/release/Scripts/PublishToNuget.ps1 | 87 -------- 16 files changed, 150 insertions(+), 405 deletions(-) delete mode 100644 eng/release/Scripts/FixupManifestToLocal.ps1 delete mode 100644 eng/release/Scripts/GenerateGithubRelease.ps1 delete mode 100644 eng/release/Scripts/PublishToNuget.ps1 diff --git a/build.proj b/build.proj index 642962000e..9fdded3637 100644 --- a/build.proj +++ b/build.proj @@ -2,6 +2,7 @@ + diff --git a/diagnostics.yml b/diagnostics.yml index a47615edcc..93c9ebd039 100644 --- a/diagnostics.yml +++ b/diagnostics.yml @@ -405,6 +405,7 @@ extends: isOfficialBuild: ${{ variables['isOfficialBuild'] }} repoAllowList: https://github.com/dotnet/diagnostics https://dev.azure.com/dnceng/internal/_git/dotnet-diagnostics productAllowList: diagnostics dotnet-diagnostics + skipFileList: MergedManifest.xml ${{ if eq(variables['isOfficialBuild'], 'true') }}: dependsOn: [ publish_using_darc ] ${{ else }}: diff --git a/eng/pipelines/prepare-release.yml b/eng/pipelines/prepare-release.yml index 4cb3c731fa..e51fd34bf4 100644 --- a/eng/pipelines/prepare-release.yml +++ b/eng/pipelines/prepare-release.yml @@ -8,6 +8,9 @@ parameters: - name: productAllowList type: string default: '' +- name: skipFileList + type: string + default: '' - name: dependsOn type: object default: [ publish_using_darc ] @@ -60,7 +63,8 @@ stages: -BarBuildId "$(BARBuildId)" ` -ReleaseVersion "$(Build.Repository.Name)_$(Build.BuildNumber)" ` -DownloadTargetPath "$(System.ArtifactsDirectory)\ReleaseTarget" ` - -AzdoToken $azdoToken + -AzdoToken $azdoToken ` + -IncludeNonShipping workingDirectory: '$(Build.Repository.LocalPath)' - task: AzureCLI@2 displayName: 'Manifest generation and asset publishing' @@ -84,4 +88,5 @@ stages: --account-name "$(dotnet-diagnostics-storage-accountname)" ` --client-id "$env:servicePrincipalId" ` --container-name "$(dotnet-diagnostics-container-name)" ` + --skip-files ${{ parameters.skipFileList }} ` -v True diff --git a/eng/release/DiagnosticsReleaseTool/Config.cs b/eng/release/DiagnosticsReleaseTool/Config.cs index 22e791e190..31cea6cfc1 100644 --- a/eng/release/DiagnosticsReleaseTool/Config.cs +++ b/eng/release/DiagnosticsReleaseTool/Config.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; +using System.Collections.Frozen; using System.IO; namespace DiagnosticsReleaseTool.Impl @@ -17,6 +19,7 @@ internal sealed class Config public string ContainerName { get; } public string[] ReleaseProductAllowList { get; } public string[] ReleaseRepoAllowList { get; } + public FrozenSet SkipFiles { get; } public Config( FileInfo toolManifest, @@ -28,7 +31,8 @@ public Config( string[] releaseRepoAllowList, string accountName, string clientId, - string containerName) + string containerName, + string[] skipFiles) { ToolManifest = toolManifest; ShouldVerifyManifest = verifyToolManifest; @@ -40,6 +44,7 @@ public Config( ContainerName = containerName; ReleaseProductAllowList = releaseProductAllowList; ReleaseRepoAllowList = releaseRepoAllowList; + SkipFiles = (skipFiles ?? []).ToFrozenSet(StringComparer.OrdinalIgnoreCase); } } } diff --git a/eng/release/DiagnosticsReleaseTool/Core/FileReleaseData.cs b/eng/release/DiagnosticsReleaseTool/Core/FileReleaseData.cs index edc5f045fa..254d9c909e 100644 --- a/eng/release/DiagnosticsReleaseTool/Core/FileReleaseData.cs +++ b/eng/release/DiagnosticsReleaseTool/Core/FileReleaseData.cs @@ -5,18 +5,20 @@ namespace ReleaseTool.Core { public class FileReleaseData { - public FileReleaseData(FileMapping fileMap, FileMetadata fileMetadata) - : this(fileMap, fileMetadata, null) { } + public FileReleaseData(FileMapping fileMap, FileMetadata fileMetadata, bool isAssetForPublicRelease = true) + : this(fileMap, fileMetadata, isAssetForPublicRelease, null) { } - private FileReleaseData(FileMapping fileMap, FileMetadata fileMetadata, string publishUri) + private FileReleaseData(FileMapping fileMap, FileMetadata fileMetadata, bool isAssetForPublicRelease, string publishUri) { FileMap = fileMap; FileMetadata = fileMetadata; + IsAssetForPublicRelease = isAssetForPublicRelease; PublishUri = publishUri; } public FileMapping FileMap { get; } public FileMetadata FileMetadata { get; } + public bool IsAssetForPublicRelease { get; } public string PublishUri { get; internal set; } } } diff --git a/eng/release/DiagnosticsReleaseTool/Core/Release.cs b/eng/release/DiagnosticsReleaseTool/Core/Release.cs index 94663fd118..dbabd772e1 100644 --- a/eng/release/DiagnosticsReleaseTool/Core/Release.cs +++ b/eng/release/DiagnosticsReleaseTool/Core/Release.cs @@ -13,24 +13,26 @@ namespace ReleaseTool.Core { public class Release : IDisposable { - // TODO: there might be a need to expose this for multiple product roots. - private readonly DirectoryInfo _productBuildPath; + private readonly DirectoryInfo _publicReleasePath; + private readonly DirectoryInfo _internalOnlyReleasePath; private readonly List _layoutWorkers; private readonly List _verifiers; private readonly IPublisher _publisher; private readonly IManifestGenerator _manifestGenerator; private readonly string _manifestSavePath; + private readonly Func _shouldSkipFile; private readonly List _filesToRelease; private ILogger _logger; - public Release(DirectoryInfo productBuildPath, + public Release(DirectoryInfo publicReleasePath, DirectoryInfo internalOnlyReleasePath, List layoutWorkers, List verifiers, - IPublisher publisher, IManifestGenerator manifestGenerator, string manifestSavePath) + IPublisher publisher, IManifestGenerator manifestGenerator, string manifestSavePath, + Func shouldSkipFile = null) { - if (productBuildPath is null) + if (publicReleasePath is null) { - throw new ArgumentException("Product build path can't be empty or null."); + throw new ArgumentException("Public release path can't be empty or null."); } if (layoutWorkers is null) @@ -48,12 +50,14 @@ public Release(DirectoryInfo productBuildPath, throw new ArgumentException($"{nameof(manifestGenerator)} can't be null."); } - _productBuildPath = productBuildPath; + _publicReleasePath = publicReleasePath; + _internalOnlyReleasePath = internalOnlyReleasePath; _layoutWorkers = layoutWorkers; _verifiers = verifiers; _publisher = publisher; _manifestGenerator = manifestGenerator; _manifestSavePath = manifestSavePath ?? Path.Join(Path.GetTempPath(), Path.GetRandomFileName(), "releaseManifest"); + _shouldSkipFile = shouldSkipFile ?? (_ => false); _filesToRelease = new List(); _logger = null; // TODO: Validate drop to publish exists. @@ -71,15 +75,34 @@ public async Task RunAsync(CancellationToken ct) int unusedFiles; try { - unusedFiles = await LayoutFilesAsync(ct); + HashSet relativePublishPathsUsed = new(); + unusedFiles = await LayoutFilesAsync( + _publicReleasePath, + isAssetForPublicRelease: true, + relativePublishPathsUsed, + ct); - // TODO: Implement switch to ignore files that are not used as option. if (unusedFiles != 0) { - _logger.LogError("{UnusedFiles} files were not handled for release.", unusedFiles); + _logger.LogError("{UnusedFiles} public release files were not handled for release.", unusedFiles); return unusedFiles; } + if (_internalOnlyReleasePath is not null) + { + unusedFiles = await LayoutFilesAsync( + _internalOnlyReleasePath, + isAssetForPublicRelease: false, + relativePublishPathsUsed, + ct); + + if (unusedFiles != 0) + { + _logger.LogError("{UnusedFiles} internal-only release files were not handled for release.", unusedFiles); + return unusedFiles; + } + } + // TODO: Verification unusedFiles = await PublishFiles(ct); @@ -181,21 +204,32 @@ private async Task PublishFiles(CancellationToken ct) return unpublishedFiles; } - private async Task LayoutFilesAsync(CancellationToken ct) + private async Task LayoutFilesAsync( + DirectoryInfo buildPath, + bool isAssetForPublicRelease, + HashSet relativePublishPathsUsed, + CancellationToken ct) { int unhandledFiles = 0; - HashSet relativePublishPathsUsed = new(); using IDisposable scope = _logger.BeginScope("Laying out files"); - _logger.LogInformation("Laying out files from {_productBuildPath}", _productBuildPath.FullName); + _logger.LogInformation("Laying out {ReleaseType} files from {BuildPath}", + isAssetForPublicRelease ? "public release" : "internal-only release", + buildPath.FullName); // TODO: Make this parallel using Task.Run + semaphore to batch process files. Need to make collections concurrent or have single // queue to aggregate results. // TODO: The file enumeration should have the possibility to inject a custom enumerator. Useful in case there's only subsets of files. // For example, shipping only files. - foreach (FileInfo file in _productBuildPath.EnumerateFiles("*", SearchOption.AllDirectories)) + foreach (FileInfo file in buildPath.EnumerateFiles("*", SearchOption.AllDirectories)) { + if (_shouldSkipFile(file)) + { + _logger.LogTrace("Skipping file {File}", file); + continue; + } + bool isProcessed = false; foreach (ILayoutWorker worker in _layoutWorkers) { @@ -236,8 +270,13 @@ private async Task LayoutFilesAsync(CancellationToken ct) return -1; } relativePublishPathsUsed.Add(dstPath); - _logger.LogTrace("{SrcPath} -> {DstPath} [{FileMetadata}]", srcPath, dstPath, fileMetadata); - _filesToRelease.Add(new FileReleaseData(fileMap, fileMetadata)); + _logger.LogTrace( + "{SrcPath} -> {DstPath} [{FileMetadata}] [IsAssetForPublicRelease: {IsAssetForPublicRelease}]", + srcPath, + dstPath, + fileMetadata, + isAssetForPublicRelease); + _filesToRelease.Add(new FileReleaseData(fileMap, fileMetadata, isAssetForPublicRelease)); } } } diff --git a/eng/release/DiagnosticsReleaseTool/DarcHelpers.cs b/eng/release/DiagnosticsReleaseTool/DarcHelpers.cs index 7fd8d54e23..514c8178ca 100644 --- a/eng/release/DiagnosticsReleaseTool/DarcHelpers.cs +++ b/eng/release/DiagnosticsReleaseTool/DarcHelpers.cs @@ -77,7 +77,7 @@ internal ReleaseMetadata GetDropMetadataForSingleRepoVariants(IEnumerable projectNames) + internal (DirectoryInfo ShippingDirectory, DirectoryInfo NonShippingDirectory) GetShippingDirectoryForSingleProjectVariants(IEnumerable projectNames) { using (Stream darcManifest = File.OpenRead(ReleaseFilePath)) using (JsonDocument jsonDoc = JsonDocument.Parse(darcManifest)) @@ -95,7 +95,19 @@ internal DirectoryInfo GetShippingDirectoryForSingleProjectVariants(IEnumerable< $"There's {matchingProducts.Count()} products that could be released in the release manifest. Expected 1"); } - return new DirectoryInfo(matchingProducts.First().GetProperty("fileshare").GetString()); + DirectoryInfo shippingDirectory = new(matchingProducts.First().GetProperty("fileshare").GetString()); + + if (!shippingDirectory.Name.Equals("shipping", StringComparison.OrdinalIgnoreCase) || + shippingDirectory.Parent is null) + { + throw new InvalidOperationException( + $"Shipping directory '{shippingDirectory.FullName}' does not end in the expected 'shipping' directory."); + } + + string nonShippingPath = Path.Combine(shippingDirectory.Parent.FullName, "nonshipping"); + DirectoryInfo nonShippingDirectory = Directory.Exists(nonShippingPath) ? new(nonShippingPath) : null; + + return (shippingDirectory, nonShippingDirectory); } } } diff --git a/eng/release/DiagnosticsReleaseTool/DiagnosticsManifestGenerator.cs b/eng/release/DiagnosticsReleaseTool/DiagnosticsManifestGenerator.cs index b2f1cee667..dfa10c49c5 100644 --- a/eng/release/DiagnosticsReleaseTool/DiagnosticsManifestGenerator.cs +++ b/eng/release/DiagnosticsReleaseTool/DiagnosticsManifestGenerator.cs @@ -81,6 +81,7 @@ private static void WriteBundledTools(Utf8JsonWriter writer, IEnumerable SkipFilesOption = + new("--skip-files") + { + Arity = ArgumentArity.ZeroOrMore, + AllowMultipleArgumentsPerToken = true, + Description = "List of file names to skip during release processing." + }; + private static readonly Option StagingPathOption = InitStagingPath(); private static Option InitStagingPath() diff --git a/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseRunner.cs b/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseRunner.cs index 6f26284b71..ac527d55cd 100644 --- a/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseRunner.cs +++ b/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseRunner.cs @@ -48,19 +48,21 @@ internal static async Task PrepareRelease(Config releaseConfig, bool verbos // TODO: Probably should use BAR ID instead as an identifier for the metadata to gather. ReleaseMetadata releaseMetadata = darcLayoutHelper.GetDropMetadataForSingleRepoVariants(releaseConfig.ReleaseRepoAllowList); - DirectoryInfo basePublishDirectory = darcLayoutHelper.GetShippingDirectoryForSingleProjectVariants(releaseConfig.ReleaseProductAllowList); + (DirectoryInfo shippingDirectory, DirectoryInfo nonShippingDirectory) = darcLayoutHelper.GetShippingDirectoryForSingleProjectVariants(releaseConfig.ReleaseProductAllowList); string publishManifestPath = Path.Combine(releaseConfig.StagingDirectory.FullName, ManifestName); IPublisher releasePublisher = new AzureBlobBublisher(releaseConfig.AccountName, releaseConfig.ClientId, releaseConfig.ContainerName, releaseConfig.ReleaseName, logger); IManifestGenerator manifestGenerator = new DiagnosticsManifestGenerator(releaseMetadata, releaseConfig.ToolManifest, logger); using Release diagnosticsRelease = new( - productBuildPath: basePublishDirectory, + publicReleasePath: shippingDirectory, + internalOnlyReleasePath: nonShippingDirectory, layoutWorkers: layoutWorkerList, verifiers: verifierList, publisher: releasePublisher, manifestGenerator: manifestGenerator, - manifestSavePath: publishManifestPath + manifestSavePath: publishManifestPath, + shouldSkipFile: file => releaseConfig.SkipFiles.Contains(file.Name) ); diagnosticsRelease.UseLogger(logger); @@ -71,6 +73,7 @@ internal static async Task PrepareRelease(Config releaseConfig, bool verbos private static ILogger GetDiagLogger(bool verbose) { IConfigurationRoot loggingConfiguration = new ConfigurationBuilder() + .SetBasePath(System.AppContext.BaseDirectory) .AddJsonFile("logging.json", optional: false, reloadOnChange: false) .Build(); diff --git a/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseTool.csproj b/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseTool.csproj index 4b4ebc75fd..e69d841153 100644 --- a/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseTool.csproj +++ b/eng/release/DiagnosticsReleaseTool/DiagnosticsReleaseTool.csproj @@ -3,12 +3,30 @@ Exe $(NetCoreAppMinTargetFramework) $(NoWarn);CA2007 + true + false + true + diagnostics-release + Microsoft.Diagnostics.ReleaseTool + Prepares .NET diagnostics assets and manifests for release. + Diagnostics;Release + $(Description) + README.md + win-x64;win-x86;osx-x64 + $(OutputPath) PreserveNewest + + diff --git a/eng/release/DiagnosticsReleaseTool/README.md b/eng/release/DiagnosticsReleaseTool/README.md index 7e33d5b488..d2404cab04 100644 --- a/eng/release/DiagnosticsReleaseTool/README.md +++ b/eng/release/DiagnosticsReleaseTool/README.md @@ -18,7 +18,7 @@ This can be shared by anyone who needs to release files to CDN and generate rele ## The Diagnostics Specifics. 1. The diagnostics repo uses only the publishers and layout workers that are included in the core. -2. The repository uses a `darc` generated drop to collect the assets to release. It's important to note only assets marked as shipping will be considered. +2. The repository uses a `darc` generated drop to collect the assets to release. Assets from the shipping and non-shipping portions of the drop are processed, with `IsAssetForPublicRelease` identifying which assets can be published publicly. - Anything that's a NuGet package will get stored in the `NugetAssets` folder of the release. - Anything that's a symbol package will get stored in the `SymbolNugetAssets` folder of the release. - Single file global tools will get stored in the `ToolBundleAssets` directory under the specific tool's RID. @@ -27,8 +27,8 @@ This can be shared by anyone who needs to release files to CDN and generate rele The manifest in the diagnostics repo consists of a JSON file with four sections: - A generic metadata section consisting of key value pairs of useful build data (commit, branch, version, date, etc). - A `PublishInstructions` section - A list containing each file to be published to our CDN. Each file has the path where it got published, its content's SHA512, the relative path to be stored in the CDN, and if needed the AKA.ms link to use. We specify the schema for our the download links in the `tool-list.json`, which we pass in to the manifest generator constructor. - - A `ToolBundleAssets` section - A list containing each single file tool published. Each item containing the name of the tool, its RID, the path relative to publish root, and the URI where it got published. - - A `NugetAssets` section - A list of all the packages to publish, with each having the relative publishing path and the URI where it got published. + - A `ToolBundleAssets` section - A list containing each single file tool published. Each item contains the name of the tool, its RID, the path relative to publish root, the URI where it was published, and whether it is intended for public release. + - A `NugetAssets` section - A list of all the packages to publish, with each having the relative publishing path, the URI where it was published, and whether it is intended for public release. A sample miniature manifest would be: diff --git a/eng/release/Scripts/AcquireBuild.ps1 b/eng/release/Scripts/AcquireBuild.ps1 index 8bc0103297..f4de61e15e 100644 --- a/eng/release/Scripts/AcquireBuild.ps1 +++ b/eng/release/Scripts/AcquireBuild.ps1 @@ -4,6 +4,7 @@ param( [Parameter(Mandatory=$true)][string] $DownloadTargetPath, [Parameter(Mandatory=$true)][string] $AzdoToken, [Parameter(Mandatory=$false)][string] $DarcVersion = $null, + [switch] $IncludeNonShipping, [switch] $help, [Parameter(ValueFromRemainingArguments=$true)][String[]]$properties ) @@ -13,6 +14,7 @@ function Write-Help() { Write-Host " -ReleaseVersion Name to give the diagnostics release." Write-Host " -DownloadTargetPath Path to download the build to." Write-Host " -AzdoToken Azure DevOps token to use for builds queries" + Write-Host " -IncludeNonShipping Include non-shipping assets in the download." Write-Host "" } @@ -30,8 +32,6 @@ if ($null -ne $properties) { } try { - $ci = $true - $darc = $null try { $darc = (Get-Command darc).Source @@ -41,20 +41,27 @@ try { $darc = Get-Darc $DarcVersion } - & $darc gather-drop ` - --id $BarBuildId ` - --release-name $ReleaseVersion ` - --output-dir $DownloadTargetPath ` - --overwrite ` - --use-azure-credential-for-blobs ` - --azdev-pat $AzdoToken ` - --separated ` - --continue-on-error ` - --ci + $darcArgs = @( + "gather-drop", + "--id", $BarBuildId, + "--release-name", $ReleaseVersion, + "--output-dir", $DownloadTargetPath, + "--overwrite", + "--use-azure-credential-for-blobs", + "--azdev-pat", $AzdoToken, + "--separated", + "--continue-on-error", + "--ci" + ) + + if ($IncludeNonShipping) { + $darcArgs += "--non-shipping" + } + + & $darc @darcArgs if ($LastExitCode -ne 0) { - Write-Host "Error: unable to gather the assets from build $BarBuildId to $DownloadTargetPath using darc." - Write-Host $_ + Write-Host "Error: unable to gather the assets from build $BarBuildId to $DownloadTargetPath using darc. Exit code: $LastExitCode" exit 1 } diff --git a/eng/release/Scripts/FixupManifestToLocal.ps1 b/eng/release/Scripts/FixupManifestToLocal.ps1 deleted file mode 100644 index 544ecb102f..0000000000 --- a/eng/release/Scripts/FixupManifestToLocal.ps1 +++ /dev/null @@ -1,86 +0,0 @@ -param( - [Parameter(Mandatory=$true)][string] $ManifestPath, - [Parameter(Mandatory=$true)][string] $StagingPath, - [Parameter(Mandatory=$false)][string] $DelegationSasToken, - [switch] $help, - [Parameter(ValueFromRemainingArguments=$true)][String[]]$properties -) - -function Write-Help() { - Write-Host "Publish packages specified in a manifest. This should not be used for large manifests." - Write-Host "Common settings:" - Write-Host " -ManifestPath Path to a publishing manifest where the NuGet packages to publish can be found." - Write-Host " -StagingPath Directory containing the staged assets from blob storage." - Write-Host "" -} - -$ErrorActionPreference = 'Stop' -Set-StrictMode -Version 2.0 - -if ($help -or (($null -ne $properties) -and ($properties.Contains('/help') -or $properties.Contains('/?')))) { - Write-Help - exit 1 -} - -if ($null -ne $properties) { - Write-Error "Unexpected extra parameters: $properties." - exit 1 -} - -if (!(Test-Path $ManifestPath)) -{ - Write-Error "Error: unable to find manifest at '$ManifestPath'." - exit 1 -} - -$manifestSize = $(Get-ChildItem $ManifestPath).length / 1kb - -# Limit size. For large manifests -if ($manifestSize -gt 500) -{ - Write-Error "Error: Manifest $ManifestPath too large." - exit 1 -} - -$manifestJson = Get-Content -Raw -Path $ManifestPath | ConvertFrom-Json - -foreach ($nugetPack in $manifestJson.NugetAssets) -{ - $packagePath = Join-Path $StagingPath $nugetPack.PublishRelativePath - if (!(Test-Path $packagePath)) - { - Write-Error "Error: unable to find package at '$packagePath'." - continue - } - Add-Member -InputObject $nugetPack -MemberType NoteProperty -Name LocalPath -Value $packagePath -} - -$toolHashToLocalPath = @{} - -foreach ($tool in $manifestJson.ToolBundleAssets) -{ - $toolPath = Join-Path $StagingPath $tool.PublishRelativePath - if (!(Test-Path $toolPath)) - { - Write-Error "Error: unable to find package at '$toolPath'." - continue - } - Add-Member -InputObject $tool -MemberType NoteProperty -Name LocalPath -Value $toolPath - $toolHashToLocalPath.Add($tool.Sha512, $toolPath) -} - -foreach ($asset in $manifestJson.PublishInstructions) -{ - $remotePath = $asset.FilePath - - if ($DelegationSasToken -ne "") - { - $remotePath = "$remotePath?$DelegationSasToken" - } - - Add-Member -InputObject $asset -MemberType NoteProperty -Name RemotePath -Value $remotePath - $asset.FilePath = $toolHashToLocalPath[$asset.Sha512] -} - -Copy-Item $ManifestPath "$ManifestPath.bak" -$manifestJson | ConvertTo-Json | %{ $_.Replace('\u0026' ,'&') } | Set-Content -Path $ManifestPath diff --git a/eng/release/Scripts/GenerateGithubRelease.ps1 b/eng/release/Scripts/GenerateGithubRelease.ps1 deleted file mode 100644 index 8b3a30768d..0000000000 --- a/eng/release/Scripts/GenerateGithubRelease.ps1 +++ /dev/null @@ -1,188 +0,0 @@ -param( - [Parameter(Mandatory=$true)][string] $ManifestPath, - [Parameter(Mandatory=$false)][string] $ReleaseNotes, - [Parameter(Mandatory=$true)][string] $GhOrganization, - [Parameter(Mandatory=$true)][string] $GhRepository, - [Parameter(Mandatory=$false)][string] $GhCliLink = "https://github.com/cli/cli/releases/download/v2.52.0/gh_2.52.0_windows_amd64.zip", - [Parameter(Mandatory=$true)][string] $TagName, - [bool] $DraftRelease = $false, - [switch] $help, - [Parameter(ValueFromRemainingArguments=$true)][String[]]$properties -) -function Write-Help() { - Write-Host "Publish release to GitHub. Expects an environtment variable GITHUB_TOKEN to perform auth." - Write-Host "Common settings:" - Write-Host " -ManifestPath Path to a publishing manifest." - Write-Host " -ReleaseNotes Path to release notes." - Write-Host " -GhOrganization GitHub organization the repository lives in." - Write-Host " -GhRepository GitHub repository in the organization to create the release on." - Write-Host " -GhCliLink GitHub CLI download link." - Write-Host " -TagName Tag to use for the release." - Write-Host " -DraftRelease Stage the release, but don't make it public yet." - Write-Host "" -} -function Get-ReleaseNotes() -{ - if ($ReleaseNotes) - { - if (!(Test-Path $ReleaseNotes)) - { - Write-Error "Error: unable to find notes at $ReleaseNotes." - exit 1 - } - - return Get-Content -Raw -Path $ReleaseNotes - } -} - -function Get-DownloadLinksAndChecksums($manifest) -{ - - $linkTable = "
`n" - $linkTable += "Packages released to NuGet`n`n" - - foreach ($nugetPackage in $manifest.NugetAssets) - { - $packageName = Split-Path $nugetPackage.PublishRelativePath -Leaf - $linkTable += "- ``" + $packageName + "```n" - } - - $linkTable += "
`n`n" - - $filePublishData = @{} - $manifest.PublishInstructions | %{ $filePublishData.Add($_.Sha512, $_) } - - $sortedTools = $manifest.ToolBundleAssets | Sort-Object -Property @{ Expression = "Rid" }, @{ Expression = "ToolName" } - - $linkTable += "
`n" - $linkTable += "Global Tools - Single File Links`n`n" - $linkTable += "*Note*: All Windows assets are signed with a trusted Microsoft Authenticode Certificate. To verify ` - integrity for Linux and macOS assets check the CSV in the assets section of the release for their SHA512 hashes.`n" - $linkTable += "| Tool | Platform | Download Link |`n" - $linkTable += "|:---:|:---:|:---:|`n" - - $checksumCsv = "`"ToolName`",`"Rid`",`"DownloadLink`",`"Sha512`"`n" - - foreach ($toolBundle in $sortedTools) - { - $hash = $toolBundle.Sha512 - $name = $toolBundle.ToolName - $rid = $toolBundle.Rid - - $link = "https://download.visualstudio.microsoft.com/download/pr/" + $filePublishData[$hash].PublishUrlSubPath - $linkTable += "| $name | $rid | [Download]($link) |`n"; - - $checksumCsv += "`"$name`",`"$rid`",`"$link`",`"$hash`"`n" - } - - $linkTable += "
`n" - return $linkTable, $checksumCsv -} - -function Post-GithubRelease($manifest, [string]$releaseBody, [string]$checksumCsvBody) -{ - $extractionPath = New-TemporaryFile | % { Remove-Item $_; New-Item -ItemType Directory -Path $_ } - $zipPath = Join-Path $extractionPath "ghcli.zip" - $ghTool = [IO.Path]::Combine($extractionPath, "bin", "gh.exe") - - Write-Host "Downloading GitHub CLI from $GhCliLink." - try - { - $progressPreference = 'silentlyContinue' - Invoke-WebRequest $GhCliLink -OutFile $zipPath - Expand-Archive -Path $zipPath -DestinationPath $extractionPath - $progressPreference = 'Continue' - } - catch - { - Write-Error "Unable to get GitHub CLI for release" - exit 1 - } - - if (!(Test-Path $ghTool)) - { - Write-Error "Error: unable to find GitHub tool at expected location." - exit 1 - } - - if (!(Test-Path env:GITHUB_TOKEN)) - { - Write-Error "Error: unable to find GitHub PAT. Please set in GITHUB_TOKEN." - exit 1 - } - - $extraParameters = @() - - if ($DraftRelease -eq $true) - { - $extraParameters += '-d' - } - - $releaseNotes = "release_notes.md" - $csvManifest = "checksums.csv" - - Set-Content -Path $releaseNotes -Value $releaseBody - Set-Content -Path $csvManifest -Value $checksumCsvBody - - if (-Not (Test-Path $releaseNotes)) { - Write-Error "Unable to find release notes" - } - - if (-Not (Test-Path $csvManifest)) { - Write-Error "Unable to find release notes" - } - - $releaseNotes = $(Get-ChildItem $releaseNotes).FullName - $csvManifest = $(Get-ChildItem $csvManifest).FullName - & $ghTool release create $TagName ` - "`"$csvManifest#File Links And Checksums CSV`"" ` - --repo "`"$GhOrganization/$GhRepository`"" ` - --title "`"Diagnostics Release - $TagName`"" ` - --notes-file "`"$releaseNotes`"" ` - --target $manifest.Commit ` - ($extraParameters -join ' ') - - $exitCode = $LASTEXITCODE - if ($exitCode -ne 0) { - Write-Error "Something failed in creating the release." - exit 1 - } -} - -$ErrorActionPreference = 'Stop' -Set-StrictMode -Version 2.0 - -if ($help -or (($null -ne $properties) -and ($properties.Contains('/help') -or $properties.Contains('/?')))) { - Write-Help - exit 1 -} - -if ($null -ne $properties) { - Write-Error "Unexpected extra parameters: $properties." - exit 1 -} - -if (!(Test-Path $ManifestPath)) -{ - Write-Error "Error: unable to find maifest at $ManifestPath." - exit 1 -} - -$manifestSize = $(Get-ChildItem $ManifestPath).length / 1kb - -# Limit size. For large manifests -if ($manifestSize -gt 500) -{ - Write-Error "Error: Manifest $ManifestPath too large." - exit 1 -} - -$manifestJson = Get-Content -Raw -Path $ManifestPath | ConvertFrom-Json -$linkCollection, $checksumCsvContent = Get-DownloadLinksAndChecksums $manifestJson - -$releaseNotesText = Get-ReleaseNotes -$releaseNotesText += "`n`n" + $linkCollection - -Post-GithubRelease -manifest $manifestJson ` - -releaseBody $releaseNotesText ` - -checksumCsvBody $checksumCsvContent diff --git a/eng/release/Scripts/PublishToNuget.ps1 b/eng/release/Scripts/PublishToNuget.ps1 deleted file mode 100644 index 16d4848745..0000000000 --- a/eng/release/Scripts/PublishToNuget.ps1 +++ /dev/null @@ -1,87 +0,0 @@ -param( - [Parameter(Mandatory=$true)][string] $ManifestPath, - [Parameter(Mandatory=$true)][string] $FeedEndpoint, - [Parameter(Mandatory=$true)][string] $FeedPat, - [switch] $help, - [Parameter(ValueFromRemainingArguments=$true)][String[]]$properties -) -function Write-Help() { - Write-Host "Publish packages specified in a manifest. This should not be used for large manifests." - Write-Host "Common settings:" - Write-Host " -ManifestPath Path to a publishing manifest where the NuGet packages to publish can be found." - Write-Host " -FeedEndpoint NuGet feed to publish the packages to." - Write-Host " -FeedPat PAT to use in the publish process." - Write-Host "" -} - -$ErrorActionPreference = 'Stop' -Set-StrictMode -Version 2.0 - -if ($help -or (($null -ne $properties) -and ($properties.Contains('/help') -or $properties.Contains('/?')))) { - Write-Help - exit 1 -} - -if ($null -ne $properties) { - Write-Error "Unexpected extra parameters: $properties." - exit 1 -} - -if (!(Test-Path $ManifestPath)) -{ - Write-Error "Error: unable to find manifest at '$ManifestPath'." - exit 1 -} - -$manifestSize = $(Get-ChildItem $ManifestPath).length / 1kb - -# Limit size. For large manifests -if ($manifestSize -gt 500) -{ - Write-Error "Error: Manifest $ManifestPath too large." - exit 1 -} - -$manifestJson = Get-Content -Raw -Path $ManifestPath | ConvertFrom-Json - -$failedToPublish = 0 -foreach ($nugetPack in $manifestJson.NugetAssets) -{ - if (!($nugetPack.PSobject.Properties.Name.Contains("LocalPath"))) - { - Write-Error "Error: unable to find LocalPath for '$nugetPack'. Ensure local manifest translation happened." - exit 1 - - continue - } - - try - { - $packagePath = $nugetPack.LocalPath; - - if ($nugetPack.PSobject.Properties.Name.Contains("Sha512") -and $(Get-FileHash -Algorithm sha512 $packagePath).Hash -ne $nugetPack.Sha512) { - Write-Host "Sha512 verification failed for $($nugetPack.PublishRelativePath)." - $failedToPublish++ - continue - } - - Write-Host "Publishing $packagePath." - & "$PSScriptRoot/../../../dotnet.cmd" nuget push $packagePath --source $FeedEndpoint --api-key $FeedPat - if ($LastExitCode -ne 0) - { - Write-Error "Error: unable to publish $($nugetPack.PublishRelativePath)." - $failedToPublish++ - } - } - catch - { - Write-Error "Error: unable to publish $($nugetPack.PublishRelativePath)." - $failedToPublish++ - } -} - -if ($failedToPublish -ne 0) -{ - Write-Error "Error: $failedToPublish packages unpublished." - exit 1 -} From cef304c50763bf24f99566cb31d55540842e7ae9 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Fri, 4 Sep 2026 19:42:59 +0000 Subject: [PATCH 5/5] Change path verification for loaded assets Prevent SOS from probing or loading dump-controlled DAC, cDAC, DBI, module, and PDB paths unless they resolve to absolute paths on the local machine. The dump can provide paths recorded on the machine where it was created. Those paths must not cause SOS to access UNC shares, device paths, or other non-local locations on the analysis machine - Add shared managed and native helpers for validating host-local absolute paths and extracting filenames with either path separator. - Validate DAC, cDAC, and DBI paths before probing, signature verification, or library loading. - Use only the filename from non-local dump paths when searching symbol stores. - Continue allowing remote symbol stores as sources, but stage downloaded files in a validated local temporary directory before use. - Prevent native PDB loading from following original or reference paths unless the associated module path is local. - Reject non-local `setclrpath` values before path normalization. - Add regression coverage for UNC, device, mixed-separator, relative, runtime-directory, symbol-store, and adjacent-PDB paths --- ...ostics.DebugServices.Implementation.csproj | 4 + .../PathUtilities.cs | 51 +++++++ .../Runtime.cs | 92 +++++++++---- .../SymbolService.cs | 107 +++++++++++---- .../Host/SetClrPathCommand.cs | 15 ++- ...osoft.Diagnostics.ExtensionCommands.csproj | 1 + src/SOS/SOS.Hosting/RuntimeWrapper.cs | 40 +++++- src/SOS/SOS.Hosting/SOS.Hosting.csproj | 1 + src/SOS/Strike/platform/runtimeimpl.cpp | 41 +++++- src/SOS/Strike/strike.cpp | 11 +- src/SOS/Strike/symbols.cpp | 20 ++- src/SOS/Strike/util.cpp | 77 +++++++++++ src/SOS/Strike/util.h | 6 + .../PathUtilitiesTests.cs | 127 ++++++++++++++++++ .../SymbolServiceTests.cs | 56 +++++++- 15 files changed, 580 insertions(+), 69 deletions(-) create mode 100644 src/Microsoft.Diagnostics.DebugServices.Implementation/PathUtilities.cs create mode 100644 src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/PathUtilitiesTests.cs diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/Microsoft.Diagnostics.DebugServices.Implementation.csproj b/src/Microsoft.Diagnostics.DebugServices.Implementation/Microsoft.Diagnostics.DebugServices.Implementation.csproj index 8a4af246cb..ba2e98c904 100644 --- a/src/Microsoft.Diagnostics.DebugServices.Implementation/Microsoft.Diagnostics.DebugServices.Implementation.csproj +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/Microsoft.Diagnostics.DebugServices.Implementation.csproj @@ -30,4 +30,8 @@ + + + + diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/PathUtilities.cs b/src/Microsoft.Diagnostics.DebugServices.Implementation/PathUtilities.cs new file mode 100644 index 0000000000..9ebf551eff --- /dev/null +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/PathUtilities.cs @@ -0,0 +1,51 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +namespace Microsoft.Diagnostics.DebugServices +{ + internal static class PathUtilities + { + private static readonly char[] s_directorySeparators = ['\\', '/']; + + public static string GetFileName(string path) + { + if (string.IsNullOrEmpty(path)) + { + return path; + } + + int index = path.LastIndexOfAny(s_directorySeparators); + return index >= 0 ? path.Substring(index + 1) : path; + } + + public static bool IsRemoteOrDevicePath(string path) + { + return path?.Length >= 2 && IsDirectorySeparator(path[0]) && IsDirectorySeparator(path[1]); + } + + public static bool IsSafeAbsoluteLocalPath(string path) + { + if (string.IsNullOrWhiteSpace(path) || IsRemoteOrDevicePath(path)) + { + return false; + } + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return path.Length >= 3 && + IsAsciiLetter(path[0]) && + path[1] == ':' && + IsDirectorySeparator(path[2]); + } + + return path[0] == '/'; + } + + private static bool IsDirectorySeparator(char character) => character is '\\' or '/'; + + private static bool IsAsciiLetter(char character) => character is >= 'A' and <= 'Z' or >= 'a' and <= 'z'; + } +} diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs b/src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs index 05db19b715..3cc1c6ee22 100644 --- a/src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs @@ -28,6 +28,7 @@ public class Runtime : IRuntime, IDisposable private bool _verifySignature; // This only applies to the regular DAC, not the CDAC private string _cdacFilePath; private string _dbiFilePath; + private string _runtimeModuleDirectory; protected readonly ServiceContainer _serviceContainer; @@ -81,7 +82,18 @@ void IDisposable.Dispose() public IModule RuntimeModule { get; } - public string RuntimeModuleDirectory { get; set; } + public string RuntimeModuleDirectory + { + get => _runtimeModuleDirectory; + set + { + if (value is not null && !PathUtilities.IsSafeAbsoluteLocalPath(value)) + { + throw new ArgumentException("Runtime module directory must be a local absolute path.", nameof(value)); + } + _runtimeModuleDirectory = value; + } + } public Version RuntimeVersion { @@ -190,10 +202,15 @@ private string GetLibraryPath(DebugLibraryKind kind) if (libraryInfo.ArchivedUnder != SymbolProperties.None) { libraryPath = DownloadFile(libraryInfo); - if (libraryPath is not null) + if (PathUtilities.IsSafeAbsoluteLocalPath(libraryPath)) { break; } + if (libraryPath is not null) + { + Trace.TraceError($"Can't load {libraryInfo.Kind} from path '{libraryPath}' because it is not local"); + } + libraryPath = null; } } } @@ -203,33 +220,62 @@ private string GetLibraryPath(DebugLibraryKind kind) private string GetLocalPath(DebugLibraryInfo libraryInfo) { - string localFilePath; - if (libraryInfo.Kind == DebugLibraryKind.CDac) + string localFilePath = GetLocalCandidatePath( + libraryInfo.Kind, + libraryInfo.FileName, + RuntimeModuleDirectory, + RuntimeModule.FileName); + + return PathUtilities.IsSafeAbsoluteLocalPath(localFilePath) && File.Exists(localFilePath) + ? localFilePath + : null; + } + + internal static string GetLocalCandidatePath( + DebugLibraryKind kind, + string libraryFileName, + string runtimeModuleDirectory, + string runtimeModuleFileName) + { + string fileName = PathUtilities.GetFileName(libraryFileName); + if (string.IsNullOrEmpty(fileName)) { - localFilePath = libraryInfo.FileName; + return null; } - else + + if (kind == DebugLibraryKind.CDac) { - if (!string.IsNullOrEmpty(RuntimeModuleDirectory)) - { - localFilePath = Path.Combine(RuntimeModuleDirectory, Path.GetFileName(libraryInfo.FileName)); - } - else - { - localFilePath = Path.Combine(Path.GetDirectoryName(RuntimeModule.FileName), Path.GetFileName(libraryInfo.FileName)); - } + return PathUtilities.IsSafeAbsoluteLocalPath(libraryFileName) ? libraryFileName : null; } - if (!File.Exists(localFilePath)) + + if (!string.IsNullOrEmpty(runtimeModuleDirectory)) { - localFilePath = null; + return PathUtilities.IsSafeAbsoluteLocalPath(runtimeModuleDirectory) + ? Path.Combine(runtimeModuleDirectory, fileName) + : null; } - return localFilePath; + + if (PathUtilities.IsSafeAbsoluteLocalPath(runtimeModuleFileName)) + { + string runtimeDirectory = Path.GetDirectoryName(runtimeModuleFileName); + if (!string.IsNullOrEmpty(runtimeDirectory)) + { + return Path.Combine(runtimeDirectory, fileName); + } + } + + return null; } private string DownloadFile(DebugLibraryInfo libraryInfo) { OSPlatform platform = Target.OperatingSystem; string filePath = null; + string fileName = PathUtilities.GetFileName(libraryInfo.FileName); + if (string.IsNullOrEmpty(fileName)) + { + return null; + } if (_symbolService.IsSymbolStoreEnabled) { @@ -240,7 +286,7 @@ private string DownloadFile(DebugLibraryInfo libraryInfo) // It is the coreclr.dll's id (timestamp/filesize) in the DacInfo used to download the the dac module. if (libraryInfo.IndexTimeStamp != 0 && libraryInfo.IndexFileSize != 0) { - key = PEFileKeyGenerator.GetKey(libraryInfo.FileName, (uint)libraryInfo.IndexTimeStamp, (uint)libraryInfo.IndexFileSize); + key = PEFileKeyGenerator.GetKey(fileName, (uint)libraryInfo.IndexTimeStamp, (uint)libraryInfo.IndexFileSize); } else { @@ -255,13 +301,13 @@ private string DownloadFile(DebugLibraryInfo libraryInfo) byte[] buildId = libraryInfo.IndexBuildId.ToArray(); IEnumerable keys = null; KeyTypeFlags flags = KeyTypeFlags.None; - string fileName = null; + string keyGeneratorFileName = null; switch (libraryInfo.ArchivedUnder) { case SymbolProperties.Self: flags = KeyTypeFlags.IdentityKey; - fileName = libraryInfo.FileName; + keyGeneratorFileName = fileName; break; case SymbolProperties.Coreclr: flags = KeyTypeFlags.DacDbiKeys; @@ -270,18 +316,18 @@ private string DownloadFile(DebugLibraryInfo libraryInfo) if (platform == OSPlatform.Linux) { - keys = ELFFileKeyGenerator.GetKeys(flags, fileName ?? "libcoreclr.so", buildId, symbolFile: false, symbolFileName: null); + keys = ELFFileKeyGenerator.GetKeys(flags, keyGeneratorFileName ?? "libcoreclr.so", buildId, symbolFile: false, symbolFileName: null); } else if (platform == OSPlatform.OSX) { - keys = MachOFileKeyGenerator.GetKeys(flags, fileName ?? "libcoreclr.dylib", buildId, symbolFile: false, symbolFileName: null); + keys = MachOFileKeyGenerator.GetKeys(flags, keyGeneratorFileName ?? "libcoreclr.dylib", buildId, symbolFile: false, symbolFileName: null); } else { Trace.TraceError($"DownloadFile: {libraryInfo}: platform not supported - {platform}"); } - key = keys?.SingleOrDefault((k) => Path.GetFileName(k.FullPathName) == Path.GetFileName(libraryInfo.FileName)); + key = keys?.SingleOrDefault((k) => PathUtilities.GetFileName(k.FullPathName) == fileName); } else { diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/SymbolService.cs b/src/Microsoft.Diagnostics.DebugServices.Implementation/SymbolService.cs index a6024147a6..0d33df2093 100644 --- a/src/Microsoft.Diagnostics.DebugServices.Implementation/SymbolService.cs +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/SymbolService.cs @@ -609,8 +609,12 @@ private string DownloadPE(IModule module, KeyTypeFlags flags) { return null; } - fileName = module.FileName; - fileKey = PEFileKeyGenerator.GetKey(Path.GetFileName(fileName), module.IndexTimeStamp.Value, module.IndexFileSize.Value); + fileName = GetFilePathForLookup(module.FileName); + if (string.IsNullOrEmpty(fileName)) + { + return null; + } + fileKey = PEFileKeyGenerator.GetKey(PathUtilities.GetFileName(fileName), module.IndexTimeStamp.Value, module.IndexFileSize.Value); if (fileKey is null) { Trace.TraceWarning($"DownLoadPE: no key generated for module {fileName} "); @@ -622,9 +626,14 @@ private string DownloadPE(IModule module, KeyTypeFlags flags) IEnumerable pdbInfos = module.GetPdbFileInfos(); foreach (PdbFileInfo pdbInfo in pdbInfos) { + string pdbPath = GetFilePathForLookup(pdbInfo.Path); + if (string.IsNullOrEmpty(pdbPath)) + { + continue; + } if (pdbInfo.IsPortable) { - fileKey = PortablePDBFileKeyGenerator.GetKey(pdbInfo.Path, pdbInfo.Guid); + fileKey = PortablePDBFileKeyGenerator.GetKey(pdbPath, pdbInfo.Guid); if (fileKey is not null) { break; @@ -632,7 +641,7 @@ private string DownloadPE(IModule module, KeyTypeFlags flags) } else { - tempFileKey ??= PDBFileKeyGenerator.GetKey(pdbInfo.Path, pdbInfo.Guid, pdbInfo.Revision); + tempFileKey ??= PDBFileKeyGenerator.GetKey(pdbPath, pdbInfo.Guid, pdbInfo.Revision); } } @@ -653,7 +662,7 @@ private string DownloadPE(IModule module, KeyTypeFlags flags) } // Check if the file is local and the key matches the module - if (File.Exists(fileName)) + if (PathUtilities.IsSafeAbsoluteLocalPath(fileName) && File.Exists(fileName)) { using Stream stream = Utilities.TryOpenFile(fileName); if (stream is not null) @@ -726,8 +735,13 @@ private string DownloadELF(IModule module, KeyTypeFlags flags) return null; } - string symbolFileName = (flags & KeyTypeFlags.SymbolKey) != 0 ? module.GetSymbolFileName() : null; - SymbolStoreKey fileKey = ELFFileKeyGenerator.GetKeys(flags, module.FileName, module.BuildId.ToArray(), symbolFile: false, symbolFileName).SingleOrDefault(); + string moduleFileName = GetFilePathForLookup(module.FileName); + if (string.IsNullOrEmpty(moduleFileName)) + { + return null; + } + string symbolFileName = (flags & KeyTypeFlags.SymbolKey) != 0 ? GetFilePathForLookup(module.GetSymbolFileName()) : null; + SymbolStoreKey fileKey = ELFFileKeyGenerator.GetKeys(flags, moduleFileName, module.BuildId.ToArray(), symbolFile: false, symbolFileName).SingleOrDefault(); if (fileKey is null) { Trace.TraceWarning($"DownloadELF: no index generated for module {module.FileName} "); @@ -736,7 +750,7 @@ private string DownloadELF(IModule module, KeyTypeFlags flags) // Check if the file is local and the key matches the module string fileName = fileKey.FullPathName; - if (File.Exists(fileName)) + if (PathUtilities.IsSafeAbsoluteLocalPath(fileName) && File.Exists(fileName)) { using ELFFile elfFile = Utilities.OpenELFFile(fileName); if (elfFile is not null) @@ -783,7 +797,13 @@ private string DownloadMachO(IModule module, KeyTypeFlags flags) return null; } - SymbolStoreKey fileKey = MachOFileKeyGenerator.GetKeys(flags, module.FileName, module.BuildId.ToArray(), symbolFile: false, module.GetSymbolFileName()).SingleOrDefault(); + string moduleFileName = GetFilePathForLookup(module.FileName); + if (string.IsNullOrEmpty(moduleFileName)) + { + return null; + } + string symbolFileName = GetFilePathForLookup(module.GetSymbolFileName()); + SymbolStoreKey fileKey = MachOFileKeyGenerator.GetKeys(flags, moduleFileName, module.BuildId.ToArray(), symbolFile: false, symbolFileName).SingleOrDefault(); if (fileKey is null) { Trace.TraceWarning($"DownloadMachO: no index generated for module {module.FileName} "); @@ -792,7 +812,7 @@ private string DownloadMachO(IModule module, KeyTypeFlags flags) // Check if the file is local and the key matches the module string fileName = fileKey.FullPathName; - if (File.Exists(fileName)) + if (PathUtilities.IsSafeAbsoluteLocalPath(fileName) && File.Exists(fileName)) { using MachOFile machOFile = Utilities.OpenMachOFile(fileName); if (machOFile is not null) @@ -843,9 +863,16 @@ private string DownloadFile(SymbolStoreKey key) file.Stream.Position = 0; // If the downloaded doesn't already exists on disk in the cache, then write it to a temporary location. - if (!File.Exists(downloadFilePath)) + if (!PathUtilities.IsSafeAbsoluteLocalPath(downloadFilePath) || !File.Exists(downloadFilePath)) { - downloadFilePath = Path.Combine(_host.GetTempDirectory(), Path.GetRandomFileName() + "-" + Path.GetFileName(key.FullPathName)); + string fileName = PathUtilities.GetFileName(key.FullPathName); + string tempDirectory = _host.GetTempDirectory(); + if (!PathUtilities.IsSafeAbsoluteLocalPath(tempDirectory)) + { + Trace.TraceError($"Symbol staging directory is not a local absolute path: {tempDirectory ?? ""}"); + return null; + } + downloadFilePath = Path.Combine(tempDirectory, Path.GetRandomFileName() + "-" + fileName); using (Stream destinationStream = File.OpenWrite(downloadFilePath)) { file.Stream.CopyTo(destinationStream); @@ -863,6 +890,11 @@ private string DownloadFile(SymbolStoreKey key) return downloadFilePath; } + private static string GetFilePathForLookup(string filePath) + { + return PathUtilities.IsSafeAbsoluteLocalPath(filePath) ? filePath : PathUtilities.GetFileName(filePath); + } + private static void ReadPortableDebugTableEntries(PEReader peReader, out DebugDirectoryEntry codeViewEntry, out DebugDirectoryEntry embeddedPdbEntry) { // See spec: https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md @@ -894,21 +926,17 @@ private SymbolFile TryOpenReaderFromCodeView(PEReader peReader, DebugDirectoryEn try { CodeViewDebugDirectoryData data = peReader.ReadCodeViewDebugDirectoryData(codeViewEntry); - string pdbPath = data.Path; + string pdbPath = PathUtilities.GetFileName(data.Path); + if (string.IsNullOrEmpty(pdbPath)) + { + return null; + } Stream pdbStream = null; - if (assemblyPath != null) + string localPdbPath = GetLocalPdbPath(assemblyPath, pdbPath); + if (localPdbPath is not null) { - try - { - pdbPath = Path.Combine(Path.GetDirectoryName(assemblyPath), GetFileName(pdbPath)); - } - catch - { - // invalid characters in CodeView path - return null; - } - pdbStream = Utilities.TryOpenFile(pdbPath); + pdbStream = Utilities.TryOpenFile(localPdbPath); } if (pdbStream == null) @@ -951,6 +979,35 @@ private SymbolFile TryOpenReaderFromCodeView(PEReader peReader, DebugDirectoryEn return result; } + internal static string GetLocalPdbPath(string assemblyPath, string pdbPath) + { + if (!PathUtilities.IsSafeAbsoluteLocalPath(assemblyPath)) + { + return null; + } + + string fileName = PathUtilities.GetFileName(pdbPath); + if (string.IsNullOrEmpty(fileName)) + { + return null; + } + + try + { + string assemblyDirectory = Path.GetDirectoryName(assemblyPath); + if (string.IsNullOrEmpty(assemblyDirectory)) + { + return null; + } + string localPdbPath = Path.Combine(assemblyDirectory, fileName); + return PathUtilities.IsSafeAbsoluteLocalPath(localPdbPath) ? localPdbPath : null; + } + catch (ArgumentException) + { + return null; + } + } + private static SymbolFile TryOpenReaderFromEmbeddedPdb(PEReader peReader, DebugDirectoryEntry embeddedPdbEntry) { SymbolFile result = null; @@ -1040,7 +1097,7 @@ private SymbolStoreFile GetSymbolStoreFile(SymbolStoreKey key) /// Sets a new store store head. /// /// symbol store (server, cache, directory, etc.) - private void SetSymbolStore(SymbolStore.SymbolStores.SymbolStore store) + internal void SetSymbolStore(SymbolStore.SymbolStores.SymbolStore store) { if (store != _symbolStore) { diff --git a/src/Microsoft.Diagnostics.ExtensionCommands/Host/SetClrPathCommand.cs b/src/Microsoft.Diagnostics.ExtensionCommands/Host/SetClrPathCommand.cs index ee6a1a144a..87be1594f2 100644 --- a/src/Microsoft.Diagnostics.ExtensionCommands/Host/SetClrPathCommand.cs +++ b/src/Microsoft.Diagnostics.ExtensionCommands/Host/SetClrPathCommand.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.IO; using Microsoft.Diagnostics.DebugServices; @@ -34,7 +35,19 @@ public override void Invoke() } else { - Runtime.RuntimeModuleDirectory = Path.GetFullPath(Argument); + if (PathUtilities.IsRemoteOrDevicePath(Argument)) + { + throw new DiagnosticsException($"Runtime directory must be local: '{Argument}'"); + } + string fullPath = Path.GetFullPath(Argument); + try + { + Runtime.RuntimeModuleDirectory = fullPath; + } + catch (ArgumentException) + { + throw new DiagnosticsException($"Runtime directory must be a local absolute path: '{Argument}'"); + } WriteLine("Set load path for DAC/DBI to '{0}'", Runtime.RuntimeModuleDirectory); } } diff --git a/src/Microsoft.Diagnostics.ExtensionCommands/Microsoft.Diagnostics.ExtensionCommands.csproj b/src/Microsoft.Diagnostics.ExtensionCommands/Microsoft.Diagnostics.ExtensionCommands.csproj index 3ef1b60737..553a5b4f27 100644 --- a/src/Microsoft.Diagnostics.ExtensionCommands/Microsoft.Diagnostics.ExtensionCommands.csproj +++ b/src/Microsoft.Diagnostics.ExtensionCommands/Microsoft.Diagnostics.ExtensionCommands.csproj @@ -25,5 +25,6 @@ + diff --git a/src/SOS/SOS.Hosting/RuntimeWrapper.cs b/src/SOS/SOS.Hosting/RuntimeWrapper.cs index ac37d23e01..fa0dbaaff8 100644 --- a/src/SOS/SOS.Hosting/RuntimeWrapper.cs +++ b/src/SOS/SOS.Hosting/RuntimeWrapper.cs @@ -215,17 +215,26 @@ private void SetRuntimeDirectory( IntPtr self, string runtimeModuleDirectory) { + if (runtimeModuleDirectory is not null && !PathUtilities.IsSafeAbsoluteLocalPath(runtimeModuleDirectory)) + { + Trace.TraceError($"Ignoring non-local runtime module directory: {runtimeModuleDirectory}"); + return; + } _runtime.RuntimeModuleDirectory = runtimeModuleDirectory; } private string GetRuntimeDirectory( IntPtr self) { - if (_runtime.RuntimeModuleDirectory is not null) + if (PathUtilities.IsSafeAbsoluteLocalPath(_runtime.RuntimeModuleDirectory)) { return _runtime.RuntimeModuleDirectory; } - return Path.GetDirectoryName(_runtime.RuntimeModule.FileName); + if (PathUtilities.IsSafeAbsoluteLocalPath(_runtime.RuntimeModule.FileName)) + { + return Path.GetDirectoryName(_runtime.RuntimeModule.FileName); + } + return null; } private int GetClrDataProcess( @@ -384,9 +393,8 @@ private IntPtr CreateClrDataProcess(IntPtr dacHandle) private IntPtr CreateCorDebugProcess() { string dbiFilePath = _runtime.GetDbiFilePath(); - if (dbiFilePath == null) + if (!IsLoadableModulePath(dbiFilePath, "DBI")) { - Trace.TraceError($"Could not find matching DBI {dbiFilePath ?? ""} for this runtime: {_runtime.RuntimeModule.FileName}"); return IntPtr.Zero; } if (_dbiHandle == IntPtr.Zero) @@ -424,6 +432,10 @@ private IntPtr CreateCorDebugProcess() // The DAC was verified in the GetDacHandle call above. Ignore the verifySignature parameter here. string dacFilePath = _runtime.GetDacFilePath(out bool _); + if (!IsLoadableModulePath(dacFilePath, "DAC")) + { + return IntPtr.Zero; + } OpenVirtualProcessImpl2Delegate openVirtualProcessImpl2 = SOSHost.GetDelegateFunction(_dbiHandle, "OpenVirtualProcessImpl2"); if (openVirtualProcessImpl2 != null) @@ -536,9 +548,10 @@ private IntPtr GetDacHandle(bool useCDac) { bool verifySignature = false; string dacFilePath = useCDac ? _runtime.GetCDacFilePath() : _runtime.GetDacFilePath(out verifySignature); - if (dacFilePath == null) + bool forceUseContractReader = _services.GetService()?.ForceUseContractReader ?? false; + string moduleName = useCDac || forceUseContractReader ? "cDAC" : "DAC"; + if (!IsLoadableModulePath(dacFilePath, moduleName)) { - Trace.TraceError($"Could not find matching DAC {dacFilePath ?? ""} {useCDac} for this runtime: {_runtime.RuntimeModule.FileName}"); return IntPtr.Zero; } IntPtr dacHandle = IntPtr.Zero; @@ -579,6 +592,21 @@ private IntPtr GetDacHandle(bool useCDac) return dacHandle; } + private bool IsLoadableModulePath(string modulePath, string moduleName) + { + if (string.IsNullOrEmpty(modulePath)) + { + Trace.TraceError($"Could not find matching {moduleName} for this runtime: {_runtime.RuntimeModule.FileName}"); + return false; + } + if (!PathUtilities.IsSafeAbsoluteLocalPath(modulePath)) + { + Trace.TraceError($"Can't load {moduleName} from path '{modulePath}' because it is not local for this runtime: {_runtime.RuntimeModule.FileName}"); + return false; + } + return true; + } + #region IRuntime delegates [UnmanagedFunctionPointer(CallingConvention.Winapi)] diff --git a/src/SOS/SOS.Hosting/SOS.Hosting.csproj b/src/SOS/SOS.Hosting/SOS.Hosting.csproj index bb438ecf78..cfa81aeb5a 100644 --- a/src/SOS/SOS.Hosting/SOS.Hosting.csproj +++ b/src/SOS/SOS.Hosting/SOS.Hosting.csproj @@ -20,5 +20,6 @@ + diff --git a/src/SOS/Strike/platform/runtimeimpl.cpp b/src/SOS/Strike/platform/runtimeimpl.cpp index ffa6d238f4..36ef521344 100644 --- a/src/SOS/Strike/platform/runtimeimpl.cpp +++ b/src/SOS/Strike/platform/runtimeimpl.cpp @@ -55,6 +55,21 @@ typedef HMODULE (STDAPICALLTYPE *LoadLibraryWFnPtr)(LPCWSTR lpLibFileName); // Current runtime instance IRuntime* g_pRuntime = nullptr; +static bool IsLoadableModulePath(const char* modulePath, const char* moduleName) +{ + if (modulePath == nullptr || modulePath[0] == '\0') + { + ExtErr("Could not find matching %s\n", moduleName); + return false; + } + if (!IsSafeAbsoluteLocalPath(modulePath)) + { + ExtErr("Can't load %s from path '%s' because it is not local\n", moduleName, modulePath); + return false; + } + return true; +} + extern "C" bool TryGetSymbolWithCallback( bool (*readMemory)(void* address, void* buffer, size_t size), ULONG64 baseAddress, @@ -391,12 +406,17 @@ ULONG Runtime::Release() \**********************************************************************/ void Runtime::SetRuntimeDirectory(LPCSTR runtimeModuleDirectory) { + if (runtimeModuleDirectory != nullptr && !IsSafeAbsoluteLocalPath(runtimeModuleDirectory)) + { + ExtDbgOut("Ignoring non-local runtime module directory: %s\n", runtimeModuleDirectory); + return; + } if (m_runtimeDirectory != nullptr) { free((void*)m_runtimeDirectory); m_runtimeDirectory = nullptr; } - if (runtimeModuleDirectory != nullptr) + if (IsSafeAbsoluteLocalPath(runtimeModuleDirectory)) { m_runtimeDirectory = _strdup(runtimeModuleDirectory); } @@ -409,6 +429,11 @@ LPCSTR Runtime::GetRuntimeDirectory() { if (m_runtimeDirectory == nullptr) { + if (!IsSafeAbsoluteLocalPath(m_name)) + { + ExtDbgOut("Error: Runtime module path is not a local absolute path: %s\n", m_name != nullptr ? m_name : ""); + return nullptr; + } if (GetFileAttributesA(m_name) == INVALID_FILE_ATTRIBUTES) { ExtDbgOut("Error: Runtime module %s doesn't exist %08x\n", m_name, HRESULT_FROM_WIN32(GetLastError())); @@ -416,7 +441,7 @@ LPCSTR Runtime::GetRuntimeDirectory() } // Parse off the file name char* runtimeDirectory = _strdup(m_name); - char* lastSlash = strrchr(runtimeDirectory, GetTargetDirectorySeparatorW()); + char* lastSlash = GetLastDirectorySeparator(runtimeDirectory); if (lastSlash != nullptr) { *lastSlash = '\0'; @@ -436,7 +461,11 @@ HRESULT Runtime::GetClrDataProcess(ClrDataProcessFlags flags, IXCLRDataProcess** *ppClrDataProcess = nullptr; LPCSTR dacFilePath = GetDacFilePath(); - if (dacFilePath == nullptr) + if (dacFilePath == nullptr || dacFilePath[0] == '\0') + { + return CORDBG_E_NO_IMAGE_AVAILABLE; + } + if (!IsLoadableModulePath(dacFilePath, "DAC")) { return CORDBG_E_NO_IMAGE_AVAILABLE; } @@ -510,9 +539,8 @@ HRESULT Runtime::GetCorDebugInterface(ICorDebugProcess** ppCorDebugProcess) } #endif const char* dacFilePath = GetDacFilePath(); - if (dacFilePath == nullptr) + if (!IsLoadableModulePath(dacFilePath, "DAC")) { - ExtErr("Could not find matching DAC\n"); return CORDBG_E_NO_IMAGE_AVAILABLE; } ArrayHolder pDacModulePath = new WCHAR[MAX_LONGPATH + 1]; @@ -524,9 +552,8 @@ HRESULT Runtime::GetCorDebugInterface(ICorDebugProcess** ppCorDebugProcess) return hr; } const char* dbiFilePath = GetDbiFilePath(); - if (dbiFilePath == nullptr) + if (!IsLoadableModulePath(dbiFilePath, "DBI")) { - ExtErr("Could not find matching DBI\n"); return CORDBG_E_NO_IMAGE_AVAILABLE; } HMODULE hDbi = LoadLibraryA(dbiFilePath); diff --git a/src/SOS/Strike/strike.cpp b/src/SOS/Strike/strike.cpp index 40ad2d5fa9..01196788d5 100644 --- a/src/SOS/Strike/strike.cpp +++ b/src/SOS/Strike/strike.cpp @@ -13717,11 +13717,16 @@ DECLARE_API(SetClrPath) } if (narg > 0) { + if (IsRemoteOrDevicePath(runtimeModulePath.data)) + { + ExtErr("Runtime directory must be local: %s\n", runtimeModulePath.data); + return E_INVALIDARG; + } std::string fullPath; - if (!GetAbsolutePath(runtimeModulePath.data, fullPath)) + if (!GetAbsolutePath(runtimeModulePath.data, fullPath) || !IsSafeAbsoluteLocalPath(fullPath.c_str())) { - ExtErr("Invalid runtime directory %s\n", fullPath.c_str()); - return E_FAIL; + ExtErr("Invalid local runtime directory %s\n", runtimeModulePath.data); + return E_INVALIDARG; } g_pRuntime->SetRuntimeDirectory(fullPath.c_str()); } diff --git a/src/SOS/Strike/symbols.cpp b/src/SOS/Strike/symbols.cpp index 5502e9bb4a..00535ee18e 100644 --- a/src/SOS/Strike/symbols.cpp +++ b/src/SOS/Strike/symbols.cpp @@ -477,9 +477,25 @@ HRESULT SymbolReader::LoadSymbolsForWindowsPDB(___in IMetaDataImport* pMD, ___in pCallback = (IUnknown*) new PERvaMemoryReader(TO_TADDR(peAddress)); } + CorSymSearchPolicyAttributes searchPolicy = (CorSymSearchPolicyAttributes)(AllowRegistryAccess | AllowSymbolServerAccess); + const WCHAR* moduleName = pModuleName; + if (IsSafeAbsoluteLocalPath(pModuleName)) + { + searchPolicy = (CorSymSearchPolicyAttributes)(searchPolicy | AllowReferencePathAccess); + } + else + { + moduleName = GetFileName(pModuleName); + } + // TODO: this should be better integrated with windbg's symbol lookup - Status = g_pSymBinder->GetReaderFromCallback(pMD, pModuleName, symbolPath, - AllowRegistryAccess | AllowSymbolServerAccess | AllowOriginalPathAccess | AllowReferencePathAccess, pCallback, &m_pSymReader); + Status = g_pSymBinder->GetReaderFromCallback( + pMD, + moduleName, + symbolPath, + searchPolicy, + pCallback, + &m_pSymReader); if (FAILED(Status) && m_pSymReader != NULL) { diff --git a/src/SOS/Strike/util.cpp b/src/SOS/Strike/util.cpp index 8a70fb362a..524f31e3b9 100644 --- a/src/SOS/Strike/util.cpp +++ b/src/SOS/Strike/util.cpp @@ -238,6 +238,83 @@ const WCHAR GetTargetDirectorySeparatorW() } } +template +static bool IsSafeAbsoluteLocalPathImpl(const T* path) +{ + if (path == nullptr || path[0] == '\0') + { + return false; + } + + auto isDirectorySeparator = [](T character) { return character == '\\' || character == '/'; }; + if (path[1] != '\0' && isDirectorySeparator(path[0]) && isDirectorySeparator(path[1])) + { + return false; + } + +#ifdef FEATURE_PAL + return path[0] == '/'; +#else + return path[1] != '\0' && + path[2] != '\0' && + ((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z')) && + path[1] == ':' && + isDirectorySeparator(path[2]); +#endif +} + +template +static const T* GetLastDirectorySeparatorImpl(const T* path) +{ + const T* lastSeparator = nullptr; + if (path != nullptr) + { + for (const T* current = path; *current != '\0'; current++) + { + if (*current == '\\' || *current == '/') + { + lastSeparator = current; + } + } + } + return lastSeparator; +} + +bool IsRemoteOrDevicePath(const char* path) +{ + return path != nullptr && + path[0] != '\0' && + path[1] != '\0' && + (path[0] == '\\' || path[0] == '/') && + (path[1] == '\\' || path[1] == '/'); +} + +bool IsSafeAbsoluteLocalPath(const char* path) +{ + return IsSafeAbsoluteLocalPathImpl(path); +} + +bool IsSafeAbsoluteLocalPath(const WCHAR* path) +{ + return IsSafeAbsoluteLocalPathImpl(path); +} + +char* GetLastDirectorySeparator(char* path) +{ + return const_cast(GetLastDirectorySeparatorImpl(path)); +} + +const WCHAR* GetLastDirectorySeparator(const WCHAR* path) +{ + return GetLastDirectorySeparatorImpl(path); +} + +const WCHAR* GetFileName(const WCHAR* path) +{ + const WCHAR* lastSeparator = GetLastDirectorySeparator(path); + return lastSeparator != nullptr ? lastSeparator + 1 : path; +} + #ifndef FEATURE_PAL // Check if a file exist diff --git a/src/SOS/Strike/util.h b/src/SOS/Strike/util.h index f7d99c7039..c8c054295e 100644 --- a/src/SOS/Strike/util.h +++ b/src/SOS/Strike/util.h @@ -1383,6 +1383,12 @@ BOOL GetSOSVersion(VS_FIXEDFILEINFO *pFileInfo); BOOL IsDumpFile (); const WCHAR GetTargetDirectorySeparatorW(); +bool IsRemoteOrDevicePath(const char* path); +bool IsSafeAbsoluteLocalPath(const char* path); +bool IsSafeAbsoluteLocalPath(const WCHAR* path); +char* GetLastDirectorySeparator(char* path); +const WCHAR* GetLastDirectorySeparator(const WCHAR* path); +const WCHAR* GetFileName(const WCHAR* path); // IsMiniDumpFile will return true if 1) we are in // a small format minidump, and g_InMinidumpSafeMode is true. diff --git a/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/PathUtilitiesTests.cs b/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/PathUtilitiesTests.cs new file mode 100644 index 0000000000..e2a9cdcb16 --- /dev/null +++ b/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/PathUtilitiesTests.cs @@ -0,0 +1,127 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; +using Microsoft.Diagnostics.DebugServices; +using Microsoft.Diagnostics.DebugServices.Implementation; +using Microsoft.Diagnostics.Runtime; +using Xunit; + +namespace Microsoft.Diagnostics.DebugServices.UnitTests +{ + public class PathUtilitiesTests + { + [Theory] + [InlineData(@"\\remote.example\share\mscordaccore.dll")] + [InlineData("//remote.example/share/mscordaccore.dll")] + [InlineData(@"\/remote/share/mscordaccore.dll")] + [InlineData(@"/\remote/share/mscordaccore.dll")] + [InlineData(@"\\?\C:\Windows\mscordaccore.dll")] + [InlineData(@"\\?\UNC\srv\share\mscordaccore.dll")] + [InlineData(@"\\.\PIPE\mscordaccore")] + [InlineData(@"\\;X:\\srv\share\mscordaccore.dll")] + [InlineData(@"\\srv@SSL\DavWWWRoot\mscordaccore.dll")] + public void IsSafeAbsoluteLocalPathRejectsRemoteOrDevicePaths(string path) + { + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath(path)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("mscordaccore.dll")] + [InlineData(@"sub\mscordaccore.dll")] + [InlineData("sub/mscordaccore.dll")] + [InlineData(@"..\mscordaccore.dll")] + [InlineData("../mscordaccore.dll")] + [InlineData(@".\mscordaccore.dll")] + public void IsSafeAbsoluteLocalPathRejectsEmptyOrRelativePaths(string path) + { + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath(path)); + } + + [Fact] + public void IsSafeAbsoluteLocalPathAcceptsOnlyHostLocalAbsolutePaths() + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.True(PathUtilities.IsSafeAbsoluteLocalPath(@"C:\Windows\mscordaccore.dll")); + Assert.True(PathUtilities.IsSafeAbsoluteLocalPath("C:/Windows/mscordaccore.dll")); + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath(@"C:mscordaccore.dll")); + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath("C:")); + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath(@"1:\mscordaccore.dll")); + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath(@"\mscordaccore.dll")); + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath("/mscordaccore.dll")); + } + else + { + Assert.True(PathUtilities.IsSafeAbsoluteLocalPath("/usr/share/dotnet/libmscordaccore.so")); + Assert.False(PathUtilities.IsSafeAbsoluteLocalPath(@"C:\Windows\mscordaccore.dll")); + } + } + + [Theory] + [InlineData(@"\\remote.example\share\mscordaccore.dll", "mscordaccore.dll")] + [InlineData("//remote.example/share/mscordaccore.dll", "mscordaccore.dll")] + [InlineData(@"C:\Windows\mscordaccore.dll", "mscordaccore.dll")] + [InlineData("/usr/share/dotnet/libmscordaccore.so", "libmscordaccore.so")] + [InlineData(@"a\b/libmscordbi.so", "libmscordbi.so")] + [InlineData("mscordaccore.dll", "mscordaccore.dll")] + [InlineData(@"dir\", "")] + [InlineData("dir/", "")] + public void GetFileNameStripsBothDirectorySeparators(string path, string expected) + { + Assert.Equal(expected, PathUtilities.GetFileName(path)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void GetFileNamePassesThroughNullOrEmptyPaths(string path) + { + Assert.Equal(path, PathUtilities.GetFileName(path)); + } + + [Theory] + [InlineData(DebugLibraryKind.Dac)] + [InlineData(DebugLibraryKind.Dbi)] + [InlineData(DebugLibraryKind.CDac)] + public void RuntimeRejectsRemoteLibraryPaths(DebugLibraryKind kind) + { + string candidate = Microsoft.Diagnostics.DebugServices.Implementation.Runtime.GetLocalCandidatePath( + kind, + @"\\remote.example\share\mscordaccore.dll", + null, + @"\\remote.example\share\coreclr.dll"); + + Assert.Null(candidate); + } + + [Fact] + public void RuntimeRejectsRemoteRuntimeDirectoryOverride() + { + string runtimeModulePath = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? @"C:\dotnet\coreclr.dll" + : "/usr/share/dotnet/libcoreclr.so"; + + string candidate = Microsoft.Diagnostics.DebugServices.Implementation.Runtime.GetLocalCandidatePath( + DebugLibraryKind.Dac, + "mscordaccore.dll", + @"\\remote.example\share", + runtimeModulePath); + + Assert.Null(candidate); + } + + [Fact] + public void SymbolServiceRejectsAdjacentPdbProbeForRemoteAssembly() + { + string pdbPath = SymbolService.GetLocalPdbPath( + @"\\remote.example\share\module.dll", + @"\\remote.example\share\module.pdb"); + + Assert.Null(pdbPath); + } + } +} diff --git a/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/SymbolServiceTests.cs b/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/SymbolServiceTests.cs index b7276ccf36..5b45f41ef9 100644 --- a/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/SymbolServiceTests.cs +++ b/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/SymbolServiceTests.cs @@ -3,9 +3,14 @@ using System; using System.Collections.Generic; +using System.IO; using System.Runtime.InteropServices; using System.Text; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Diagnostics.DebugServices.Implementation; +using Microsoft.SymbolStore; +using Microsoft.SymbolStore.SymbolStores; using Xunit; namespace Microsoft.Diagnostics.DebugServices.UnitTests @@ -13,12 +18,22 @@ namespace Microsoft.Diagnostics.DebugServices.UnitTests /// /// Test the service event implementation /// - public class SymbolServiceTests : IHost + public class SymbolServiceTests : IHost, IDisposable { + private readonly string _tempDirectory = Path.Combine(Path.GetTempPath(), "SymbolServiceTests-" + Guid.NewGuid().ToString("N")); + public SymbolServiceTests() { } + public void Dispose() + { + if (Directory.Exists(_tempDirectory)) + { + Directory.Delete(_tempDirectory, recursive: true); + } + } + [Fact] public void SymbolPathTests() { @@ -120,6 +135,20 @@ public void OpenSymbolFile_ReturnsNull_ForInvalidPdbStream() Assert.Null(result); } + [Fact] + public void DownloadFileStagesRemoteStoreFileLocally() + { + byte[] contents = new byte[] { 1, 2, 3, 4 }; + SymbolService symbolService = new(this); + symbolService.SetSymbolStore(new TestSymbolStore(contents, @"\\server\share\mscordaccore.dll")); + + string filePath = symbolService.DownloadFile("mscordaccore.dll/test", "mscordaccore.dll"); + + Assert.True(PathUtilities.IsSafeAbsoluteLocalPath(filePath)); + Assert.StartsWith(_tempDirectory, filePath, StringComparison.OrdinalIgnoreCase); + Assert.Equal(contents, File.ReadAllBytes(filePath)); + } + #region IHost public IServiceEvent OnShutdownEvent { get; } = new ServiceEvent(); @@ -134,9 +163,32 @@ public void OpenSymbolFile_ReturnsNull_ForInvalidPdbStream() public int AddTarget(ITarget target) => throw new NotImplementedException(); - public string GetTempDirectory() => throw new NotImplementedException(); + public string GetTempDirectory() + { + Directory.CreateDirectory(_tempDirectory); + return _tempDirectory; + } #endregion + + private sealed class TestSymbolStore : Microsoft.SymbolStore.SymbolStores.SymbolStore + { + private readonly byte[] _contents; + private readonly string _fileName; + + public TestSymbolStore(byte[] contents, string fileName) + : base(Microsoft.Diagnostics.DebugServices.Implementation.Tracer.Instance) + { + _contents = contents; + _fileName = fileName; + } + + protected override Task GetFileInner(SymbolStoreKey key, CancellationToken token) + { + Stream stream = new MemoryStream(_contents, writable: false); + return Task.FromResult(new SymbolStoreFile(stream, _fileName)); + } + } } public static class SymbolServiceExtensions