Skip to content

Commit e62391b

Browse files
peterwaldCopilotshyamnamboodiripad
authored
Add Path Validation for DiskBasedResponseCache and DiskBasedResultStore (#7397)
* Add Path Validation for DiskBasedResponseCache and DiskBasedResultStore * Refactor async test methods in PathValidationTests to use Task instead of void * Update error message 'value' to 'parameter' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix separator tests on Mac/Linux * Update src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Utilities/PathValidation.cs Co-authored-by: Shyam N <shyamnamboodiripad@users.noreply.github.com> * Update src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Utilities/PathValidation.cs Co-authored-by: Shyam N <shyamnamboodiripad@users.noreply.github.com> * Improve root path check. * Enhance EnsureWithinRoot method to handle directory separators for .NET Framework and add tests for UNC paths and edge cases * Fix net standard builds * Retrigger build --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shyam N <shyamnamboodiripad@users.noreply.github.com>
1 parent 99478fd commit e62391b

4 files changed

Lines changed: 620 additions & 8 deletions

File tree

src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Storage/DiskBasedResponseCache.cs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using System.Linq;
1414
using System.Threading;
1515
using System.Threading.Tasks;
16+
using Microsoft.Extensions.AI.Evaluation.Reporting.Utilities;
1617
using Microsoft.Extensions.Caching.Distributed;
1718

1819
namespace Microsoft.Extensions.AI.Evaluation.Reporting.Storage;
@@ -37,13 +38,18 @@ internal DiskBasedResponseCache(
3738
Func<DateTime> provideDateTime,
3839
TimeSpan? timeToLiveForCacheEntries = null)
3940
{
41+
PathValidation.ValidatePathSegment(scenarioName, nameof(scenarioName));
42+
PathValidation.ValidatePathSegment(iterationName, nameof(iterationName));
43+
4044
_scenarioName = scenarioName;
4145
_iterationName = iterationName;
4246

4347
storageRootPath = Path.GetFullPath(storageRootPath);
4448
string cacheRootPath = GetCacheRootPath(storageRootPath);
4549

46-
_iterationPath = Path.Combine(cacheRootPath, scenarioName, iterationName);
50+
_iterationPath = PathValidation.EnsureWithinRoot(
51+
cacheRootPath,
52+
Path.Combine(cacheRootPath, scenarioName, iterationName));
4753
_provideDateTime = provideDateTime;
4854
_timeToLiveForCacheEntries = timeToLiveForCacheEntries ?? Defaults.DefaultTimeToLiveForCacheEntries;
4955
}
@@ -289,7 +295,9 @@ private static string GetContentsFilePath(string keyPath)
289295

290296
private (string keyPath, string entryFilePath, string contentsFilePath, bool filesExist) GetPaths(string key)
291297
{
292-
string keyPath = Path.Combine(_iterationPath, key);
298+
PathValidation.ValidatePathSegment(key, nameof(key));
299+
300+
string keyPath = PathValidation.EnsureWithinRoot(_iterationPath, Path.Combine(_iterationPath, key));
293301
string entryFilePath = GetEntryFilePath(keyPath);
294302
string contentsFilePath = GetContentsFilePath(keyPath);
295303

@@ -316,4 +324,4 @@ private CacheEntry CreateEntry()
316324

317325
return new CacheEntry(_scenarioName, _iterationName, creation, expiration);
318326
}
319-
}
327+
}

src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Storage/DiskBasedResultStore.cs

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ public async IAsyncEnumerable<ScenarioRunResult> ReadResultsAsync(
5656
string? iterationName = null,
5757
[EnumeratorCancellation] CancellationToken cancellationToken = default)
5858
{
59+
PathValidation.ValidatePathSegment(executionName, nameof(executionName));
60+
PathValidation.ValidatePathSegment(scenarioName, nameof(scenarioName));
61+
PathValidation.ValidatePathSegment(iterationName, nameof(iterationName));
5962
IEnumerable<FileInfo> resultFiles =
6063
EnumerateResultFiles(executionName, scenarioName, iterationName, cancellationToken);
6164

@@ -89,8 +92,14 @@ public async ValueTask WriteResultsAsync(
8992
{
9093
cancellationToken.ThrowIfCancellationRequested();
9194

95+
PathValidation.ValidatePathSegment(result.ExecutionName, nameof(result.ExecutionName));
96+
PathValidation.ValidatePathSegment(result.ScenarioName, nameof(result.ScenarioName));
97+
PathValidation.ValidatePathSegment(result.IterationName, nameof(result.IterationName));
98+
9299
var resultDir =
93-
new DirectoryInfo(Path.Combine(_resultsRootPath, result.ExecutionName, result.ScenarioName));
100+
new DirectoryInfo(PathValidation.EnsureWithinRoot(
101+
_resultsRootPath,
102+
Path.Combine(_resultsRootPath, result.ExecutionName, result.ScenarioName)));
94103

95104
resultDir.Create();
96105

@@ -113,14 +122,21 @@ public ValueTask DeleteResultsAsync(
113122
string? iterationName = null,
114123
CancellationToken cancellationToken = default)
115124
{
125+
PathValidation.ValidatePathSegment(executionName, nameof(executionName));
126+
PathValidation.ValidatePathSegment(scenarioName, nameof(scenarioName));
127+
PathValidation.ValidatePathSegment(iterationName, nameof(iterationName));
128+
116129
if (executionName is null && scenarioName is null && iterationName is null)
117130
{
118131
Directory.Delete(_resultsRootPath, recursive: true);
119132
_ = Directory.CreateDirectory(_resultsRootPath);
120133
}
121134
else if (executionName is not null && scenarioName is null && iterationName is null)
122135
{
123-
var executionDir = new DirectoryInfo(Path.Combine(_resultsRootPath, executionName));
136+
var executionDir = new DirectoryInfo(
137+
PathValidation.EnsureWithinRoot(
138+
_resultsRootPath,
139+
Path.Combine(_resultsRootPath, executionName)));
124140

125141
if (executionDir.Exists)
126142
{
@@ -130,7 +146,10 @@ public ValueTask DeleteResultsAsync(
130146
else if (executionName is not null && scenarioName is not null && iterationName is null)
131147
{
132148
var scenarioDir =
133-
new DirectoryInfo(Path.Combine(_resultsRootPath, executionName, scenarioName));
149+
new DirectoryInfo(
150+
PathValidation.EnsureWithinRoot(
151+
_resultsRootPath,
152+
Path.Combine(_resultsRootPath, executionName, scenarioName)));
134153

135154
if (scenarioDir.Exists)
136155
{
@@ -140,7 +159,10 @@ public ValueTask DeleteResultsAsync(
140159
else if (executionName is not null && scenarioName is not null && iterationName is not null)
141160
{
142161
var resultFile =
143-
new FileInfo(Path.Combine(_resultsRootPath, executionName, scenarioName, $"{iterationName}.json"));
162+
new FileInfo(
163+
PathValidation.EnsureWithinRoot(
164+
_resultsRootPath,
165+
Path.Combine(_resultsRootPath, executionName, scenarioName, $"{iterationName}.json")));
144166

145167
if (resultFile.Exists)
146168
{
@@ -206,6 +228,8 @@ public async IAsyncEnumerable<string> GetScenarioNamesAsync(
206228
string executionName,
207229
[EnumeratorCancellation] CancellationToken cancellationToken = default)
208230
{
231+
PathValidation.ValidatePathSegment(executionName, nameof(executionName));
232+
209233
IEnumerable<DirectoryInfo> executionDirs = EnumerateExecutionDirs(executionName, cancellationToken);
210234

211235
IEnumerable<DirectoryInfo> scenarioDirs =
@@ -225,6 +249,9 @@ public async IAsyncEnumerable<string> GetIterationNamesAsync(
225249
string scenarioName,
226250
[EnumeratorCancellation] CancellationToken cancellationToken = default)
227251
{
252+
PathValidation.ValidatePathSegment(executionName, nameof(executionName));
253+
PathValidation.ValidatePathSegment(scenarioName, nameof(scenarioName));
254+
228255
IEnumerable<FileInfo> resultFiles =
229256
EnumerateResultFiles(executionName, scenarioName, cancellationToken: cancellationToken);
230257

@@ -260,7 +287,10 @@ private IEnumerable<DirectoryInfo> EnumerateExecutionDirs(
260287
}
261288
else
262289
{
263-
var executionDir = new DirectoryInfo(Path.Combine(_resultsRootPath, executionName));
290+
var executionDir = new DirectoryInfo(
291+
PathValidation.EnsureWithinRoot(
292+
_resultsRootPath,
293+
Path.Combine(_resultsRootPath, executionName)));
264294
if (executionDir.Exists)
265295
{
266296
yield return executionDir;
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.IO;
6+
using Microsoft.Shared.Diagnostics;
7+
8+
#if NET
9+
using System.Runtime.InteropServices;
10+
#endif
11+
12+
namespace Microsoft.Extensions.AI.Evaluation.Reporting.Utilities;
13+
14+
internal static class PathValidation
15+
{
16+
private static readonly char[] _invalidFileNameChars = Path.GetInvalidFileNameChars();
17+
18+
#pragma warning disable CA1802 // Use literals where appropriate
19+
private static readonly StringComparison _pathComparison =
20+
#if NET
21+
// Windows paths are case-insensitive; Linux/macOS paths are case-sensitive.
22+
RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
23+
? StringComparison.OrdinalIgnoreCase
24+
: StringComparison.Ordinal;
25+
#else
26+
StringComparison.OrdinalIgnoreCase; // .NET Framework and .NET Standard only run on Windows
27+
#endif
28+
#pragma warning restore CA1802 // Use literals where appropriate
29+
30+
/// <summary>
31+
/// Validates that a path segment is a safe single directory or file name.
32+
/// Throws <see cref="ArgumentException"/> if the segment contains path separators,
33+
/// invalid file name characters, or directory traversal sequences.
34+
/// </summary>
35+
internal static void ValidatePathSegment(string? segment, string paramName)
36+
{
37+
if (segment is null)
38+
{
39+
return;
40+
}
41+
42+
if (segment.Length == 0
43+
|| segment != segment.Trim()
44+
|| segment is "."
45+
|| segment is ".."
46+
|| segment.IndexOfAny(_invalidFileNameChars) >= 0)
47+
{
48+
Throw.ArgumentException(
49+
paramName,
50+
$"The parameter '{paramName}' contains invalid path characters or directory traversal sequences.");
51+
}
52+
}
53+
54+
/// <summary>
55+
/// Verifies that a fully resolved path is contained within the specified root directory.
56+
/// Both paths are canonicalized via <see cref="Path.GetFullPath(string)"/> before comparison.
57+
/// Throws <see cref="InvalidOperationException"/> if the resolved path escapes the root.
58+
/// </summary>
59+
internal static string EnsureWithinRoot(string rootPath, string resolvedPath)
60+
{
61+
string fullRoot = Path.GetFullPath(rootPath);
62+
string normalizedRoot = fullRoot;
63+
string fullResolved = Path.GetFullPath(resolvedPath);
64+
65+
// Ensure the root ends with a directory separator so that a root of
66+
// "/foo/bar" does not match a path like "/foo/bar-sibling/file".
67+
#if NET
68+
if (!normalizedRoot.EndsWith(Path.DirectorySeparatorChar) &&
69+
!normalizedRoot.EndsWith(Path.AltDirectorySeparatorChar))
70+
#else
71+
if (!normalizedRoot.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal) &&
72+
!normalizedRoot.EndsWith(Path.AltDirectorySeparatorChar.ToString(), StringComparison.Ordinal))
73+
#endif
74+
{
75+
normalizedRoot += Path.DirectorySeparatorChar;
76+
}
77+
78+
if (!fullResolved.StartsWith(normalizedRoot, _pathComparison) &&
79+
!string.Equals(fullRoot, fullResolved, _pathComparison))
80+
{
81+
throw new InvalidOperationException(
82+
"The resolved path escapes the configured root directory. " +
83+
"This may indicate a path traversal attempt.");
84+
}
85+
86+
return fullResolved;
87+
}
88+
}

0 commit comments

Comments
 (0)