diff --git a/Terminal.Gui/FileServices/FileSystemTreeBuilder.cs b/Terminal.Gui/FileServices/FileSystemTreeBuilder.cs index e6237a1569..80e9a740d0 100644 --- a/Terminal.Gui/FileServices/FileSystemTreeBuilder.cs +++ b/Terminal.Gui/FileServices/FileSystemTreeBuilder.cs @@ -72,5 +72,5 @@ private IEnumerable TryGetChildren (IFileSystemInfo entry) } } - private static bool IsReparsePoint (IFileSystemInfo entry) => (entry.Attributes & FileAttributes.ReparsePoint) == FileAttributes.ReparsePoint; + internal static bool IsReparsePoint (IFileSystemInfo entry) => (entry.Attributes & FileAttributes.ReparsePoint) == FileAttributes.ReparsePoint; } diff --git a/Terminal.Gui/Resources/Strings.Designer.cs b/Terminal.Gui/Resources/Strings.Designer.cs index d47ad67f22..3420c367a3 100644 --- a/Terminal.Gui/Resources/Strings.Designer.cs +++ b/Terminal.Gui/Resources/Strings.Designer.cs @@ -636,6 +636,15 @@ public static string fdNewFailed { } } + /// + /// Looks up a localized string similar to Name must not contain path separators or navigate outside the current directory.. + /// + public static string fdPathTraversalError { + get { + return ResourceManager.GetString("fdPathTraversalError", resourceCulture); + } + } + /// /// Looks up a localized string similar to New Folder. /// diff --git a/Terminal.Gui/Resources/Strings.resx b/Terminal.Gui/Resources/Strings.resx index e232d92960..7a77e89e8b 100644 --- a/Terminal.Gui/Resources/Strings.resx +++ b/Terminal.Gui/Resources/Strings.resx @@ -199,6 +199,9 @@ New Failed + + Name must not contain path separators or navigate outside the current directory. + New Folder diff --git a/Terminal.Gui/Views/FileDialogs/DefaultFileOperations.cs b/Terminal.Gui/Views/FileDialogs/DefaultFileOperations.cs index 09570a9716..0f6be01782 100644 --- a/Terminal.Gui/Views/FileDialogs/DefaultFileOperations.cs +++ b/Terminal.Gui/Views/FileDialogs/DefaultFileOperations.cs @@ -5,6 +5,36 @@ namespace Terminal.Gui.Views; /// Default file operation handlers using modal dialogs. public class DefaultFileOperations : IFileOperations { + /// + /// Determines whether a candidate path is safely contained within the specified root directory. + /// Returns if the name contains path-traversal sequences that escape the root. + /// + internal static bool IsContainedIn (string root, string candidate) + { + string rootFull = Path.GetFullPath (root) + .TrimEnd (Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar) + + Path.DirectorySeparatorChar; + + string candidateFull = Path.GetFullPath (candidate); + + return candidateFull.StartsWith (rootFull, StringComparison.Ordinal); + } + + /// + /// Returns if the name contains characters that are not valid in a file or directory name, + /// including path separators, null characters, and control characters. + /// + internal static bool ContainsInvalidNameCharacters (string name) + { + if (string.IsNullOrWhiteSpace (name)) + { + return true; + } + + char [] invalidChars = Path.GetInvalidFileNameChars (); + + return name.IndexOfAny (invalidChars) >= 0; + } /// public bool Delete (IApplication? app, IEnumerable toDelete) { @@ -72,7 +102,24 @@ public bool Delete (IApplication? app, IEnumerable toDelete) { if (toRename is IFileInfo f) { - IFileInfo newLocation = fileSystem.FileInfo.New (Path.Combine (f.Directory?.FullName ?? throw new InvalidOperationException (), newName)); + if (ContainsInvalidNameCharacters (newName)) + { + MessageBox.ErrorQuery (app, Strings.fdRenameFailedTitle, Strings.fdPathTraversalError, Strings.btnOk); + + return null; + } + + string parentDir = f.Directory?.FullName ?? throw new InvalidOperationException (); + string combined = Path.Combine (parentDir, newName); + + if (!IsContainedIn (parentDir, combined)) + { + MessageBox.ErrorQuery (app, Strings.fdRenameFailedTitle, Strings.fdPathTraversalError, Strings.btnOk); + + return null; + } + + IFileInfo newLocation = fileSystem.FileInfo.New (combined); f.MoveTo (newLocation.FullName); return newLocation; @@ -81,8 +128,24 @@ public bool Delete (IApplication? app, IEnumerable toDelete) { var d = (IDirectoryInfo)toRename; - IDirectoryInfo newLocation = - fileSystem.DirectoryInfo.New (Path.Combine (d.Parent?.FullName ?? throw new InvalidOperationException (), newName)); + if (ContainsInvalidNameCharacters (newName)) + { + MessageBox.ErrorQuery (app, Strings.fdRenameFailedTitle, Strings.fdPathTraversalError, Strings.btnOk); + + return null; + } + + string parentDir = d.Parent?.FullName ?? throw new InvalidOperationException (); + string combined = Path.Combine (parentDir, newName); + + if (!IsContainedIn (parentDir, combined)) + { + MessageBox.ErrorQuery (app, Strings.fdRenameFailedTitle, Strings.fdPathTraversalError, Strings.btnOk); + + return null; + } + + IDirectoryInfo newLocation = fileSystem.DirectoryInfo.New (combined); d.MoveTo (newLocation.FullName); return newLocation; @@ -113,7 +176,23 @@ public bool Delete (IApplication? app, IEnumerable toDelete) try { - IDirectoryInfo newDir = fileSystem.DirectoryInfo.New (Path.Combine (inDirectory.FullName, result)); + if (ContainsInvalidNameCharacters (result)) + { + MessageBox.ErrorQuery (app, Strings.fdNewFailed, Strings.fdPathTraversalError, Strings.btnOk); + + return null; + } + + string combined = Path.Combine (inDirectory.FullName, result); + + if (!IsContainedIn (inDirectory.FullName, combined)) + { + MessageBox.ErrorQuery (app, Strings.fdNewFailed, Strings.fdPathTraversalError, Strings.btnOk); + + return null; + } + + IDirectoryInfo newDir = fileSystem.DirectoryInfo.New (combined); newDir.Create (); return newDir; diff --git a/Terminal.Gui/Views/FileDialogs/FileDialog.cs b/Terminal.Gui/Views/FileDialogs/FileDialog.cs index c30621228c..82ce7817d9 100644 --- a/Terminal.Gui/Views/FileDialogs/FileDialog.cs +++ b/Terminal.Gui/Views/FileDialogs/FileDialog.cs @@ -1,5 +1,6 @@ using System.Collections.ObjectModel; using System.IO.Abstractions; +using Terminal.Gui.FileServices; namespace Terminal.Gui.Views; @@ -522,7 +523,7 @@ private void RecursiveFind (IDirectoryInfo directory) } } - if (f.FileSystemInfo is IDirectoryInfo sub) + if (f.FileSystemInfo is IDirectoryInfo sub && !FileSystemTreeBuilder.IsReparsePoint (sub)) { RecursiveFind (sub); } diff --git a/Tests/UnitTestsParallelizable/Views/DefaultFileOperationsSecurityTests.cs b/Tests/UnitTestsParallelizable/Views/DefaultFileOperationsSecurityTests.cs new file mode 100644 index 0000000000..ab2ea4441f --- /dev/null +++ b/Tests/UnitTestsParallelizable/Views/DefaultFileOperationsSecurityTests.cs @@ -0,0 +1,85 @@ +// Copilot + +namespace UnitTests.Views; + +/// +/// Tests for path-traversal and invalid-name validation in . +/// +public class DefaultFileOperationsSecurityTests +{ + [Theory] + [InlineData ("/home/user/docs", "/home/user/docs/file.txt", true)] + [InlineData ("/home/user/docs", "/home/user/docs/sub/file.txt", true)] + [InlineData ("/home/user/docs", "/home/user/file.txt", false)] + [InlineData ("/home/user/docs", "/home/user/docs/../file.txt", false)] + [InlineData ("/home/user/docs", "/home/file.txt", false)] + public void IsContainedIn_DetectsPathTraversal_Unix (string root, string candidate, bool expected) + { + if (!OperatingSystem.IsLinux () && !OperatingSystem.IsMacOS ()) + { + return; + } + + bool result = DefaultFileOperations.IsContainedIn (root, candidate); + Assert.Equal (expected, result); + } + + [Theory] + [InlineData ("C:\\Users\\docs", "C:\\Users\\docs\\file.txt", true)] + [InlineData ("C:\\Users\\docs", "C:\\Users\\docs\\sub\\file.txt", true)] + [InlineData ("C:\\Users\\docs", "C:\\Users\\file.txt", false)] + [InlineData ("C:\\Users\\docs", "C:\\Users\\docs\\..\\file.txt", false)] + public void IsContainedIn_DetectsPathTraversal_Windows (string root, string candidate, bool expected) + { + if (!OperatingSystem.IsWindows ()) + { + return; + } + + bool result = DefaultFileOperations.IsContainedIn (root, candidate); + Assert.Equal (expected, result); + } + + [Theory] + [InlineData ("validname", false)] + [InlineData ("my-file.txt", false)] + [InlineData ("../escape", true)] + [InlineData ("sub/dir", true)] + [InlineData ("", true)] + [InlineData (" ", true)] + [InlineData ("file\0name", true)] + public void ContainsInvalidNameCharacters_DetectsInvalidNames (string name, bool expected) + { + bool result = DefaultFileOperations.ContainsInvalidNameCharacters (name); + Assert.Equal (expected, result); + } + + [Fact] + public void IsContainedIn_RootIsContainedInItself_WhenSubPath () + { + // A path that is exactly the root + separator + name should be contained + if (OperatingSystem.IsWindows ()) + { + Assert.True (DefaultFileOperations.IsContainedIn ("C:\\root", "C:\\root\\child")); + } + else + { + Assert.True (DefaultFileOperations.IsContainedIn ("/root", "/root/child")); + } + } + + [Fact] + public void IsContainedIn_RootItself_IsNotContained () + { + // The root directory path itself (without trailing separator) is NOT considered "contained" + // because it's not a child path + if (OperatingSystem.IsWindows ()) + { + Assert.False (DefaultFileOperations.IsContainedIn ("C:\\root", "C:\\root")); + } + else + { + Assert.False (DefaultFileOperations.IsContainedIn ("/root", "/root")); + } + } +}