Skip to content

Commit 60c3232

Browse files
brianrobbrianrob
authored
Bring v3.2.4 security and reliability fixes into main (#2435)
## Summary These changes shipped in the **PerfView and TraceEvent 3.2.4** release. This PR brings them into `main` so that all future releases also contain them. ## Hardening fixes **Malformed-input bounds checking** - Fix BPerf WildCopy so short copies do not write past the requested output range - Validate BPerf event record bounds - Validate dynamic event array bounds - Bounds-check StackBytesSize in EventPipe V3 event headers - Bounds-check GCDynamic event payloads - Bounds-check TDH metadata offsets in RegisteredTraceEventParser - Bound CodeView PDB filename read by IMAGE_DEBUG_DIRECTORY SizeOfData - Bound PE resource name reads by the resource data directory size **Path-containment hardening** - Fix source-server path traversal and command injection - Validate PDB extraction target paths against the symbol directory - Sanitize R2R perf map symbol paths - Constrain DiagSession resource extraction paths - Validate PdbScope XML module paths - Sanitize provider names when writing dynamic manifests ## Release mechanics - Bump ReleaseVersion to 3.2.4
2 parents 422494e + 69fa0e6 commit 60c3232

35 files changed

Lines changed: 5088 additions & 264 deletions

‎src/Directory.Build.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
<PropertyGroup>
2121
<!-- These are the versions of the things we are CREATING in this repository -->
22-
<ReleaseVersion>3.2.3</ReleaseVersion>
22+
<ReleaseVersion>3.2.4</ReleaseVersion>
2323
<FastSerializationVersion>$(ReleaseVersion)</FastSerializationVersion>
2424
<HeapDumpDllVersion>$(ReleaseVersion)</HeapDumpDllVersion>
2525
<MemoryGraphVersion>$(ReleaseVersion)</MemoryGraphVersion>

‎src/Directory.Packages.props‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
<PackageVersion Include="System.Buffers" Version="4.6.1" />
3030
<PackageVersion Include="System.Collections.Immutable" Version="9.0.8" />
3131
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="9.0.8" />
32+
<PackageVersion Include="System.IO.Hashing" Version="9.0.8" />
3233
<PackageVersion Include="System.Memory" Version="4.6.3" />
3334
<PackageVersion Include="System.Numerics.Vectors" Version="4.6.1" />
3435
<PackageVersion Include="System.Reflection.Metadata" Version="9.0.8" />

‎src/PerfView.Tests/DiagSessionPerfViewFileTests.cs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
using PerfView;
2+
using System.IO;
3+
using System.Linq;
4+
using Xunit;
5+
6+
namespace PerfViewTests
7+
{
8+
public class DiagSessionPerfViewFileTests
9+
{
10+
[Theory]
11+
[InlineData(@"..\..\victim", "victim")]
12+
[InlineData(@"\..\..\Startup", "Startup")]
13+
[InlineData(@"C:\temp\symbols", "symbols")]
14+
[InlineData("symbols/cache", "cache")]
15+
[InlineData("symbols\\cache", "cache")]
16+
[InlineData("foo.bar", "foo")]
17+
[InlineData("plain", "plain")]
18+
[InlineData("C:relative", "relative")]
19+
[InlineData("foo:bar", "bar")]
20+
public void GetSafeDiagSessionResourceDirectoryName_StripsPathComponents(string resourceName, string expected)
21+
{
22+
string safeName = DiagSessionPerfViewFile.GetSafeDiagSessionResourceDirectoryName(resourceName);
23+
24+
Assert.Equal(expected, safeName);
25+
Assert.DoesNotContain(Path.DirectorySeparatorChar, safeName);
26+
Assert.DoesNotContain(Path.AltDirectorySeparatorChar, safeName);
27+
Assert.DoesNotContain(safeName, c => Path.GetInvalidFileNameChars().Contains(c));
28+
}
29+
30+
[Theory]
31+
[InlineData(null)]
32+
[InlineData("")]
33+
[InlineData(".")]
34+
[InlineData("..")]
35+
[InlineData(@"\")]
36+
[InlineData("/")]
37+
[InlineData(@"foo\..")]
38+
[InlineData(@"foo\.")]
39+
public void GetSafeDiagSessionResourceDirectoryName_RejectsUnsafeNames(string resourceName)
40+
{
41+
string safeName = DiagSessionPerfViewFile.GetSafeDiagSessionResourceDirectoryName(resourceName);
42+
43+
Assert.Null(safeName);
44+
}
45+
}
46+
}

‎src/PerfView.Tests/Memory/PathUtilitiesTests.cs‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
using Microsoft.Diagnostics.Utilities;
2+
using Xunit;
3+
4+
namespace PerfViewTests.Memory
5+
{
6+
public class PathUtilitiesTests
7+
{
8+
[Theory]
9+
[InlineData(@"\\server\share\module.dll")]
10+
[InlineData(@"\\?\UNC\server\share\module.dll")]
11+
[InlineData(@"\\?\unc\server\share\module.dll")]
12+
[InlineData(@"\\.\UNC\server\share\module.dll")]
13+
[InlineData(@"\\?\GLOBALROOT\Device\Mup\server\share\module.dll")]
14+
[InlineData(@"\\?\GLOBALROOT\Device\LanmanRedirector\server\share\module.dll")]
15+
[InlineData(@"\??\UNC\server\share\module.dll")]
16+
[InlineData(@"\??\unc\server\share\module.dll")]
17+
[InlineData("https://server/share/module.dll")]
18+
[InlineData("http://server/share/module.dll")]
19+
[InlineData("ftp://server/module.dll")]
20+
public void IsRemotePathDetectsRemotePaths(string modulePath)
21+
{
22+
Assert.True(PathUtilities.IsRemotePath(modulePath));
23+
}
24+
25+
[Theory]
26+
[InlineData(@"C:\Symbols\foo.pdb\01234567890123456789012345678901FFFFFFFF\foo.pdb")]
27+
[InlineData(@"C:\Users\dev\src\bin\foo.dll")]
28+
[InlineData(@"module.dll")]
29+
[InlineData(@"subdir\module.dll")]
30+
[InlineData(@"..\module.dll")]
31+
[InlineData(@"D:\drive\path.dll")]
32+
[InlineData(@"\\?\C:\Windows\notepad.exe")]
33+
[InlineData(@"\\.\C:\Windows\notepad.exe")]
34+
[InlineData(@"\\?\Volume{12345678-1234-1234-1234-1234567890ab}\foo.dll")]
35+
public void IsRemotePathAcceptsLocalPaths(string modulePath)
36+
{
37+
Assert.False(PathUtilities.IsRemotePath(modulePath));
38+
}
39+
40+
[Theory]
41+
[InlineData(null)]
42+
[InlineData("")]
43+
public void IsRemotePathHandlesEmptyInputWithoutThrowing(string modulePath)
44+
{
45+
// Empty/null inputs are not remote (they will be caught elsewhere).
46+
Assert.False(PathUtilities.IsRemotePath(modulePath));
47+
}
48+
49+
[Theory]
50+
[InlineData(null)]
51+
[InlineData("")]
52+
[InlineData(".")]
53+
[InlineData("..")]
54+
public void SanitizeFileName_ReturnsNullForRejectedInput(string input)
55+
{
56+
// null / empty / "." / ".." all return null so callers can choose how to
57+
// handle the missing name (skip the resource, substitute a placeholder,
58+
// etc.) instead of being forced to accept an arbitrary string.
59+
Assert.Null(PathUtilities.SanitizeFileName(input));
60+
}
61+
62+
[Theory]
63+
[InlineData(@"..\outside", ".._outside")]
64+
[InlineData(@"..\..\..\Startup\x", ".._.._.._Startup_x")]
65+
[InlineData("../forward/slash", ".._forward_slash")]
66+
[InlineData(@"C:\Windows\System32\evil", "C__Windows_System32_evil")]
67+
[InlineData(@"\\server\share\evil", "__server_share_evil")]
68+
[InlineData(@"with:colons", "with_colons")]
69+
[InlineData("with|pipes?and*wildcards", "with_pipes_and_wildcards")]
70+
public void SanitizeFileName_ReplacesInvalidCharactersAndSeparators(string input, string expected)
71+
{
72+
// Every path separator, volume separator, and Path.GetInvalidFileNameChars
73+
// character is replaced with '_'. Control characters are also replaced.
74+
Assert.Equal(expected, PathUtilities.SanitizeFileName(input));
75+
}
76+
77+
[Theory]
78+
[InlineData("CON", "_CON")]
79+
[InlineData("nul", "_nul")]
80+
[InlineData("PRN", "_PRN")]
81+
[InlineData("AUX", "_AUX")]
82+
[InlineData("COM1", "_COM1")]
83+
[InlineData("LPT9", "_LPT9")]
84+
[InlineData("CLOCK$", "_CLOCK$")]
85+
[InlineData("CONIN$", "_CONIN$")]
86+
[InlineData("conout$", "_conout$")]
87+
[InlineData("CONIN$.log", "_CONIN$.log")]
88+
[InlineData("NUL.", "_NUL")]
89+
[InlineData("NUL ", "_NUL")]
90+
[InlineData("NUL. . ", "_NUL")]
91+
[InlineData("NUL.evil", "_NUL.evil")]
92+
[InlineData("CON.foo.bar", "_CON.foo.bar")]
93+
[InlineData("com1.data", "_com1.data")]
94+
[InlineData("LPT5.tar.gz", "_LPT5.tar.gz")]
95+
public void SanitizeFileName_RewritesReservedDosDeviceNames(string input, string expected)
96+
{
97+
// Win32 matches a reserved device name on the stem before the first '.'
98+
// in the basename, so "NUL.evil" or "COM1.data" still open the device.
99+
// The sanitizer prefixes such names with '_' to make them safe.
100+
Assert.Equal(expected, PathUtilities.SanitizeFileName(input));
101+
}
102+
103+
[Theory]
104+
[InlineData("Trailing.", "Trailing")]
105+
[InlineData("Trailing ", "Trailing")]
106+
[InlineData("Trailing.. .", "Trailing")]
107+
public void SanitizeFileName_StripsTrailingDotsAndSpaces(string input, string expected)
108+
{
109+
// Windows silently trims trailing '.' and ' ' from file names; stripping
110+
// them ourselves prevents two distinct names colliding on disk and
111+
// closes the "NUL." device-name evasion.
112+
Assert.Equal(expected, PathUtilities.SanitizeFileName(input));
113+
}
114+
115+
[Theory]
116+
[InlineData("...")]
117+
[InlineData(" ")]
118+
public void SanitizeFileName_ReturnsNullWhenAllCharactersAreStripped(string input)
119+
{
120+
// Inputs that consist entirely of trailing-trim characters (or sanitize
121+
// to nothing) return null rather than the empty string.
122+
Assert.Null(PathUtilities.SanitizeFileName(input));
123+
}
124+
125+
[Theory]
126+
[InlineData("Contoso.Provider-Valid_1", "Contoso.Provider-Valid_1")]
127+
[InlineData("My Provider", "My Provider")]
128+
[InlineData("provider.with.dots", "provider.with.dots")]
129+
public void SanitizeFileName_PreservesValidNames(string input, string expected)
130+
{
131+
Assert.Equal(expected, PathUtilities.SanitizeFileName(input));
132+
}
133+
}
134+
}
135+

‎src/PerfView.Tests/Memory/PdbScopeMemoryGraphTests.cs‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
using PerfView;
2+
using System;
3+
using System.IO;
4+
using Xunit;
5+
6+
namespace PerfViewTests.Memory
7+
{
8+
public class PdbScopeMemoryGraphTests : IDisposable
9+
{
10+
private readonly string m_testDirectory;
11+
12+
public PdbScopeMemoryGraphTests()
13+
{
14+
m_testDirectory = Path.Combine(Path.GetTempPath(), "PdbScopeMemoryGraphTests", Guid.NewGuid().ToString("N"));
15+
Directory.CreateDirectory(m_testDirectory);
16+
}
17+
18+
public void Dispose()
19+
{
20+
if (Directory.Exists(m_testDirectory))
21+
{
22+
Directory.Delete(m_testDirectory, recursive: true);
23+
}
24+
}
25+
26+
[Theory]
27+
[InlineData(@"\\server\share\module.dll")]
28+
[InlineData(@"\\?\UNC\server\share\module.dll")]
29+
[InlineData("https://server/share/module.dll")]
30+
public void TryResolveTrustedFilePathRejectsRemotePaths(string modulePath)
31+
{
32+
string pdbScopeFilePath = GetPdbScopeFilePath();
33+
34+
Assert.False(PdbScopeMemoryGraph.TryResolveTrustedFilePath(modulePath, pdbScopeFilePath, out string trustedFilePath));
35+
Assert.Null(trustedFilePath);
36+
}
37+
38+
[Fact]
39+
public void TryResolveTrustedFilePathRejectsRootedPathsOutsidePdbScopeDirectory()
40+
{
41+
string pdbScopeFilePath = GetPdbScopeFilePath();
42+
string outsideDirectory = Path.Combine(Path.GetPathRoot(m_testDirectory), "PdbScopeMemoryGraphTestsOutside");
43+
string outsideModulePath = Path.Combine(outsideDirectory, "module.dll");
44+
45+
Assert.False(PdbScopeMemoryGraph.TryResolveTrustedFilePath(outsideModulePath, pdbScopeFilePath, out string trustedFilePath));
46+
Assert.Null(trustedFilePath);
47+
}
48+
49+
[Theory]
50+
[InlineData("module.dll")]
51+
[InlineData(@"subdirectory\module.dll")]
52+
public void TryResolveTrustedFilePathAllowsRelativePathsUnderPdbScopeDirectory(string modulePath)
53+
{
54+
string pdbScopeFilePath = GetPdbScopeFilePath();
55+
56+
Assert.True(PdbScopeMemoryGraph.TryResolveTrustedFilePath(modulePath, pdbScopeFilePath, out string trustedFilePath));
57+
Assert.Equal(Path.GetFullPath(Path.Combine(m_testDirectory, modulePath)), trustedFilePath);
58+
}
59+
60+
[Fact]
61+
public void TryResolveTrustedFilePathAllowsRootedPathsUnderPdbScopeDirectory()
62+
{
63+
string pdbScopeFilePath = GetPdbScopeFilePath();
64+
string modulePath = Path.Combine(m_testDirectory, "module.dll");
65+
66+
Assert.True(PdbScopeMemoryGraph.TryResolveTrustedFilePath(modulePath, pdbScopeFilePath, out string trustedFilePath));
67+
Assert.Equal(modulePath, trustedFilePath);
68+
}
69+
70+
[Theory]
71+
[InlineData(@"..\module.dll")]
72+
[InlineData(@"subdirectory\..\..\module.dll")]
73+
public void TryResolveTrustedFilePathRejectsRelativePathsEscapingPdbScopeDirectory(string modulePath)
74+
{
75+
string pdbScopeFilePath = GetPdbScopeFilePath();
76+
77+
Assert.False(PdbScopeMemoryGraph.TryResolveTrustedFilePath(modulePath, pdbScopeFilePath, out string trustedFilePath));
78+
Assert.Null(trustedFilePath);
79+
}
80+
81+
[Fact]
82+
public void PdbScopeXmlWithRemoteModuleFilePathDoesNotThrow()
83+
{
84+
CommandProcessor originalCommandProcessor = App.CommandProcessor;
85+
string pdbScopeFilePath = GetPdbScopeFilePath();
86+
try
87+
{
88+
App.CommandProcessor = new CommandProcessor() { LogFile = TextWriter.Null };
89+
File.WriteAllText(
90+
pdbScopeFilePath,
91+
@"<PdbscopeReport><Section Start=""8192"" Size=""16"" Name="".text"" /><Module Base=""4096"" FilePath=""\\server\share\module.dll"" /><Symbol addr=""2000"" size=""16"" name=""Main"" tag=""code"" /></PdbscopeReport>");
92+
93+
new PdbScopeMemoryGraph(pdbScopeFilePath);
94+
}
95+
finally
96+
{
97+
App.CommandProcessor = originalCommandProcessor;
98+
}
99+
}
100+
101+
private string GetPdbScopeFilePath()
102+
{
103+
return Path.Combine(m_testDirectory, "test.imageSize.xml");
104+
}
105+
}
106+
}

‎src/PerfView/App.cs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,11 +904,33 @@ public static SymbolReader GetSymbolReader(string etlFilePath = null, SymbolRead
904904

905905
return result == System.Windows.MessageBoxResult.Yes;
906906
};
907+
908+
ret.AuthorizeSourceServerCommand = request =>
909+
{
910+
var result = XamlMessageBox.Show(
911+
request.Command + "\n\n" +
912+
"This command was derived from PDB-supplied data. Do you want to run it?",
913+
"Source Server Command",
914+
System.Windows.MessageBoxButton.YesNo);
915+
916+
bool allowed = result == System.Windows.MessageBoxResult.Yes;
917+
log.WriteLine("Source Server command authorization {0} by user: {1}", allowed ? "GRANTED" : "DENIED", request.Command);
918+
return allowed;
919+
};
907920
}
908921
else
909922
#endif
910923
{
911924
ret.SecurityCheck = (pdbFile => true);
925+
ret.AuthorizeSourceServerCommand = request =>
926+
{
927+
#if PERFVIEW_COLLECT
928+
log.WriteLine("Source Server command auto-approved in PerfViewCollect: {0}", request.Command);
929+
#else
930+
log.WriteLine("Source Server command auto-approved because /TrustPdbs is set: {0}", request.Command);
931+
#endif
932+
return true;
933+
};
912934
}
913935
ret.SourceCacheDirectory = Path.Combine(CacheFiles.CacheDir, "src");
914936
if (localSymDir != null)

‎src/PerfView/Extensibility.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,12 @@ private static void UnZipIfNecessary(ref string inputFileName, TextWriter log, b
708708
log.WriteLine("Putting symbols in {0}", dirForPdbs);
709709
}
710710

711-
var pdbTargetPath = Path.Combine(dirForPdbs, pdbRelativePath);
711+
if (!SymbolCachePathUtilities.TryGetPdbTargetPath(dirForPdbs, pdbRelativePath, out var pdbTargetPath))
712+
{
713+
log.WriteLine("WARNING: found PDB file with invalid path {0}, skipping extraction", pdbRelativePath);
714+
continue;
715+
}
716+
712717
var pdbTargetName = Path.GetFileName(pdbTargetPath);
713718
if (!File.Exists(pdbTargetPath) || (new System.IO.FileInfo(pdbTargetPath).Length != entry.Length))
714719
{

‎src/PerfView/PerfView.csproj‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
<PackageReference Include="PerfView.SupportFiles" PrivateAssets="all" />
104104
<PackageReference Include="System.Buffers" GeneratePathProperty="true" />
105105
<PackageReference Include="System.Diagnostics.DiagnosticSource" GeneratePathProperty="true" />
106+
<PackageReference Include="System.IO.Hashing" GeneratePathProperty="true" />
106107
<PackageReference Include="System.Security.Cryptography.ProtectedData" GeneratePathProperty="true" />
107108
<PackageReference Include="System.Memory" GeneratePathProperty="true" />
108109
<PackageReference Include="System.Numerics.Vectors" GeneratePathProperty="true" />
@@ -179,9 +180,15 @@
179180
<Compile Include="..\Utilities\FileUtilities.cs">
180181
<Link>Utilities\FileUtilities.cs</Link>
181182
</Compile>
183+
<Compile Include="..\Utilities\PathUtilities.cs">
184+
<Link>Utilities\PathUtilities.cs</Link>
185+
</Compile>
182186
<Compile Include="..\Utilities\StringUtilities.cs">
183187
<Link>Utilities\StringUtilities.cs</Link>
184188
</Compile>
189+
<Compile Include="..\Utilities\SymbolCachePathUtilities.cs">
190+
<Link>Utilities\SymbolCachePathUtilities.cs</Link>
191+
</Compile>
185192
<Compile Include="..\Utilities\SymbolsAuthenticationUtilities.cs">
186193
<Link>Utilities\SymbolsAuthenticationUtilities.cs</Link>
187194
</Compile>
@@ -614,6 +621,13 @@
614621
<Link>System.Diagnostics.DiagnosticSource.dll</Link>
615622
<Visible>False</Visible>
616623
</EmbeddedResource>
624+
<EmbeddedResource Include="$(PkgSystem_IO_Hashing)\lib\net462\System.IO.Hashing.dll">
625+
<Type>Non-Resx</Type>
626+
<WithCulture>false</WithCulture>
627+
<LogicalName>.\System.IO.Hashing.dll</LogicalName>
628+
<Link>System.IO.Hashing.dll</Link>
629+
<Visible>False</Visible>
630+
</EmbeddedResource>
617631
<EmbeddedResource Include="$(PkgSystem_Memory)\lib\net462\System.Memory.dll">
618632
<Type>Non-Resx</Type>
619633
<WithCulture>false</WithCulture>

0 commit comments

Comments
 (0)