From 7d54d88b65a531cb51954a01303e5d2e281735ea Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Thu, 8 Oct 2026 09:04:40 +0000 Subject: [PATCH] Harden directory escape in SCP recursive download An E record received before any D record made the directory depth counter negative and moved the current directory to the parent of the destination, so that a following C record wrote outside it; each further E record climbed another level. On Windows, a D record named "...", ".. " or " .." (legal names on a Unix server) normalizes to the current directory, so a D and E pair climbed one level with a balanced counter. An unmatched E record is now rejected before it is acknowledged. Every C and D record name goes through ResolveChild, which requires the full path to be a direct child of the current directory with exactly the name that was sent. This also rejects Windows device names such as "CON", and names that Windows would silently change, such as a trailing dot, unless the destination uses the \\?\ prefix. Rejected names are reported to the server and thrown as ScpException. ResolveChild replaces EnsureValidLocalName. --- src/Renci.SshNet/ScpClient.cs | 165 ++++++++--- .../Renci.SshNet.IntegrationTests/ScpTests.cs | 55 ++++ .../ScpClientTest_Download_InvalidRecord.cs | 20 ++ ...est_Download_PathAndDirectoryInfo_Names.cs | 262 ++++++++++++++++++ .../ScpClientTest_EnsureValidLocalName.cs | 102 ------- ...cpClientTest_ResolveChild_RootDirectory.cs | 35 +++ 6 files changed, 498 insertions(+), 141 deletions(-) create mode 100644 test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_PathAndDirectoryInfo_Names.cs delete mode 100644 test/Renci.SshNet.Tests/Classes/ScpClientTest_EnsureValidLocalName.cs create mode 100644 test/Renci.SshNet.Tests/Classes/ScpClientTest_ResolveChild_RootDirectory.cs diff --git a/src/Renci.SshNet/ScpClient.cs b/src/Renci.SshNet/ScpClient.cs index 4afe4920..07422753 100644 --- a/src/Renci.SshNet/ScpClient.cs +++ b/src/Renci.SshNet/ScpClient.cs @@ -564,9 +564,27 @@ namespace Renci.SshNet /// Local directory information. /// is or empty. /// is . - /// File or directory with the specified path does not exist on the remote host, or the server sent an invalid or unexpected SCP record. + /// + /// File or directory with the specified path does not exist on the remote host, the server sent an invalid + /// or unexpected SCP record, or the server sent a file or directory name that is not a valid local name. + /// /// The secure copy execution request was rejected by the server. /// Client is not connected. + /// + /// + /// Each file and directory name sent by the server must be a plain name that refers to an entry + /// directly inside the directory being downloaded into. The download is aborted with an + /// if a name contains a directory separator or another character that is + /// not valid in a local file name, refers to the current or parent directory, or would be resolved by + /// the local file system to a different name or location, so that the names sent by the server cannot + /// refer to a location outside of . + /// + /// + /// Existing files in are overwritten by files with the same name, and + /// symbolic links and junctions that already exist within it are followed. If the download fails, the + /// files received so far, including part of the file being received, are left in place. + /// + /// public void Download(string directoryName, DirectoryInfo directoryInfo) { ArgumentException.ThrowIfNullOrEmpty(directoryName); @@ -946,30 +964,61 @@ namespace Renci.SshNet } /// - /// Ensures that a file or directory name received from the remote host in the SCP - /// protocol stream is a plain local name that cannot redirect a write outside of the - /// caller-supplied destination directory. + /// Combines a file or directory name received from the remote host with a local directory, + /// ensuring that the result is a direct child of that directory with exactly that name. /// + /// The local directory, as returned by . /// The file or directory name as sent by the server. - /// - /// is empty, refers to the current or parent directory, or - /// contains a character that is not valid in a local file name (such as a directory - /// separator). - /// - internal static void EnsureValidLocalName(string name) + /// + /// The full path of the child, or if is empty, refers + /// to the current or parent directory, contains a character that is not valid in a local file name + /// (such as a directory separator), or the local file system would resolve it to a different name or + /// location (for example, Windows removes trailing dots and spaces, so "..." refers to + /// itself). + /// + internal static string? ResolveChild(string directory, string name) { - // A legitimate SCP server only ever sends a single, plain path component in a - // C (file) or D (directory) record. Reject anything that is empty, refers to the - // current/parent directory, or carries a directory separator, drive qualifier or - // other character that is not valid in a local file name. Path.GetInvalidFileNameChars() - // is platform-aware: on Windows it includes '\', '/' and ':'; on Unix it includes '/'. - if (string.IsNullOrEmpty(name) || - name.Equals(".", StringComparison.Ordinal) || - name.Equals("..", StringComparison.Ordinal) || - name.IndexOfAny(InvalidLocalNameChars) >= 0) + if (string.IsNullOrEmpty(name) || name.IndexOfAny(InvalidLocalNameChars) >= 0) { - throw new ScpException($"The server sent a file or directory name (\"{name}\") that is not a valid local name."); + return null; } + + var path = Path.GetFullPath(Path.Combine(directory, name)); + + if (!string.Equals(Path.GetDirectoryName(path), directory, PathHelpers.StringComparison) || + !string.Equals(Path.GetFileName(path), name, PathHelpers.StringComparison)) + { + return null; + } + + return path; + } + + /// + /// Returns the full path of a local directory without a trailing directory separator, unless + /// the directory is a root. + /// + /// The path of the directory. + /// The normalized full path. + internal static string NormalizeDirectory(string path) + { + var fullPath = Path.GetFullPath(path); + var root = Path.GetPathRoot(fullPath); + + if (root is not null && fullPath.Length > root.Length) + { + fullPath = fullPath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + } + + return fullPath; + } + + [DoesNotReturn] + private void SendInvalidLocalNameErrorAndThrow(IChannel channel) + { + SendErrorConfirmation(channel, "File or directory name is not a valid local name."); + + throw new ScpException("The server sent a file or directory name that is not a valid local name."); } /// @@ -1044,8 +1093,8 @@ namespace Renci.SshNet var modifiedUtc = DateTime.UtcNow; var accessedUtc = modifiedUtc; - var startDirectoryFullName = directoryInfo.FullName; - var currentDirectoryFullName = startDirectoryFullName; + var destination = NormalizeDirectory(directoryInfo.FullName); + var currentDirectoryFullName = destination; var directoryCounter = 0; while (true) @@ -1054,6 +1103,11 @@ namespace Renci.SshNet if (message == "E") { + if (directoryCounter <= 0) + { + SendInvalidRecordErrorAndThrow(channel); + } + SendSuccessConfirmation(channel); // Send reply directoryCounter--; @@ -1065,7 +1119,7 @@ namespace Renci.SshNet var currentDirectoryParent = new DirectoryInfo(currentDirectoryFullName).Parent; - Debug.Assert(currentDirectoryParent is not null, $"Should be {directoryCounter.ToString(CultureInfo.InvariantCulture)} levels deeper than {startDirectoryFullName}."); + Debug.Assert(currentDirectoryParent is not null, $"Should be {directoryCounter.ToString(CultureInfo.InvariantCulture)} levels deeper than {destination}."); currentDirectoryFullName = currentDirectoryParent.FullName; @@ -1075,31 +1129,35 @@ namespace Renci.SshNet var match = DirectoryInfoRegex.Match(message); if (match.Success) { - SendSuccessConfirmation(channel); // Send reply - // Read directory var filename = match.Result("${filename}"); - DirectoryInfo newDirectoryInfo; if (directoryCounter > 0) { - // The server-supplied name is combined into a local path; ensure it - // cannot escape the destination directory. - EnsureValidLocalName(filename); + var path = ResolveChild(currentDirectoryFullName, filename); - newDirectoryInfo = Directory.CreateDirectory(Path.Combine(currentDirectoryFullName, filename)); + if (path is null) + { + SendInvalidLocalNameErrorAndThrow(channel); + } + + SendSuccessConfirmation(channel); // Send reply + + var newDirectoryInfo = Directory.CreateDirectory(path); newDirectoryInfo.LastAccessTimeUtc = accessedUtc; newDirectoryInfo.LastWriteTimeUtc = modifiedUtc; + + currentDirectoryFullName = path; } else { + SendSuccessConfirmation(channel); // Send reply + // Don't create directory for first level - newDirectoryInfo = directoryInfo; + currentDirectoryFullName = destination; } directoryCounter++; - - currentDirectoryFullName = newDirectoryInfo.FullName; continue; } @@ -1109,15 +1167,18 @@ namespace Renci.SshNet { var fileName = match.Result("${filename}"); + var path = ResolveChild(currentDirectoryFullName, fileName); + + if (path is null) + { + SendInvalidLocalNameErrorAndThrow(channel); + } + + var fileInfo = new FileInfo(path); + // Read file SendSuccessConfirmation(channel); // Send reply - // The server-supplied name is combined into a local path; ensure it - // cannot escape the destination directory. - EnsureValidLocalName(fileName); - - var fileInfo = new FileInfo(Path.Combine(currentDirectoryFullName, fileName)); - DownloadFileContent(channel, input, fileInfo, fileName, length, modifiedUtc, accessedUtc); if (directoryCounter == 0) @@ -1142,5 +1203,31 @@ namespace Renci.SshNet SendInvalidRecordErrorAndThrow(channel); } } + + // https://github.com/dotnet/runtime/blob/a1115d3990ce45403dd80d96e39c43066e9ece65/src/libraries/Common/src/System/IO/PathInternal.CaseSensitivity.cs + private static class PathHelpers + { + internal static StringComparison StringComparison + { + get + { + return IsCaseSensitive ? + StringComparison.Ordinal : + StringComparison.OrdinalIgnoreCase; + } + } + + internal static bool IsCaseSensitive + { + get + { +#if NET + return !(OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS()); +#else + return Path.DirectorySeparatorChar != '\\'; +#endif + } + } + } } } diff --git a/test/Renci.SshNet.IntegrationTests/ScpTests.cs b/test/Renci.SshNet.IntegrationTests/ScpTests.cs index 5e6704d5..e98dc013 100644 --- a/test/Renci.SshNet.IntegrationTests/ScpTests.cs +++ b/test/Renci.SshNet.IntegrationTests/ScpTests.cs @@ -2070,6 +2070,61 @@ namespace Renci.SshNet.IntegrationTests } } + /// + /// A recursive download must never write outside of the destination directory when the + /// remote tree contains a directory named "...". It is an ordinary name on the (Linux) + /// server, but Windows path normalization resolves it to the current directory. + /// + [TestMethod] + public void Scp_Download_DirectoryInfo_DirectoryNamedThreeDots_StaysInsideDestination() + { + var remoteDirectory = "/tmp/sshnet-scp-dots-" + Guid.NewGuid().ToString("N"); + + using (var client = new SftpClient(_connectionInfoFactory.Create())) + { + client.Connect(); + client.CreateDirectory(remoteDirectory); + client.CreateDirectory(remoteDirectory + "/..."); + client.WriteAllText(remoteDirectory + "/.../inner.txt", "inner"); + client.WriteAllText(remoteDirectory + "/outer.txt", "outer"); + } + + var localRoot = Path.Combine(Path.GetTempPath(), "sshnet-scp-dots-" + Guid.NewGuid().ToString("N")); + var destination = Directory.CreateDirectory(Path.Combine(localRoot, "download")); + + try + { + using (var client = new ScpClient(_connectionInfoFactory.Create(), _remotePathTransformation)) + { + client.Connect(); + + if (Path.DirectorySeparatorChar == '\\') + { + _ = Assert.ThrowsExactly(() => client.Download(remoteDirectory, destination)); + } + else + { + client.Download(remoteDirectory, destination); + Assert.AreEqual("inner", File.ReadAllText(Path.Combine(destination.FullName, "...", "inner.txt"))); + Assert.AreEqual("outer", File.ReadAllText(Path.Combine(destination.FullName, "outer.txt"))); + } + } + + // Nothing should be written outside of the destination directory on any platform. + CollectionAssert.AreEqual(new[] { destination.FullName }, Directory.GetFileSystemEntries(localRoot)); + } + finally + { + using (var client = new SshClient(_connectionInfoFactory.Create())) + { + client.Connect(); + _ = client.RunCommand("rm -rf '" + remoteDirectory + "'"); + } + + Directory.Delete(localRoot, recursive: true); + } + } + /// /// A file whose name contains the text of a D record must be downloaded as a file, with its /// content written verbatim rather than interpreted as SCP records. diff --git a/test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_InvalidRecord.cs b/test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_InvalidRecord.cs index dd348445..9573bd68 100644 --- a/test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_InvalidRecord.cs +++ b/test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_InvalidRecord.cs @@ -85,6 +85,26 @@ namespace Renci.SshNet.Tests.Classes Assert.IsEmpty(Directory.GetFileSystemEntries(Destination)); } + [TestMethod] + public void DownloadDirectoryInfo_EndDirectoryBeforeAnyDirectory_ShouldThrowScpException() + { + // Without the check, the E record would move the download to the parent of the destination, + // and the following file would overwrite the existing file there. + var existing = Path.Combine(DestinationParent, "existing.txt"); + File.WriteAllText(existing, "original"); + + var transcript = new ScpServerTranscript() + .Line("E") + .File("existing.txt", "data"); + + var result = DownloadDirectory(transcript); + + AssertRejected(result, expectedSuccessStatuses: 1); + Assert.AreEqual("original", File.ReadAllText(existing)); + AssertNothingOutsideDestination(expectedOutside: "a/b/existing.txt"); + Assert.IsEmpty(Directory.GetFileSystemEntries(Destination)); + } + [TestMethod] [DataRow("Xjunk")] [DataRow("C0644 99999999999999999999 f")] diff --git a/test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_PathAndDirectoryInfo_Names.cs b/test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_PathAndDirectoryInfo_Names.cs new file mode 100644 index 00000000..acbaeebb --- /dev/null +++ b/test/Renci.SshNet.Tests/Classes/ScpClientTest_Download_PathAndDirectoryInfo_Names.cs @@ -0,0 +1,262 @@ +using System; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Renci.SshNet.Common; +using Renci.SshNet.Tests.Common; + +namespace Renci.SshNet.Tests.Classes +{ + /// + /// Verifies how a recursive download handles the file and directory names sent by the server: + /// plain names are created as-is, and names that are not a plain local name, or that the local + /// file system would resolve to a different name or location, are rejected. + /// + /// + /// Each name is checked both in a C (file) record and in a nested D (directory) record; the name + /// in the first D record is ignored, as the destination itself is used for it. + /// + [TestClass] + public class ScpClientTest_Download_PathAndDirectoryInfo_Names : ScpClientTranscriptTestBase + { + [TestMethod] + [DataRow("file.txt")] + [DataRow(".hidden")] + [DataRow("файл.txt")] + [DataRow("~")] + public void PlainName_ShouldBeDownloaded(string name) + { + var result = DownloadDirectory(new ScpServerTranscript() + .Line("D0755 0 remote") + .Line("D0755 0 " + name) + .File(name, "content") + .Line("E") + .Line("E")); + + Assert.IsNull(result.Exception, result.Exception?.ToString()); + CollectionAssert.AreEquivalent(new[] { name, name + "/" + name }, ListEntries(Destination)); + } + + [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows, IgnoreMessage = "These are ordinary names on Unix-like platforms.")] + [DataRow("...")] + [DataRow(".. ")] + [DataRow("file.")] + [DataRow("file ")] + [DataRow("CON")] + [DataRow("PROGRA~1")] + [DataRow("name\\with\\backslashes")] + [DataRow("file.txt:stream")] + [DataRow("C:file.txt")] + [DataRow("\\\\?\\C:\\file.txt")] + [DataRow("\\\\.\\C:\\file.txt")] + [DataRow("\\??\\C:\\file.txt")] + public void OrdinaryUnixName_ShouldBeDownloaded(string name) + { + var result = DownloadDirectory(new ScpServerTranscript() + .Line("D0755 0 remote") + .Line("D0755 0 " + name) + .File(name, "content") + .Line("E") + .Line("E")); + + Assert.IsNull(result.Exception, result.Exception?.ToString()); + CollectionAssert.AreEquivalent(new[] { name, name + "/" + name }, ListEntries(Destination)); + } + + [TestMethod] + [OSCondition(OperatingSystems.Windows, IgnoreMessage = "The \\\\?\\ prefix only exists on Windows.")] + public void PlainName_LiteralPathDestination_OnWindows_ShouldBeDownloaded() + { + var result = Download("sub", isDirectory: true, LiteralPath(Destination)); + + Assert.IsNull(result.Exception, result.Exception?.ToString()); + CollectionAssert.AreEquivalent(new[] { "sub", "sub/inner.txt" }, ListEntries(Destination)); + } + + [TestMethod] + [DataRow(".")] + [DataRow("..")] + [DataRow("sub/child")] + [DataRow("../escaped")] + [DataRow("/etc/passwd")] + [DataRow("nul\0byte")] + public void InvalidName_ShouldThrowScpException(string name) + { + foreach (var isDirectory in new[] { false, true }) + { + var result = Download(name, isDirectory); + + AssertNothingOutsideDestination(context: name); + Assert.IsInstanceOfType(result.Exception, result.Exception?.ToString()); + Assert.Contains("not a valid local name", result.Exception.Message, StringComparison.Ordinal); + Assert.IsEmpty(Directory.GetFileSystemEntries(Destination)); + + // The record with the invalid name is reported to the server instead of being acknowledged: + // the only success statuses are for the exec request and the first D record. + Assert.AreEqual(2, result.SentData.Count(data => data.Length == 1 && data[0] == 0)); + Assert.IsTrue(result.SentData.Any(data => data.Length == 1 && data[0] == 1), "No error status was sent to the server."); + } + } + + [TestMethod] + [OSCondition(OperatingSystems.Windows, IgnoreMessage = "These names are only invalid, or changed by path normalization, on Windows.")] + [DataRow("..\\escaped")] + [DataRow("C:\\Windows\\file.txt")] + [DataRow("C:file.txt")] + [DataRow("file.txt:stream")] + [DataRow("\\\\?\\C:\\file.txt")] + [DataRow("\\\\.\\C:\\file.txt")] + [DataRow("\\??\\C:\\file.txt")] + [DataRow("\\\\server\\share\\file.txt")] + [DataRow("...")] // Path normalization removes trailing dots and spaces. + [DataRow(".. ")] + [DataRow(" ..")] + [DataRow(". ")] + [DataRow("file.")] + [DataRow("file ")] + public void InvalidName_OnWindows_ShouldThrowScpException(string name) + { + foreach (var isDirectory in new[] { false, true }) + { + var result = Download(name, isDirectory); + + AssertNothingOutsideDestination(context: name); + Assert.IsInstanceOfType(result.Exception, result.Exception?.ToString()); + Assert.Contains("not a valid local name", result.Exception.Message, StringComparison.Ordinal); + Assert.IsEmpty(Directory.GetFileSystemEntries(Destination)); + Assert.AreEqual(2, result.SentData.Count(data => data.Length == 1 && data[0] == 0)); + Assert.IsTrue(result.SentData.Any(data => data.Length == 1 && data[0] == 1), "No error status was sent to the server."); + } + } + + [TestMethod] + [OSCondition(OperatingSystems.Windows, IgnoreMessage = "The \\\\?\\ prefix only exists on Windows.")] + [DataRow(".")] + [DataRow("..")] + [DataRow("...")] + [DataRow("file.")] + [DataRow("file ")] + public void NameChangedByNormalization_LiteralPathDestination_OnWindows_ShouldStayWithinDestination(string name) + { + // Path.GetFullPath does not normalize a path with the \\?\ prefix, so these names reach the + // file system as they are: "." and ".." are refused with an IOException, and the others are + // created with exactly that name. Either way, nothing may be written outside the destination. + foreach (var isDirectory in new[] { false, true }) + { + var result = Download(name, isDirectory, LiteralPath(Destination)); + + AssertNothingOutsideDestination(context: name); + Assert.IsTrue(result.Exception is null or ScpException or IOException, result.Exception?.ToString()); + + ClearDestination(); + } + } + + [TestMethod] + [OSCondition(OperatingSystems.Windows, IgnoreMessage = "Device names are only reserved on Windows.")] + [DataRow("CON")] + [DataRow("con.txt")] + [DataRow("CON .txt")] + [DataRow("NUL")] + [DataRow("COM1")] + [DataRow("LPT1")] + public void ReservedDeviceName_OnWindows_ShouldBeRejectedOrWrittenToDestination(string name) + { + // Where Windows treats the name as a device in any directory, Path.GetFullPath resolves it to + // a device path (such as \\.\CON), which is not a child of the destination and is rejected. + // Where it does not (as in recent versions of Windows), it is an ordinary name. + foreach (var isDirectory in new[] { false, true }) + { + var result = Download(name, isDirectory); + + AssertNothingOutsideDestination(context: name); + + if (result.Exception is null) + { + // Read through a literal path, so that the name cannot be redirected to a device. + var file = isDirectory ? Path.Combine(Destination, name, "inner.txt") : Path.Combine(Destination, name); + Assert.AreEqual("content", File.ReadAllText(LiteralPath(file))); + } + else + { + Assert.IsInstanceOfType(result.Exception, result.Exception.ToString()); + Assert.Contains("not a valid local name", result.Exception.Message, StringComparison.Ordinal); + } + + ClearDestination(); + } + } + + [TestMethod] + [OSCondition(OperatingSystems.Windows, IgnoreMessage = "8.3 short names only exist on Windows.")] + public void ShortNameOfExistingDirectory_OnWindows_ShouldStayWithinDestination() + { + var existing = Directory.CreateDirectory(Path.Combine(Destination, "LongDirectoryName")); + + var buffer = new char[1024]; + var length = GetShortPathName(existing.FullName, buffer, (uint)buffer.Length); + var shortName = length is 0 or > 1024 ? existing.Name : Path.GetFileName(new string(buffer, 0, (int)length)); + + if (string.Equals(shortName, existing.Name, StringComparison.OrdinalIgnoreCase)) + { + Assert.Inconclusive("8.3 short names are not generated on this volume."); + } + + var result = Download(shortName, isDirectory: true); + + // .NET Framework's Path.GetFullPath expands a short name, so the resolved name differs from + // the name that was sent and is rejected. Elsewhere, the short name is used as-is, and the + // file system resolves it to the existing directory. Either way, the download must stay + // within the destination. + if (result.Exception is null) + { + Assert.AreEqual("content", File.ReadAllText(Path.Combine(existing.FullName, "inner.txt"))); + } + else + { + Assert.IsInstanceOfType(result.Exception); + Assert.Contains("not a valid local name", result.Exception.Message, StringComparison.Ordinal); + } + + AssertNothingOutsideDestination(); + } + + [TestMethod] + public void DestinationWithTrailingSeparator_ShouldBeDownloaded() + { + var result = Download("sub", isDirectory: true, Destination + Path.DirectorySeparatorChar); + + Assert.IsNull(result.Exception, result.Exception?.ToString()); + CollectionAssert.AreEquivalent(new[] { "sub", "sub/inner.txt" }, ListEntries(Destination)); + } + +#pragma warning disable SYSLIB1054 // Use LibraryImport; not available on .NET Framework, and only used by a Windows-only test. + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true, EntryPoint = "GetShortPathNameW")] + private static extern uint GetShortPathName(string longPath, [Out] char[] shortPath, uint bufferLength); +#pragma warning restore SYSLIB1054 + + /// + /// Downloads a remote directory containing either a file named , or a + /// directory named that contains a file named inner.txt. + /// + private ScpTranscriptResult Download(string name, bool isDirectory, string destinationPath = null) + { + var transcript = new ScpServerTranscript().Line("D0755 0 remote"); + + if (isDirectory) + { + _ = transcript.Line("D0755 0 " + name).File("inner.txt", "content").Line("E"); + } + else + { + _ = transcript.File(name, "content"); + } + + return DownloadDirectory(transcript.Line("E"), destinationPath); + } + } +} diff --git a/test/Renci.SshNet.Tests/Classes/ScpClientTest_EnsureValidLocalName.cs b/test/Renci.SshNet.Tests/Classes/ScpClientTest_EnsureValidLocalName.cs deleted file mode 100644 index 866ce823..00000000 --- a/test/Renci.SshNet.Tests/Classes/ScpClientTest_EnsureValidLocalName.cs +++ /dev/null @@ -1,102 +0,0 @@ -using Microsoft.VisualStudio.TestTools.UnitTesting; - -using Renci.SshNet.Common; - -namespace Renci.SshNet.Tests.Classes -{ - /// - /// Tests for , which guards the recursive - /// download against server-supplied SCP file/directory names that would write outside the - /// caller-supplied destination directory. - /// - [TestClass] - public class ScpClientTest_EnsureValidLocalName - { - [TestMethod] - public void PlainFileName_DoesNotThrow() - { - ScpClient.EnsureValidLocalName("owned.txt"); - ScpClient.EnsureValidLocalName("2024-report.tar.gz"); - ScpClient.EnsureValidLocalName("file with spaces.dat"); - ScpClient.EnsureValidLocalName("..."); - } - - [TestMethod] - public void UnicodeFileName_DoesNotThrow() - { - ScpClient.EnsureValidLocalName("файл.txt"); - } - - [TestMethod] - public void Empty_ThrowsScpException() - { - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName(string.Empty)); - } - - [TestMethod] - public void CurrentDirectory_ThrowsScpException() - { - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName(".")); - } - - [TestMethod] - public void ParentDirectory_ThrowsScpException() - { - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("..")); - } - - [TestMethod] - public void ForwardSlashPath_ThrowsScpException() - { - // '/' is an invalid file name character on every platform. - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("sub/child")); - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("../escaped/owned.txt")); - } - - [TestMethod] - public void RootedUnixPath_ThrowsScpException() - { - // Contains '/', so it is rejected regardless of platform. - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("/tmp/sshnet-owned.txt")); - } - - [TestMethod] - public void NullCharacter_ThrowsScpException() - { - // NUL is an invalid file name character on every platform. - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("safe\0evil")); - } - - [TestMethod] - [OSCondition(OperatingSystems.Windows, IgnoreMessage = "'\\' is only a path separator (and invalid file name char) on Windows.")] - public void BackslashPath_OnWindows_ThrowsScpException() - { - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("..\\escaped\\owned.txt")); - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("sub\\child")); - } - - [TestMethod] - [OSCondition(OperatingSystems.Windows, IgnoreMessage = "':' is only an invalid file name char on Windows.")] - public void DriveQualifiedPath_OnWindows_ThrowsScpException() - { - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("C:\\Windows\\System32\\evil.dll")); - } - - [TestMethod] - [OSCondition(OperatingSystems.Windows, IgnoreMessage = "':' is only an invalid file name char on Windows.")] - public void AlternateDataStreamName_OnWindows_ThrowsScpException() - { - // NTFS alternate data stream syntax: writes a hidden stream of "safe.txt". - _ = Assert.ThrowsExactly(() => ScpClient.EnsureValidLocalName("safe.txt:evil")); - } - - [TestMethod] - [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows, IgnoreMessage = "'\\' is a valid file name byte only on Unix-like platforms.")] - public void BackslashName_OnUnix_DoesNotThrow() - { - // On Unix, '\' is an ordinary file name byte and cannot traverse directories, - // so a name containing it must remain accepted (no behaviour change). - ScpClient.EnsureValidLocalName("name\\with\\backslashes.txt"); - } - } -} diff --git a/test/Renci.SshNet.Tests/Classes/ScpClientTest_ResolveChild_RootDirectory.cs b/test/Renci.SshNet.Tests/Classes/ScpClientTest_ResolveChild_RootDirectory.cs new file mode 100644 index 00000000..842f4cce --- /dev/null +++ b/test/Renci.SshNet.Tests/Classes/ScpClientTest_ResolveChild_RootDirectory.cs @@ -0,0 +1,35 @@ +using System.IO; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Renci.SshNet.Tests.Classes +{ + /// + /// Tests for and + /// with a root directory as the destination, + /// which a download test cannot use. Other cases are covered by + /// . + /// + [TestClass] + public class ScpClientTest_ResolveChild_RootDirectory + { + [TestMethod] + public void RootDirectory_ShouldReturnDirectChild() + { + var root = ScpClient.NormalizeDirectory(Path.GetPathRoot(Path.GetTempPath())); + + Assert.AreEqual(Path.Combine(root, "file.txt"), ScpClient.ResolveChild(root, "file.txt")); + } + + [TestMethod] + [DataRow(".")] + [DataRow("..")] + public void RootDirectory_DotOrDotDot_ShouldBeRejected(string name) + { + // "." and ".." resolve to the root itself, which has no parent directory. + var root = ScpClient.NormalizeDirectory(Path.GetPathRoot(Path.GetTempPath())); + + Assert.IsNull(ScpClient.ResolveChild(root, name)); + } + } +}