diff --git a/dotnet/Directory.Packages.props b/dotnet/Directory.Packages.props index f4a51a045a48..534116db67a1 100644 --- a/dotnet/Directory.Packages.props +++ b/dotnet/Directory.Packages.props @@ -13,7 +13,7 @@ - + diff --git a/dotnet/src/Plugins/Plugins.Core/CodeInterpreter/SessionsPythonPlugin.cs b/dotnet/src/Plugins/Plugins.Core/CodeInterpreter/SessionsPythonPlugin.cs index 9433de2a86bb..681298cd0dc7 100644 --- a/dotnet/src/Plugins/Plugins.Core/CodeInterpreter/SessionsPythonPlugin.cs +++ b/dotnet/src/Plugins/Plugins.Core/CodeInterpreter/SessionsPythonPlugin.cs @@ -111,7 +111,7 @@ public async Task ExecuteCodeAsync( /// /// Uploads a file to the `/mnt/data` directory of the current session. /// - /// The name of the remote file, relative to `/mnt/data`. + /// The name of the remote file, relative to `/mnt/data`. Must not be rooted or contain `.` or `..` path segments. /// The path to the file on the local machine. /// The cancellation token. /// The metadata of the uploaded file. @@ -120,13 +120,14 @@ public async Task ExecuteCodeAsync( /// Thrown when file operations are disabled or the path is not allowed. [KernelFunction, Description("Uploads a file to the `/mnt/data` directory of the current session.")] public async Task UploadFileAsync( - [Description("The name of the remote file, relative to `/mnt/data`.")] string remoteFileName, + [Description("The name of the remote file, relative to `/mnt/data`. Must not be rooted or contain `.` or `..` path segments.")] string remoteFileName, [Description("The path to the file on the local machine.")] string localFilePath, CancellationToken cancellationToken = default) { Verify.NotNullOrWhiteSpace(remoteFileName, nameof(remoteFileName)); Verify.NotNullOrWhiteSpace(localFilePath, nameof(localFilePath)); + ValidateRemoteFileName(remoteFileName); var validatedLocalPath = this.ValidateLocalPathForUpload(localFilePath); this._logger.LogInformation("Uploading file: {LocalFilePath} to {RemoteFileName}", validatedLocalPath, remoteFileName); @@ -292,6 +293,21 @@ private async Task SendAsync(HttpClient httpClient, HttpMet return await httpClient.SendWithSuccessCheckAsync(request, cancellationToken).ConfigureAwait(false); } + private static void ValidateRemoteFileName(string remoteFileName) + { + var hasDrivePrefix = remoteFileName.Length >= 2 + && remoteFileName[1] == ':' + && char.IsLetter(remoteFileName[0]); + + if (remoteFileName[0] is '/' or '\\' + || hasDrivePrefix + || remoteFileName.Split('/', '\\').Any(segment => segment is "." or "..")) + { + throw new InvalidOperationException( + "The remote file name must be relative and must not contain '.' or '..' path segments."); + } + } + /// /// Validates that the local file path is within allowed upload directories. /// diff --git a/dotnet/src/Plugins/Plugins.UnitTests/Core/SessionsPythonPluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/Core/SessionsPythonPluginTests.cs index 7899f6fb9c2d..ce262555f183 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/Core/SessionsPythonPluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/Core/SessionsPythonPluginTests.cs @@ -278,6 +278,61 @@ public async Task ItShouldUploadFileAsync() Assert.Equal(this._messageHandlerStub.FirstMultipartContent, requestPayload); } + [Theory] + [InlineData(".")] + [InlineData("..")] + [InlineData("../test-file.txt")] + [InlineData("../../test-file.txt")] + [InlineData("./test-file.txt")] + [InlineData("nested/./test-file.txt")] + [InlineData("nested/../test-file.txt")] + [InlineData("nested/..")] + [InlineData("/mnt/data/../../test-file.txt")] + [InlineData(@"nested\.\test-file.txt")] + [InlineData(@"nested\..\test-file.txt")] + [InlineData(@"\test-file.txt")] + [InlineData(@"\\server\share\test-file.txt")] + [InlineData(@"C:\test-file.txt")] + [InlineData("C:test-file.txt")] + [InlineData("C:/test-file.txt")] + [InlineData("/test-file.txt")] + public async Task ItShouldRejectUnsafeUploadRemoteFileNamesAsync(string remoteFileName) + { + // Arrange + var plugin = new SessionsPythonPlugin(this._settingsWithFileOperationsEnabled, this._httpClientFactory); + + // Act + var exception = await Assert.ThrowsAsync( + () => plugin.UploadFileAsync(remoteFileName, FileTestDataFilePath)); + + // Assert + Assert.Contains("must not contain '.' or '..' path segments", exception.Message); + Assert.Null(this._messageHandlerStub.RequestUri); + } + + [Theory] + [InlineData("nested/dir/test-file.txt")] + [InlineData(@"nested\dir\test-file.txt")] + [InlineData(".hidden/test-file.txt")] + [InlineData("nested/a..b/test-file.txt")] + public async Task ItShouldAllowSafeUploadRemoteFileNamesAsync(string remoteFileName) + { + // Arrange + var responseContent = await File.ReadAllTextAsync(UpdaloadFileTestDataFilePath); + this._messageHandlerStub.ResponseToReturn = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(responseContent), + }; + + var plugin = new SessionsPythonPlugin(this._settingsWithFileOperationsEnabled, this._httpClientFactory); + + // Act + await plugin.UploadFileAsync(remoteFileName, FileTestDataFilePath); + + // Assert + Assert.NotNull(this._messageHandlerStub.RequestUri); + } + [Fact] public async Task ItShouldDownloadFileWithoutSavingInDiskAsync() {