Skip to content

Commit e99c633

Browse files
.Net: Harden file path validation in Core, Document, and Web plugins (#14118)
## Motivation and Context This change hardens file system path handling in the file-based plugins. Previously, the path used for allowlist validation could differ from the path ultimately used for file I/O, so the two operations were not guaranteed to refer to the same file system location. ## Description - Adds a shared `PathUtilities.GetSafeFullPath` helper that produces a canonical full path for a given input. - Updates `FileIOPlugin`, `SessionsPythonPlugin`, `DocumentPlugin` and `WebFileDownloadPlugin` to validate and perform I/O against the same canonical path, and to compare paths using OS-appropriate string comparison. - Adds regression tests covering the new behavior. ## Contribution Checklist - [x] The code builds clean without any errors or warnings - [x] The PR follows the SK Contribution Guidelines - [x] All unit tests pass, and I have added new tests where possible --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 99d1953 commit e99c633

11 files changed

Lines changed: 865 additions & 47 deletions

File tree

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
3+
using System;
4+
using System.Diagnostics.CodeAnalysis;
5+
using System.IO;
6+
using System.Runtime.InteropServices;
7+
8+
namespace Microsoft.SemanticKernel;
9+
10+
/// <summary>
11+
/// Provides utility methods for secure path resolution, including symbolic link handling.
12+
/// </summary>
13+
[ExcludeFromCodeCoverage]
14+
internal static class PathUtilities
15+
{
16+
internal static StringComparison PathComparison { get; } =
17+
RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
18+
? StringComparison.OrdinalIgnoreCase
19+
: StringComparison.Ordinal;
20+
21+
/// <summary>
22+
/// Returns the canonical full path for <paramref name="path"/> with symbolic links handled safely.
23+
/// On .NET 6 and later, symbolic links are resolved to their final target. On older frameworks,
24+
/// where links cannot be resolved safely, any path containing a symbolic link is rejected.
25+
/// </summary>
26+
/// <param name="path">The path to canonicalize.</param>
27+
/// <returns>The fully resolved canonical path.</returns>
28+
/// <exception cref="InvalidOperationException">
29+
/// On frameworks older than .NET 6, thrown if the path contains a symbolic link.
30+
/// </exception>
31+
internal static string GetSafeFullPath(string path)
32+
{
33+
var fullPath = Path.GetFullPath(path);
34+
35+
#if NET6_0_OR_GREATER
36+
return ResolveExistingSegments(fullPath);
37+
#else
38+
ThrowIfPathContainsReparsePoint(fullPath);
39+
return fullPath;
40+
#endif
41+
}
42+
43+
#if NET6_0_OR_GREATER
44+
private static string ResolveExistingSegments(string fullPath)
45+
{
46+
var root = Path.GetPathRoot(fullPath) ?? string.Empty;
47+
return ResolveSegments(root, GetSegmentsAfterRoot(fullPath, root));
48+
}
49+
50+
private static string ResolveSegments(string basePath, string[] segments)
51+
{
52+
var resolvedPath = basePath;
53+
for (var i = 0; i < segments.Length; i++)
54+
{
55+
var candidatePath = CombinePath(resolvedPath, segments[i]);
56+
57+
if (!TryGetFileSystemInfo(candidatePath, out var fileSystemInfo))
58+
{
59+
return CombineRemainingPath(candidatePath, segments, i + 1);
60+
}
61+
62+
var linkTarget = ResolveLinkTarget(fileSystemInfo);
63+
if (linkTarget is not null)
64+
{
65+
// The link target may itself contain unresolved symbolic links,
66+
// so canonicalize it before continuing with the remaining segments.
67+
resolvedPath = ResolveExistingSegments(linkTarget);
68+
}
69+
else
70+
{
71+
resolvedPath = candidatePath;
72+
}
73+
}
74+
75+
return resolvedPath;
76+
}
77+
78+
private static bool TryGetFileSystemInfo(string path, [NotNullWhen(true)] out FileSystemInfo? fileSystemInfo)
79+
{
80+
if (!TryGetAttributes(path, out var attributes))
81+
{
82+
fileSystemInfo = null;
83+
return false;
84+
}
85+
86+
fileSystemInfo = (attributes & FileAttributes.Directory) != 0
87+
? new DirectoryInfo(path)
88+
: new FileInfo(path);
89+
return true;
90+
}
91+
92+
private static string? ResolveLinkTarget(FileSystemInfo fileSystemInfo)
93+
{
94+
if ((fileSystemInfo.Attributes & FileAttributes.ReparsePoint) == 0)
95+
{
96+
return null;
97+
}
98+
99+
try
100+
{
101+
var target = fileSystemInfo.ResolveLinkTarget(returnFinalTarget: true);
102+
return target?.FullName
103+
?? throw new InvalidOperationException(
104+
$"Access denied: path contains a symbolic link that cannot be resolved at '{fileSystemInfo.FullName}'.");
105+
}
106+
catch (IOException)
107+
{
108+
throw new InvalidOperationException(
109+
$"Access denied: path contains a symbolic link that cannot be resolved at '{fileSystemInfo.FullName}'.");
110+
}
111+
}
112+
#endif
113+
114+
#if !NET6_0_OR_GREATER
115+
private static void ThrowIfPathContainsReparsePoint(string fullPath)
116+
{
117+
var root = Path.GetPathRoot(fullPath) ?? string.Empty;
118+
var segments = GetSegmentsAfterRoot(fullPath, root);
119+
120+
var currentPath = root;
121+
foreach (var segment in segments)
122+
{
123+
currentPath = CombinePath(currentPath, segment);
124+
125+
if (TryGetAttributes(currentPath, out var attributes) &&
126+
(attributes & FileAttributes.ReparsePoint) != 0)
127+
{
128+
throw new InvalidOperationException(
129+
$"Access denied: path contains a symbolic link at '{currentPath}'.");
130+
}
131+
}
132+
}
133+
#endif
134+
135+
private static string[] GetSegmentsAfterRoot(string fullPath, string root)
136+
{
137+
return fullPath.Substring(root.Length)
138+
.Split(new[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar },
139+
StringSplitOptions.RemoveEmptyEntries);
140+
}
141+
142+
private static bool TryGetAttributes(string path, out FileAttributes attributes)
143+
{
144+
try
145+
{
146+
attributes = File.GetAttributes(path);
147+
return true;
148+
}
149+
catch (FileNotFoundException)
150+
{
151+
}
152+
catch (DirectoryNotFoundException)
153+
{
154+
}
155+
156+
attributes = default;
157+
return false;
158+
}
159+
160+
private static string CombinePath(string basePath, string segment)
161+
{
162+
return string.IsNullOrEmpty(basePath) ? segment : Path.Combine(basePath, segment);
163+
}
164+
165+
private static string CombineRemainingPath(string basePath, string[] segments, int startIndex)
166+
{
167+
var result = basePath;
168+
for (var i = startIndex; i < segments.Length; i++)
169+
{
170+
result = Path.Combine(result, segments[i]);
171+
}
172+
173+
return result;
174+
}
175+
}

‎dotnet/src/Plugins/Plugins.Core/CodeInterpreter/SessionsPythonPlugin.cs‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -308,18 +308,18 @@ private string ValidateLocalPathForUpload(string localFilePath)
308308
"File upload requires 'AllowedUploadDirectories' to be configured.");
309309
}
310310

311-
var canonicalPath = Path.GetFullPath(localFilePath);
311+
var canonicalPath = PathUtilities.GetSafeFullPath(localFilePath);
312312

313313
foreach (var allowedDir in this._settings.AllowedUploadDirectories)
314314
{
315-
var canonicalAllowedDir = Path.GetFullPath(allowedDir);
315+
var canonicalAllowedDir = PathUtilities.GetSafeFullPath(allowedDir);
316316
// Ensure we match the directory correctly by appending separator
317317
var separator = Path.DirectorySeparatorChar.ToString();
318-
var allowedDirWithSeparator = canonicalAllowedDir.EndsWith(separator, StringComparison.OrdinalIgnoreCase)
318+
var allowedDirWithSeparator = canonicalAllowedDir.EndsWith(separator, PathUtilities.PathComparison)
319319
? canonicalAllowedDir
320320
: canonicalAllowedDir + separator;
321321

322-
if (canonicalPath.StartsWith(allowedDirWithSeparator, StringComparison.OrdinalIgnoreCase))
322+
if (canonicalPath.StartsWith(allowedDirWithSeparator, PathUtilities.PathComparison))
323323
{
324324
return canonicalPath;
325325
}
@@ -340,29 +340,31 @@ private string ValidateLocalPathForDownload(string localFilePath)
340340
// If no restrictions configured, allow all paths (permissive by default for downloads)
341341
if (this._settings.AllowedDownloadDirectories is null || !this._settings.AllowedDownloadDirectories.Any())
342342
{
343-
return Path.GetFullPath(localFilePath);
343+
return PathUtilities.GetSafeFullPath(localFilePath);
344344
}
345345

346-
// Get the directory of the target file path
347-
var targetDirectory = Path.GetDirectoryName(localFilePath);
346+
var canonicalFilePath = PathUtilities.GetSafeFullPath(localFilePath);
347+
348+
// Get the directory of the resolved target file path.
349+
var targetDirectory = Path.GetDirectoryName(canonicalFilePath);
348350
if (string.IsNullOrEmpty(targetDirectory))
349351
{
350352
targetDirectory = ".";
351353
}
352354

353-
var canonicalTargetDir = Path.GetFullPath(targetDirectory);
354-
var canonicalFilePath = Path.GetFullPath(localFilePath);
355+
var canonicalTargetDir = PathUtilities.GetSafeFullPath(targetDirectory);
355356

356357
foreach (var allowedDir in this._settings.AllowedDownloadDirectories)
357358
{
358-
var canonicalAllowedDir = Path.GetFullPath(allowedDir);
359+
var canonicalAllowedDir = PathUtilities.GetSafeFullPath(allowedDir);
359360
// Ensure we match the directory correctly by appending separator
360361
var separator = Path.DirectorySeparatorChar.ToString();
361-
var allowedDirWithSeparator = canonicalAllowedDir.EndsWith(separator, StringComparison.OrdinalIgnoreCase)
362+
var allowedDirWithSeparator = canonicalAllowedDir.EndsWith(separator, PathUtilities.PathComparison)
362363
? canonicalAllowedDir
363364
: canonicalAllowedDir + separator;
364365

365-
if (canonicalTargetDir.StartsWith(allowedDirWithSeparator, StringComparison.OrdinalIgnoreCase))
366+
if (canonicalTargetDir.StartsWith(allowedDirWithSeparator, PathUtilities.PathComparison)
367+
|| (canonicalTargetDir + separator).Equals(allowedDirWithSeparator, PathUtilities.PathComparison))
366368
{
367369
return canonicalFilePath;
368370
}

‎dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ public IEnumerable<string>? AllowedFolders
5757
[KernelFunction, Description("Read a file")]
5858
public async Task<string> ReadAsync([Description("Source file")] string path)
5959
{
60-
if (!this.IsFilePathAllowed(path))
60+
if (!this.TryGetAllowedFilePath(path, out var canonicalPath))
6161
{
6262
throw new InvalidOperationException("Reading from the provided location is not allowed.");
6363
}
6464

65-
using var reader = File.OpenText(path);
65+
using var reader = File.OpenText(canonicalPath);
6666
return await reader.ReadToEndAsync().ConfigureAwait(false);
6767
}
6868

@@ -80,19 +80,19 @@ public async Task WriteAsync(
8080
[Description("Destination file")] string path,
8181
[Description("File content")] string content)
8282
{
83-
if (!this.IsFilePathAllowed(path))
83+
if (!this.TryGetAllowedFilePath(path, out var canonicalPath))
8484
{
8585
throw new InvalidOperationException("Writing to the provided location is not allowed.");
8686
}
8787

88-
if (this.DisableFileOverwrite && File.Exists(path))
88+
if (this.DisableFileOverwrite && File.Exists(canonicalPath))
8989
{
9090
throw new InvalidOperationException("Overwriting existing files is disabled.");
9191
}
9292

9393
byte[] text = Encoding.UTF8.GetBytes(content);
9494
var fileMode = this.DisableFileOverwrite ? FileMode.CreateNew : FileMode.Create;
95-
using var writer = new FileStream(path, fileMode, FileAccess.Write, FileShare.None);
95+
using var writer = new FileStream(canonicalPath, fileMode, FileAccess.Write, FileShare.None);
9696
await writer.WriteAsync(text
9797
#if !NET
9898
, 0, text.Length
@@ -108,9 +108,10 @@ await writer.WriteAsync(text
108108
/// to verify it is in the allowed folder list. Paths are canonicalized before comparison.
109109
/// Subdirectories of allowed folders are also permitted.
110110
/// </summary>
111-
private bool IsFilePathAllowed(string path)
111+
private bool TryGetAllowedFilePath(string path, out string canonicalPath)
112112
{
113113
Verify.NotNullOrWhiteSpace(path);
114+
canonicalPath = string.Empty;
114115

115116
if (path.StartsWith("\\\\", StringComparison.OrdinalIgnoreCase))
116117
{
@@ -124,30 +125,36 @@ private bool IsFilePathAllowed(string path)
124125
throw new ArgumentException("Invalid file path, a fully qualified file location must be specified.", nameof(path));
125126
}
126127

127-
if (File.Exists(path) && File.GetAttributes(path).HasFlag(FileAttributes.ReadOnly))
128+
canonicalPath = PathUtilities.GetSafeFullPath(path);
129+
130+
if (File.Exists(canonicalPath) && File.GetAttributes(canonicalPath).HasFlag(FileAttributes.ReadOnly))
128131
{
129132
// Most environments will throw this with OpenWrite, but running inside docker on Linux will not.
130-
throw new UnauthorizedAccessException($"File is read-only: {path}");
133+
throw new UnauthorizedAccessException($"File is read-only: {canonicalPath}");
131134
}
132135

133136
if (this._allowedFolders is null || this._allowedFolders.Count == 0)
134137
{
135138
return false;
136139
}
137140

138-
var canonicalDir = Path.GetFullPath(directoryPath);
141+
var canonicalDir = Path.GetDirectoryName(canonicalPath);
142+
if (string.IsNullOrEmpty(canonicalDir))
143+
{
144+
throw new ArgumentException("Invalid file path, a fully qualified file location must be specified.", nameof(path));
145+
}
139146

140147
foreach (var allowedFolder in this._allowedFolders)
141148
{
142-
var canonicalAllowed = Path.GetFullPath(allowedFolder);
149+
var canonicalAllowed = PathUtilities.GetSafeFullPath(allowedFolder);
143150
var separator = Path.DirectorySeparatorChar.ToString();
144-
if (!canonicalAllowed.EndsWith(separator, StringComparison.OrdinalIgnoreCase))
151+
if (!canonicalAllowed.EndsWith(separator, PathUtilities.PathComparison))
145152
{
146153
canonicalAllowed += separator;
147154
}
148155

149-
if (canonicalDir.StartsWith(canonicalAllowed, StringComparison.OrdinalIgnoreCase)
150-
|| (canonicalDir + separator).Equals(canonicalAllowed, StringComparison.OrdinalIgnoreCase))
156+
if (canonicalDir.StartsWith(canonicalAllowed, PathUtilities.PathComparison)
157+
|| (canonicalDir + separator).Equals(canonicalAllowed, PathUtilities.PathComparison))
151158
{
152159
return true;
153160
}

‎dotnet/src/Plugins/Plugins.Document/DocumentPlugin.cs‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Collections.Generic;
55
using System.ComponentModel;
66
using System.IO;
7-
using System.Runtime.InteropServices;
87
using System.Threading;
98
using System.Threading.Tasks;
109
using Microsoft.Extensions.Logging;
@@ -146,7 +145,7 @@ private static string CanonicalizePath(string path)
146145
{
147146
Verify.NotNullOrWhiteSpace(path);
148147

149-
if (path.StartsWith("\\\\", StringComparison.OrdinalIgnoreCase))
148+
if (IsUncOrExtendedPath(path))
150149
{
151150
throw new ArgumentException("Invalid file path, UNC paths are not supported.", nameof(path));
152151
}
@@ -157,19 +156,19 @@ private static string CanonicalizePath(string path)
157156

158157
// Re-check after expansion: an env var could have expanded to a UNC
159158
// or extended-path prefix (e.g., %NETSHARE% → \\server\share).
160-
if (expanded.StartsWith("\\\\", StringComparison.OrdinalIgnoreCase))
159+
if (IsUncOrExtendedPath(expanded))
161160
{
162161
throw new ArgumentException("Invalid file path, UNC paths are not supported.", nameof(path));
163162
}
164163

165-
return Path.GetFullPath(expanded);
164+
return PathUtilities.GetSafeFullPath(expanded);
166165
}
167166

168-
// Use case-insensitive comparison on Windows (case-insensitive FS), case-sensitive on Linux/macOS.
169-
private static readonly StringComparison s_pathComparison =
170-
RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
171-
? StringComparison.OrdinalIgnoreCase
172-
: StringComparison.Ordinal;
167+
private static bool IsUncOrExtendedPath(string path)
168+
{
169+
return path.StartsWith("\\\\", StringComparison.OrdinalIgnoreCase) ||
170+
path.StartsWith("//", StringComparison.OrdinalIgnoreCase);
171+
}
173172

174173
/// <summary>
175174
/// Checks whether a canonicalized file path falls within one of the allowed directories.
@@ -191,15 +190,15 @@ private bool IsFilePathAllowed(string canonicalPath)
191190

192191
foreach (var allowedDirectory in this._allowedDirectories)
193192
{
194-
var canonicalAllowed = Path.GetFullPath(allowedDirectory);
193+
var canonicalAllowed = PathUtilities.GetSafeFullPath(allowedDirectory);
195194
var separator = Path.DirectorySeparatorChar.ToString();
196-
if (!canonicalAllowed.EndsWith(separator, s_pathComparison))
195+
if (!canonicalAllowed.EndsWith(separator, PathUtilities.PathComparison))
197196
{
198197
canonicalAllowed += separator;
199198
}
200199

201-
if (directoryPath.StartsWith(canonicalAllowed, s_pathComparison)
202-
|| (directoryPath + separator).Equals(canonicalAllowed, s_pathComparison))
200+
if (directoryPath.StartsWith(canonicalAllowed, PathUtilities.PathComparison)
201+
|| (directoryPath + separator).Equals(canonicalAllowed, PathUtilities.PathComparison))
203202
{
204203
return true;
205204
}

0 commit comments

Comments
 (0)