Skip to content

Validate SOS diagnostic module paths #6037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
hoyosjs wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Validate SOS diagnostic module paths #6037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/Microsoft.Diagnostics.DebugServices.Implementation/DataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private sealed class DataReaderModule : ModuleInfo
private IResourceNode _resourceRoot;

public DataReaderModule(IDataReader reader, IModule module)
: base(module.ImageBase, module.FileName)
: base(module.ImageBase, GetFilePathForClrMd(module.FileName))
{
_reader = reader;
_module = module;
Expand Down Expand Up @@ -179,7 +179,7 @@ public override PdbInfo Pdb
return default;
}
}
return new PdbInfo(pdbFileInfo.Path, pdbFileInfo.Guid, pdbFileInfo.Revision);
return new PdbInfo(GetFilePathForClrMd(pdbFileInfo.Path), pdbFileInfo.Guid, pdbFileInfo.Revision);
}
catch (DiagnosticsException ex)
{
Expand All @@ -206,5 +206,10 @@ public override ulong GetExportSymbolAddress(string symbol)

public override IResourceNode ResourceRoot => _resourceRoot ??= ModuleInfo.TryCreateResourceRoot(_reader, _module.ImageBase, _module.ImageSize, _module.IsFileLayout.GetValueOrDefault(false));
}

internal static string GetFilePathForClrMd(string filePath)
{
return PathUtilities.IsSafeAbsoluteLocalPath(filePath) ? filePath : PathUtilities.GetFileName(filePath);
}
}
}
4 changes: 4 additions & 0 deletions ...cs.DebugServices.Implementation/Microsoft.Diagnostics.DebugServices.Implementation.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,8 @@
<ProjectReference Include="$(MSBuildThisFileDirectory)..\Microsoft.Diagnostics.DebugServices\Microsoft.Diagnostics.DebugServices.csproj" />
<ProjectReference Include="$(MSBuildThisFileDirectory)..\Microsoft.SymbolStore\Microsoft.SymbolStore.csproj" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Microsoft.Diagnostics.DebugServices.UnitTests" />
</ItemGroup>
</Project>
59 changes: 59 additions & 0 deletions src/Microsoft.Diagnostics.DebugServices.Implementation/PathUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// 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);
string fileName = index >= 0 ? path.Substring(index + 1) : path;
int drivePrefixLength = 0;
while (fileName.Length - drivePrefixLength >= 2 &&
IsAsciiLetter(fileName[drivePrefixLength]) &&
fileName[drivePrefixLength + 1] == ':')
{
drivePrefixLength += 2;
}
return drivePrefixLength > 0 ? fileName.Substring(drivePrefixLength) : fileName;
}

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';
}
}
78 changes: 62 additions & 16 deletions src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class Runtime : IRuntime, IDisposable
private string _dacFilePath;
private bool _verifySignature;
private string _dbiFilePath;
private string _runtimeModuleDirectory;

protected readonly ServiceContainer _serviceContainer;

Expand Down Expand Up @@ -78,7 +79,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
{
Expand Down Expand Up @@ -256,10 +268,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;
}
}
}
Expand All @@ -269,26 +286,55 @@ private string GetLibraryPath(DebugLibraryKind kind)

private string GetLocalPath(DebugLibraryInfo libraryInfo)
{
string localFilePath;
if (!string.IsNullOrEmpty(RuntimeModuleDirectory))
string localFilePath = GetLocalCandidatePath(
libraryInfo.FileName,
RuntimeModuleDirectory,
RuntimeModule.FileName);

return PathUtilities.IsSafeAbsoluteLocalPath(localFilePath) && File.Exists(localFilePath)
? localFilePath
: null;
}

internal static string GetLocalCandidatePath(
string libraryFileName,
string runtimeModuleDirectory,
string runtimeModuleFileName)
{
string fileName = PathUtilities.GetFileName(libraryFileName);
if (string.IsNullOrEmpty(fileName))
{
localFilePath = Path.Combine(RuntimeModuleDirectory, Path.GetFileName(libraryInfo.FileName));
return null;
}
else

if (!string.IsNullOrEmpty(runtimeModuleDirectory))
{
localFilePath = Path.Combine(Path.GetDirectoryName(RuntimeModule.FileName), Path.GetFileName(libraryInfo.FileName));
return PathUtilities.IsSafeAbsoluteLocalPath(runtimeModuleDirectory)
? Path.Combine(runtimeModuleDirectory, fileName)
: null;
}
if (localFilePath is null || !File.Exists(localFilePath))

if (PathUtilities.IsSafeAbsoluteLocalPath(runtimeModuleFileName))
{
localFilePath = null;
string runtimeDirectory = Path.GetDirectoryName(runtimeModuleFileName);
if (!string.IsNullOrEmpty(runtimeDirectory))
{
return Path.Combine(runtimeDirectory, fileName);
}
}
return localFilePath;

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)
{
Expand All @@ -299,7 +345,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
{
Expand All @@ -314,13 +360,13 @@ private string DownloadFile(DebugLibraryInfo libraryInfo)
byte[] buildId = libraryInfo.IndexBuildId.ToArray();
IEnumerable<SymbolStoreKey> 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;
Expand All @@ -329,18 +375,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
{
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Diagnostics.DebugServices.Implementation/RuntimeProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public IEnumerable<IRuntime> EnumerateRuntimes(int startingRuntimeId, RuntimeEnu
// (the bundled binary next to sos); matching by file name alone would let a name-hijacked DLL
// loaded from elsewhere (target runtime dir, symbol cache, ...) bypass verification.
string trustedCDacPath = _services.GetService<IHostAssetResolver>()?.GetCDacPath();
string normalizedTrustedCDacPath = string.IsNullOrEmpty(trustedCDacPath) ? null : Path.GetFullPath(trustedCDacPath);
string normalizedTrustedCDacPath = PathUtilities.IsSafeAbsoluteLocalPath(trustedCDacPath) ? Path.GetFullPath(trustedCDacPath) : null;
StringComparison pathComparison = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;

DataTarget dataTarget = new(_services.GetService<IDataReader>(), new DataTargetOptions()
Expand All @@ -55,7 +55,7 @@ public IEnumerable<IRuntime> EnumerateRuntimes(int startingRuntimeId, RuntimeEnu
DacSignatureVerificationOverride = (dacFilePath) =>
{
if (normalizedTrustedCDacPath is not null
&& !string.IsNullOrEmpty(dacFilePath)
&& PathUtilities.IsSafeAbsoluteLocalPath(dacFilePath)
&& string.Equals(Path.GetFullPath(dacFilePath), normalizedTrustedCDacPath, pathComparison))
{
return false;
Expand Down
Loading
Loading