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

@@ -564,9 +564,27 @@ namespace Renci.SshNet
         /// <param name="directoryInfo">Local directory information.</param>
         /// <exception cref="ArgumentException"><paramref name="directoryName"/> is <see langword="null"/> or empty.</exception>
         /// <exception cref="ArgumentNullException"><paramref name="directoryInfo"/> is <see langword="null"/>.</exception>
-        /// <exception cref="ScpException">File or directory with the specified path does not exist on the remote host, or the server sent an invalid or unexpected SCP record.</exception>
+        /// <exception cref="ScpException">
+        /// 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.
+        /// </exception>
         /// <exception cref="SshException">The secure copy execution request was rejected by the server.</exception>
         /// <exception cref="SshConnectionException">Client is not connected.</exception>
+        /// <remarks>
+        /// <para>
+        /// 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
+        /// <see cref="ScpException"/> 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 <paramref name="directoryInfo"/>.
+        /// </para>
+        /// <para>
+        /// Existing files in <paramref name="directoryInfo"/> 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.
+        /// </para>
+        /// </remarks>
         public void Download(string directoryName, DirectoryInfo directoryInfo)
         {
             ArgumentException.ThrowIfNullOrEmpty(directoryName);
@@ -946,30 +964,61 @@ namespace Renci.SshNet
         }
 
         /// <summary>
-        /// 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.
         /// </summary>
+        /// <param name="directory">The local directory, as returned by <see cref="NormalizeDirectory(string)"/>.</param>
         /// <param name="name">The file or directory name as sent by the server.</param>
-        /// <exception cref="ScpException">
-        /// <paramref name="name"/> 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).
-        /// </exception>
-        internal static void EnsureValidLocalName(string name)
+        /// <returns>
+        /// The full path of the child, or <see langword="null"/> if <paramref name="name"/> 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
+        /// <paramref name="directory"/> itself).
+        /// </returns>
+        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;
+        }
+
+        /// <summary>
+        /// Returns the full path of a local directory without a trailing directory separator, unless
+        /// the directory is a root.
+        /// </summary>
+        /// <param name="path">The path of the directory.</param>
+        /// <returns>The normalized full path.</returns>
+        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.");
         }
 
         /// <summary>
@@ -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
+                }
+            }
+        }
     }
 }

test/Renci.SshNet.IntegrationTests/ScpTests.cs

@@ -2070,6 +2070,61 @@ namespace Renci.SshNet.IntegrationTests
             }
         }
 
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [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<ScpException>(() => 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);
+            }
+        }
+
         /// <summary>
         /// 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.

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")]

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
+{
+    /// <summary>
+    /// 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.
+    /// </summary>
+    /// <remarks>
+    /// 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.
+    /// </remarks>
+    [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<ScpException>(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<ScpException>(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<ScpException>(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<ScpException>(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
+
+        /// <summary>
+        /// Downloads a remote directory containing either a file named <paramref name="name"/>, or a
+        /// directory named <paramref name="name"/> that contains a file named <c>inner.txt</c>.
+        /// </summary>
+        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);
+        }
+    }
+}

test/Renci.SshNet.Tests/Classes/ScpClientTest_EnsureValidLocalName.cs

@@ -1,102 +0,0 @@
-using Microsoft.VisualStudio.TestTools.UnitTesting;
-
-using Renci.SshNet.Common;
-
-namespace Renci.SshNet.Tests.Classes
-{
-    /// <summary>
-    /// Tests for <see cref="ScpClient.EnsureValidLocalName(string)"/>, which guards the recursive
-    /// download against server-supplied SCP file/directory names that would write outside the
-    /// caller-supplied destination directory.
-    /// </summary>
-    [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<ScpException>(() => ScpClient.EnsureValidLocalName(string.Empty));
-        }
-
-        [TestMethod]
-        public void CurrentDirectory_ThrowsScpException()
-        {
-            _ = Assert.ThrowsExactly<ScpException>(() => ScpClient.EnsureValidLocalName("."));
-        }
-
-        [TestMethod]
-        public void ParentDirectory_ThrowsScpException()
-        {
-            _ = Assert.ThrowsExactly<ScpException>(() => ScpClient.EnsureValidLocalName(".."));
-        }
-
-        [TestMethod]
-        public void ForwardSlashPath_ThrowsScpException()
-        {
-            // '/' is an invalid file name character on every platform.
-            _ = Assert.ThrowsExactly<ScpException>(() => ScpClient.EnsureValidLocalName("sub/child"));
-            _ = Assert.ThrowsExactly<ScpException>(() => ScpClient.EnsureValidLocalName("../escaped/owned.txt"));
-        }
-
-        [TestMethod]
-        public void RootedUnixPath_ThrowsScpException()
-        {
-            // Contains '/', so it is rejected regardless of platform.
-            _ = Assert.ThrowsExactly<ScpException>(() => ScpClient.EnsureValidLocalName("/tmp/sshnet-owned.txt"));
-        }
-
-        [TestMethod]
-        public void NullCharacter_ThrowsScpException()
-        {
-            // NUL is an invalid file name character on every platform.
-            _ = Assert.ThrowsExactly<ScpException>(() => 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<ScpException>(() => ScpClient.EnsureValidLocalName("..\\escaped\\owned.txt"));
-            _ = Assert.ThrowsExactly<ScpException>(() => 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<ScpException>(() => 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<ScpException>(() => 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");
-        }
-    }
-}

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
+{
+    /// <summary>
+    /// Tests for <see cref="ScpClient.ResolveChild(string, string)"/> and
+    /// <see cref="ScpClient.NormalizeDirectory(string)"/> with a root directory as the destination,
+    /// which a download test cannot use. Other cases are covered by
+    /// <see cref="ScpClientTest_Download_PathAndDirectoryInfo_Names"/>.
+    /// </summary>
+    [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));
+        }
+    }
+}