From f7948b9a7951ff6273f1f521783b79098760757b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Mon, 12 Oct 2026 18:45:18 +0300 Subject: [PATCH 1/4] Reject unsafe trash containers and bound metadata reads Co-Authored-By: Codex Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../Trash/DirectorySizesCache.cs | 5 +- .../Trash/LinuxTrashService.cs | 177 +++++++++--------- .../Trash/FdRestoreCompatibilityTests.cs | 11 +- .../Trash/LinuxTrashHardeningTests.cs | 105 +++++++++++ 4 files changed, 208 insertions(+), 90 deletions(-) diff --git a/src/Files.Platform.Linux/Trash/DirectorySizesCache.cs b/src/Files.Platform.Linux/Trash/DirectorySizesCache.cs index 83f1563136d0..d021e1738610 100644 --- a/src/Files.Platform.Linux/Trash/DirectorySizesCache.cs +++ b/src/Files.Platform.Linux/Trash/DirectorySizesCache.cs @@ -72,7 +72,10 @@ public void Clear() { try { - File.Delete(_path); + if (_directory is null) + File.Delete(_path); + else if (_directory.EntryExists("directorysizes")) + PosixNative.UnlinkAt(_directory.Descriptor, "directorysizes", 0, _path); } catch (IOException) { diff --git a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs index 2e6b5e3eee13..75dfbb783f07 100644 --- a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs +++ b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs @@ -26,6 +26,7 @@ public sealed class LinuxTrashService : ITrashService private const string FilesDirectoryName = "files"; private const string InfoDirectoryName = "info"; private const int MaxUniqueNameAttempts = 10000; + private const int MaxMetadataBytes = 64 * 1024; private const string OutsideVolumeMessage = "Original location outside the volume"; private const UnixFileMode PrivateDirectoryMode = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute; @@ -176,26 +177,33 @@ public Task EmptyAsync(CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - foreach (var directory in new[] { location.FilesDirectory, location.InfoDirectory }) + try { - if (!Directory.Exists(directory)) - continue; - - foreach (var entry in Directory.EnumerateFileSystemEntries(directory)) + using var root = OpenTrashRoot(location); + using var files = DirectoryHandle.OpenChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); + using var info = DirectoryHandle.OpenChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); + foreach (var directory in new[] { files, info }) { - cancellationToken.ThrowIfCancellationRequested(); - try + foreach (var name in directory.ListNames()) { - DeleteEntry(entry); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - failures++; + cancellationToken.ThrowIfCancellationRequested(); + try + { + _options.FileOperationsHooks?.BeforeDeleteEntry?.Invoke(Path.Combine(directory.Path, name)); + directory.DeleteEntry(name); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + failures++; + } } } + new DirectorySizesCache(location.Root, root).Clear(); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + failures++; } - - new DirectorySizesCache(location.Root).Clear(); } if (failures > 0) @@ -288,7 +296,7 @@ private TrashOperationResult TrashOne(string path) if (isDirectory) new DirectorySizesCache(location.Root).Set(name, size, new DateTimeOffset(info.LastWriteTimeUtc).ToUnixTimeSeconds()); - var deletionDate = ReadDeletionDate(infoPath) ?? new DateTimeOffset(_options.LocalNow()); + var deletionDate = ReadDeletionDate(infoPath, CancellationToken.None) ?? new DateTimeOffset(_options.LocalNow()); var item = new TrashItem(trashedPath, full, deletionDate, size, isDirectory, infoPath); return TrashOperationResult.Success(path, trashedPath, item); } @@ -343,35 +351,29 @@ private IReadOnlyList ListCore(CancellationToken cancellationToken) foreach (var location in GetLocations()) { - if (!Directory.Exists(location.InfoDirectory)) - continue; - - var cache = new DirectorySizesCache(location.Root); - - IEnumerable infoFiles; try { - infoFiles = Directory.EnumerateFiles(location.InfoDirectory, "*" + TrashInfoFile.Extension).ToList(); + using var root = OpenTrashRoot(location); + using var files = DirectoryHandle.OpenChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); + using var info = DirectoryHandle.OpenChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); + var cache = new DirectorySizesCache(location.Root, root); + foreach (var infoName in info.ListNames().Where(n => n.EndsWith(TrashInfoFile.Extension, StringComparison.Ordinal))) + { + cancellationToken.ThrowIfCancellationRequested(); + var item = TryReadItem(location, Path.Combine(location.InfoDirectory, infoName), cache, info, files, cancellationToken); + if (item is not null) + result.Add(item); + } } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - continue; - } - - foreach (var infoPath in infoFiles) - { - cancellationToken.ThrowIfCancellationRequested(); - - var item = TryReadItem(location, infoPath, cache); - if (item is not null) - result.Add(item); } } return result; } - private TrashItem? TryReadItem(TrashLocation location, string infoPath, DirectorySizesCache cache) + private TrashItem? TryReadItem(TrashLocation location, string infoPath, DirectorySizesCache cache, DirectoryHandle info, DirectoryHandle files, CancellationToken cancellationToken) { try { @@ -380,15 +382,18 @@ private IReadOnlyList ListCore(CancellationToken cancellationToken) var trashedPath = Path.Combine(location.FilesDirectory, name); // Orphaned or corrupt .trashinfo files are ignored. - if (!IsValidEntryName(name) || !EntryExists(trashedPath)) + if (!IsValidEntryName(name) || !files.EntryExists(name)) return null; - if (!TrashInfoFile.TryParse(File.ReadAllText(infoPath), out var storedPath, out var date)) + using var stream = OpenMetadataAt(info.Descriptor, fileName, infoPath); + if (!TrashInfoFile.TryParse(ReadMetadata(stream, cancellationToken), out var storedPath, out var date)) return null; var originalPath = ResolveOriginalPath(location, storedPath, out var invalidReason) ?? storedPath; - var infoMtime = new DateTimeOffset(new FileInfo(infoPath).LastWriteTimeUtc); + if (!PosixNative.TryStat(stream.SafeFileHandle.DangerousGetHandle().ToInt32(), out var metadataStat)) + return null; + var infoMtime = new DateTimeOffset(metadataStat.ModifiedUtc); var deletionDate = date is { } d ? new DateTimeOffset(DateTime.SpecifyKind(d, DateTimeKind.Unspecified), TimeZoneInfo.Local.GetUtcOffset(d)) : infoMtime; @@ -413,11 +418,11 @@ private IReadOnlyList ListCore(CancellationToken cancellationToken) } } - private static DateTimeOffset? ReadDeletionDate(string infoPath) + private static DateTimeOffset? ReadDeletionDate(string infoPath, CancellationToken cancellationToken) { try { - if (TrashInfoFile.TryParse(File.ReadAllText(infoPath), out _, out var date) && date is { } d) + if (TrashInfoFile.TryParse(ReadMetadata(infoPath, cancellationToken), out _, out var date) && date is { } d) return new DateTimeOffset(DateTime.SpecifyKind(d, DateTimeKind.Unspecified), TimeZoneInfo.Local.GetUtcOffset(d)); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) @@ -436,18 +441,15 @@ private async Task RestoreOneAsync(TrashItem item, TrashRe if (!TryGetItemLocation(item, out var location, out var name, out var infoPath)) return TrashOperationResult.Failure(item.TrashedPath, "The item is not located in a trash folder."); - using var trashRoot = DirectoryHandle.OpenPath(location.TopDirectory is null - ? RealPath(location.Root) - : Path.Combine(RealPath(Path.GetDirectoryName(location.Root)!), Path.GetFileName(location.Root))); + using var trashRoot = OpenTrashRoot(location); using var files = DirectoryHandle.OpenChild(trashRoot.Descriptor, FilesDirectoryName, location.FilesDirectory); using var info = DirectoryHandle.OpenChild(trashRoot.Descriptor, InfoDirectoryName, location.InfoDirectory); if (!files.EntryExists(name)) return TrashOperationResult.Failure(item.TrashedPath, "The trashed item no longer exists."); // Re-derive the destination from the pinned .trashinfo, never from the caller-supplied item. - using var infoStream = PosixNative.OpenFileAt(info.Descriptor, name + TrashInfoFile.Extension, infoPath, false); - using var reader = new StreamReader(infoStream); - if (!TrashInfoFile.TryParse(reader.ReadToEnd(), out var storedPath, out _)) + using var infoStream = OpenMetadataAt(info.Descriptor, name + TrashInfoFile.Extension, infoPath); + if (!TrashInfoFile.TryParse(ReadMetadata(infoStream, cancellationToken), out var storedPath, out _)) return TrashOperationResult.Failure(item.TrashedPath, "The .trashinfo file is invalid."); var destination = ResolveOriginalPath(location, storedPath, out var invalidReason); @@ -544,11 +546,17 @@ private TrashOperationResult DeleteOne(TrashItem item) if (!TryGetItemLocation(item, out var location, out var name, out var infoPath)) return TrashOperationResult.Failure(item.TrashedPath, "The item is not located in a trash folder."); - if (EntryExists(item.TrashedPath)) - DeleteEntry(item.TrashedPath); - - TryDelete(infoPath); - new DirectorySizesCache(location.Root).Remove([name]); + using var root = OpenTrashRoot(location); + using var files = DirectoryHandle.OpenChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); + using var info = DirectoryHandle.OpenChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); + if (files.EntryExists(name)) + { + _options.FileOperationsHooks?.BeforeDeleteEntry?.Invoke(item.TrashedPath); + files.DeleteEntry(name); + } + if (info.EntryExists(name + TrashInfoFile.Extension)) + info.DeleteEntry(name + TrashInfoFile.Extension); + new DirectorySizesCache(location.Root, root).Remove([name]); return TrashOperationResult.Success(item.TrashedPath); } @@ -651,7 +659,7 @@ private List GetLocations() { var locations = new List(); - if (Directory.Exists(HomeRoot)) + if (Directory.Exists(HomeRoot) && IsTrusted(new TrashLocation(HomeRoot, null))) locations.Add(new TrashLocation(HomeRoot, null)); string homeMount; @@ -727,8 +735,8 @@ private List GetLocations() private TrashLocation? PrepareLocation(TrashLocation location, bool create) { - // A topdir trash that another user pre-created, symlinked or made writable must never be used. - if (location.TopDirectory is not null && EntryExists(location.Root) && !IsTrusted(location)) + // A trash directory that another user pre-created, symlinked or made writable must never be used. + if (EntryExists(location.Root) && !IsTrusted(location)) return null; if (!create) @@ -737,7 +745,7 @@ private List GetLocations() try { EnsureLayout(location); - return location.TopDirectory is not null && !IsTrusted(location) ? null : location; + return !IsTrusted(location) ? null : location; } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -746,14 +754,11 @@ private List GetLocations() } /// - /// Checks that a topdir trash and its files/info subdirectories are real directories owned by the current - /// user that are not writable by group or others. The home trash is not checked. + /// Checks that a trash and its files/info subdirectories are real directories owned by the current + /// user that are not writable by group or others. /// private bool IsTrusted(TrashLocation location) { - if (location.TopDirectory is null) - return true; - var forbidden = UnixFileMode.GroupWrite | UnixFileMode.OtherWrite; foreach (var directory in new[] { location.Root, location.FilesDirectory, location.InfoDirectory }) @@ -887,39 +892,43 @@ private static void TryDelete(string path) } } - private static void DeleteEntry(string path) - { - if (!IsDirectoryEntry(path)) - { - File.Delete(path); - return; - } + private static DirectoryHandle OpenTrashRoot(TrashLocation location) + => DirectoryHandle.OpenPath(Path.Combine(RealPath(Path.GetDirectoryName(location.Root)!), Path.GetFileName(location.Root))); - try - { - Directory.Delete(path, recursive: true); - } - catch (UnauthorizedAccessException) - { - // Directories without write permission cannot be emptied; make the tree writable and retry. - MakeTreeWritable(path); - Directory.Delete(path, recursive: true); - } + private static FileStream OpenMetadataAt(int descriptor, string name, string path) + { + var stat = PosixNative.StatAt(descriptor, name, path); + if (!stat.IsRegularFile || stat.Size > MaxMetadataBytes) + throw new IOException("Trash metadata is not a bounded regular file."); + // OpenFileAt also validates the opened inode and refuses links or blocking special files. + return PosixNative.OpenFileAt(descriptor, name, path, false); } - private static void MakeTreeWritable(string path) + private static string ReadMetadata(string path, CancellationToken cancellationToken) { - var mode = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute; + using var parent = DirectoryHandle.OpenPath(Path.GetDirectoryName(path)!); + using var stream = OpenMetadataAt(parent.Descriptor, Path.GetFileName(path), path); + return ReadMetadata(stream, cancellationToken); + } - try - { - File.SetUnixFileMode(path, File.GetUnixFileMode(path) | mode); - foreach (var directory in Directory.EnumerateDirectories(path, "*", new EnumerationOptions { RecurseSubdirectories = true, AttributesToSkip = FileAttributes.ReparsePoint, IgnoreInaccessible = true })) - File.SetUnixFileMode(directory, File.GetUnixFileMode(directory) | mode); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + private static string ReadMetadata(FileStream stream, CancellationToken cancellationToken) + { + if (stream.Length > MaxMetadataBytes) + throw new IOException("Trash metadata is too large."); + var bytes = new byte[MaxMetadataBytes + 1]; + var count = 0; + while (count < bytes.Length) { + cancellationToken.ThrowIfCancellationRequested(); + var read = stream.Read(bytes, count, Math.Min(4096, bytes.Length - count)); + if (read == 0) + break; + count += read; } + if (count > MaxMetadataBytes) + throw new IOException("Trash metadata is too large."); + using var reader = new StreamReader(new MemoryStream(bytes, 0, count), Encoding.UTF8); + return reader.ReadToEnd(); } /// diff --git a/tests/Files.Platform.Tests/Trash/FdRestoreCompatibilityTests.cs b/tests/Files.Platform.Tests/Trash/FdRestoreCompatibilityTests.cs index a7cf2cd0487f..bc32d58e2d60 100644 --- a/tests/Files.Platform.Tests/Trash/FdRestoreCompatibilityTests.cs +++ b/tests/Files.Platform.Tests/Trash/FdRestoreCompatibilityTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Files Community +// Copyright (c) Files Community // Licensed under the MIT License. #pragma warning disable CA1416 @@ -157,7 +157,7 @@ public async Task Restore_CrossDevice_CancellationKeepsTrashAndInfo() } [TestMethod] - public async Task Restore_HomeTrashSymlink_ResolvesRoot() + public async Task Restore_HomeTrashSymlink_IsRejected() { var original = WriteFile(_home, "entry", "data"); var item = Single(await _service.TrashAsync([original])).Item!; @@ -166,9 +166,10 @@ public async Task Restore_HomeTrashSymlink_ResolvesRoot() var result = Single(await _service.RestoreAsync([item])); - Assert.IsTrue(result.Succeeded, result.ErrorMessage); - Assert.AreEqual("data", File.ReadAllText(original)); - Assert.IsFalse(File.Exists(item.TrashId)); + Assert.IsFalse(result.Succeeded); + Assert.IsFalse(File.Exists(original)); + Assert.AreEqual("data", File.ReadAllText(item.TrashedPath)); + Assert.IsTrue(File.Exists(item.TrashId)); } [TestMethod] diff --git a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs index 5d243f79f5f2..62b1f7f87563 100644 --- a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs +++ b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs @@ -17,6 +17,111 @@ namespace Files.Platform.Tests.Trash { public sealed partial class LinuxTrashServiceTests { + [TestMethod] + [DataRow("Trash")] + [DataRow("files")] + [DataRow("info")] + public async Task Empty_HomeContainerSymlink_PreservesOutsideEntries(string container) + { + var outside = Path.Combine(_root, "outside"); + var victim = WriteFile(container == "Trash" ? Path.Combine(outside, "files") : outside, "important"); + Directory.CreateDirectory(HomeTrash); + Directory.CreateSymbolicLink(container == "Trash" ? HomeTrash + "-link" : Path.Combine(HomeTrash, container), outside); + if (container == "Trash") + { + Directory.Delete(HomeTrash); + Directory.Move(HomeTrash + "-link", HomeTrash); + } + await _service.EmptyAsync(); + Assert.IsTrue(File.Exists(victim)); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task Metadata_Oversized_IsIgnoredAndCannotRestore(bool symlink) + { + var item = Single(await _service.TrashAsync([WriteFile(_home, "large.txt")])).Item!; + var content = File.ReadAllText(item.TrashId) + new string('x', 65536); + File.Delete(item.TrashId); + if (symlink) + { + var target = WriteFile(_root, "metadata", content); + File.CreateSymbolicLink(item.TrashId, target); + } + else + File.WriteAllText(item.TrashId, content); + Assert.IsEmpty(await _service.ListAsync()); + Assert.IsFalse(Single(await _service.RestoreAsync([item])).Succeeded); + Assert.IsTrue(File.Exists(item.TrashedPath)); + } + + [TestMethod] + public async Task Metadata_Fifo_IsIgnoredWithoutBlocking() + { + var item = Single(await _service.TrashAsync([WriteFile(_home, "pipe.txt")])).Item!; + File.Delete(item.TrashId); + var start = new System.Diagnostics.ProcessStartInfo("mkfifo") { UseShellExecute = false }; + start.ArgumentList.Add(item.TrashId); + using var process = System.Diagnostics.Process.Start(start)!; + await process.WaitForExitAsync(); + Assert.AreEqual(0, process.ExitCode); + Assert.IsEmpty(await _service.ListAsync().WaitAsync(TimeSpan.FromSeconds(5))); + Assert.IsFalse(Single(await _service.RestoreAsync([item])).Succeeded); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task Delete_ContainerSwapAfterPinning_PreservesOutsideEntries(bool empty) + { + var outside = Path.Combine(_root, "outside"); + var victim = WriteFile(outside, "entry"); + var service = new LinuxTrashService(new LinuxTrashOptions + { + DataHome = _dataHome, UserId = Uid, OwnershipInspector = _inspector, + MountResolver = new FakeMountResolver(_root, _home, _usb), + FileOperationsHooks = new Files.Platform.Linux.FileOperations.LinuxFileOperationsHooks + { + BeforeDeleteEntry = _ => + { + var files = Path.Combine(HomeTrash, "files"); + if (new DirectoryInfo(files).LinkTarget is not null) return; + Directory.Move(files, files + ".old"); + Directory.CreateSymbolicLink(files, outside); + } + } + }); + using var watcher = service.Watcher; + var item = Single(await service.TrashAsync([WriteFile(_home, "entry")])).Item!; + if (empty) await service.EmptyAsync(); + else Assert.IsTrue(Single(await service.DeletePermanentlyAsync([item])).Succeeded); + Assert.AreEqual("hello", File.ReadAllText(victim)); + } + + [TestMethod] + public async Task Metadata_ValidSymlink_IsIgnored() + { + var item = Single(await _service.TrashAsync([WriteFile(_home, "link.txt")])).Item!; + File.Move(item.TrashId, item.TrashId + ".real"); + File.CreateSymbolicLink(item.TrashId, item.TrashId + ".real"); + Assert.IsEmpty(await _service.ListAsync()); + Assert.IsFalse(Single(await _service.RestoreAsync([item])).Succeeded); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task Empty_HomeTrashUntrustedOwnerOrMode_PreservesEntries(bool wrongOwner) + { + var item = Single(await _service.TrashAsync([WriteFile(_home, "entry")])).Item!; + if (wrongOwner) _inspector.Owners[HomeTrash] = Uid + 1; + else File.SetUnixFileMode(HomeTrash, (UnixFileMode)0x1FF); + await _service.EmptyAsync(); + Assert.IsTrue(File.Exists(item.TrashedPath)); + Assert.IsFalse(Single(await _service.DeletePermanentlyAsync([item])).Succeeded); + } + private static string TamperInfo(string infoPath, string newPath) { File.WriteAllText(infoPath, $"[Trash Info]\nPath={newPath}\nDeletionDate=2026-10-04T13:05:09\n"); From efb43c2ee8aa7cc57b7ee455f51bf0b344bf2bac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Mon, 12 Oct 2026 23:58:37 +0300 Subject: [PATCH 2/4] Address review findings Co-Authored-By: Codex Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../Launching/DesktopExecExpander.cs | 32 +++++++++++- .../Trash/LinuxTrashService.cs | 52 +++++++++++++++---- .../Mime/ServiceMenuTests.cs | 15 ++++++ .../Trash/LinuxTrashHardeningTests.cs | 38 ++++++++++++++ .../Trash/LinuxTrashServiceTests.cs | 5 +- 5 files changed, 128 insertions(+), 14 deletions(-) diff --git a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs index 5bdc356b6839..777646ccbce0 100644 --- a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs +++ b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs @@ -14,7 +14,7 @@ namespace Files.Platform.Linux.Launching /// public static class DesktopExecExpander { - private static readonly HashSet Shells = ["sh", "bash", "dash", "zsh", "ksh", "fish", "ash", "csh", "tcsh"]; + private static readonly HashSet Shells = ["sh", "bash", "dash", "zsh", "ksh", "fish", "ash", "csh", "tcsh", "rbash"]; /// /// Splits an Exec value into arguments using shell quoting rules (double quotes, single quotes, backslash). @@ -115,7 +115,7 @@ public static IReadOnlyList> ExpandServiceMenu(DesktopAppl if (app.Exec.Any(c => c is '|' or '&' or ';' or '<' or '>' or '`' or '$' or '*' or '?' or '~' or '#' || char.IsControl(c))) return []; var tokens = Tokenize(app.Exec); if (tokens is null || tokens.Count == 0 || tokens[0].Contains('%') || tokens[0].Contains('=') || - tokens.Any(t => Shells.Contains(System.IO.Path.GetFileName(t))) || + InvokesShell(tokens) || (System.IO.Path.GetFileName(tokens[0]) == "env" && tokens.Any(t => t.StartsWith("-S", StringComparison.Ordinal) || t.StartsWith("--split-string", StringComparison.Ordinal)))) return []; var targetCodes = tokens.Select(t => t is "%f" or "%F" or "%u" or "%U" ? t[1] : LongOptionTargetCode(t)).ToArray(); if (targetCodes.Count(c => c != '\0') > 1 || @@ -165,6 +165,34 @@ public static IReadOnlyList> ExpandServiceMenu(DesktopAppl return result; } + private static bool InvokesShell(IReadOnlyList tokens) + { + for (var i = 0; i < tokens.Count; i++) + { + var executable = tokens[i]; + if (executable.Contains('=') || executable.StartsWith('-')) continue; + var name = System.IO.Path.GetFileName(executable); + try + { + var path = executable.Contains('/') ? executable : + (Environment.GetEnvironmentVariable("PATH") ?? "").Split(':') + .Select(p => System.IO.Path.Combine(p, executable)).FirstOrDefault(System.IO.File.Exists); + if (path is not null) + name = System.IO.Path.GetFileName(Trash.LinuxTrashService.RealPath(path)); + } + catch (Exception ex) when (ex is System.IO.IOException or UnauthorizedAccessException) { } + if (!Shells.Contains(name) && !Shells.Contains(System.IO.Path.GetFileName(executable))) continue; + for (var j = i + 1; j < tokens.Count; j++) + { + var arg = tokens[j]; + if (arg == "--") return j + 1 < tokens.Count; + if (arg.StartsWith('-') && !arg.StartsWith("--", StringComparison.Ordinal) && arg.Contains('c')) return true; + if (!arg.StartsWith('-')) return true; + } + } + return false; + } + // Accept only --name=%x with an ASCII option name and no surrounding value text. private static char LongOptionTargetCode(string token) { diff --git a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs index acdbf3295ad5..c3f73f375376 100644 --- a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs +++ b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs @@ -180,10 +180,17 @@ public Task EmptyAsync(CancellationToken cancellationToken = default) try { using var root = OpenTrashRoot(location); - using var files = DirectoryHandle.OpenChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); - using var info = DirectoryHandle.OpenChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); - foreach (var directory in new[] { files, info }) + foreach (var child in new[] { FilesDirectoryName, InfoDirectoryName }) { + var path = Path.Combine(location.Root, child); + using var directory = DirectoryHandle.TryOpen(root.Descriptor, child, path, true, out var errno); + if (directory is null) + { + if (PosixNative.IsNotFound(errno)) continue; + failures++; + continue; + } + ValidateTrustedDirectory(directory); foreach (var name in directory.ListNames()) { cancellationToken.ThrowIfCancellationRequested(); @@ -354,8 +361,8 @@ private IReadOnlyList ListCore(CancellationToken cancellationToken) try { using var root = OpenTrashRoot(location); - using var files = DirectoryHandle.OpenChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); - using var info = DirectoryHandle.OpenChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); + using var files = OpenTrustedChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); + using var info = OpenTrustedChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); var cache = new DirectorySizesCache(location.Root, root); foreach (var infoName in info.ListNames().Where(n => n.EndsWith(TrashInfoFile.Extension, StringComparison.Ordinal))) { @@ -442,8 +449,8 @@ private async Task RestoreOneAsync(TrashItem item, TrashRe return TrashOperationResult.Failure(item.TrashedPath, "The item is not located in a trash folder."); using var trashRoot = OpenTrashRoot(location); - using var files = DirectoryHandle.OpenChild(trashRoot.Descriptor, FilesDirectoryName, location.FilesDirectory); - using var info = DirectoryHandle.OpenChild(trashRoot.Descriptor, InfoDirectoryName, location.InfoDirectory); + using var files = OpenTrustedChild(trashRoot.Descriptor, FilesDirectoryName, location.FilesDirectory); + using var info = OpenTrustedChild(trashRoot.Descriptor, InfoDirectoryName, location.InfoDirectory); if (!files.EntryExists(name)) return TrashOperationResult.Failure(item.TrashedPath, "The trashed item no longer exists."); @@ -547,8 +554,8 @@ private TrashOperationResult DeleteOne(TrashItem item) return TrashOperationResult.Failure(item.TrashedPath, "The item is not located in a trash folder."); using var root = OpenTrashRoot(location); - using var files = DirectoryHandle.OpenChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); - using var info = DirectoryHandle.OpenChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); + using var files = OpenTrustedChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); + using var info = OpenTrustedChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); if (files.EntryExists(name)) { _options.FileOperationsHooks?.BeforeDeleteEntry?.Invoke(item.TrashedPath); @@ -892,8 +899,31 @@ private static void TryDelete(string path) } } - private static DirectoryHandle OpenTrashRoot(TrashLocation location) - => DirectoryHandle.OpenPath(Path.Combine(RealPath(Path.GetDirectoryName(location.Root)!), Path.GetFileName(location.Root))); + private DirectoryHandle OpenTrashRoot(TrashLocation location) + { + // Walk every component with openat/O_NOFOLLOW; never resolve a checked ancestor again. + using var parent = DirectoryHandle.OpenPath(Path.GetDirectoryName(location.Root)!); + if (location.TopDirectory is not null && Path.GetFileName(parent.Path) == ".Trash") + { + if (!PosixNative.TryStat(parent.Descriptor, out var shared) || (shared.Mode & 0x200) == 0) + throw new IOException("Shared trash directory is not sticky."); + } + return OpenTrustedChild(parent.Descriptor, Path.GetFileName(location.Root), location.Root); + } + + private DirectoryHandle OpenTrustedChild(int descriptor, string name, string path) + { + var directory = DirectoryHandle.OpenChild(descriptor, name, path); + try { ValidateTrustedDirectory(directory); return directory; } + catch { directory.Dispose(); throw; } + } + + private void ValidateTrustedDirectory(DirectoryHandle directory) + { + if (!PosixNative.TryStat(directory.Descriptor, out var stat) || !stat.IsDirectory || + stat.OwnerUserId != _options.UserId || (stat.Mode & 0x12) != 0) + throw new IOException("Trash directory is not trusted."); + } private static FileStream OpenMetadataAt(int descriptor, string name, string path) { diff --git a/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs b/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs index a5489484af0b..0bf11028ccc9 100644 --- a/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs +++ b/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs @@ -28,6 +28,21 @@ private static string Text(string extra = "", string exec = "tool %F") => private static LinuxServiceMenuService Service(XdgFixture fx) => new(fx.Directories, new LinuxMimeTypeService(fx.Directories, CultureInfo.InvariantCulture), CultureInfo.GetCultureInfo("fr-FR")); + [TestMethod] + public void Expansion_RejectsResolvedShellAliasesInWrappers() + { + var path = Path.Combine(Path.GetTempPath(), "shell-alias-" + Guid.NewGuid().ToString("N")); + File.CreateSymbolicLink(path, "/bin/sh"); + try + { + foreach (var exec in new[] { path + " -c code", "env VAR=bash " + path + " script", "env rbash -c code", "env tcsh script" }) + Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(new DesktopApplication("run", "Run", exec, "/menu.desktop"), ["/a"]).Count, exec); + foreach (var exec in new[] { "env VAR=bash tool %f", "bash --version", "tool VAR=bash %f" }) + Assert.AreEqual(1, DesktopExecExpander.ExpandServiceMenu(new DesktopApplication("run", "Run", exec, "/menu.desktop"), ["/a"]).Count, exec); + } + finally { File.Delete(path); } + } + [TestMethod] public async Task Cache_ReusesParsingButRefiltersAndInvalidatesChanges() { diff --git a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs index 62b1f7f87563..3d1c1bd49776 100644 --- a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs +++ b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs @@ -17,6 +17,44 @@ namespace Files.Platform.Tests.Trash { public sealed partial class LinuxTrashServiceTests { + [TestMethod] + [DataRow("files")] + [DataRow("info")] + public async Task Empty_PartialLayout_DeletesExistingChild(string child) + { + var item = Single(await _service.TrashAsync([WriteFile(_home, "entry")])).Item!; + Directory.Delete(Path.Combine(HomeTrash, child == "files" ? "info" : "files"), true); + await _service.EmptyAsync(); + Assert.IsEmpty(Directory.GetFileSystemEntries(Path.Combine(HomeTrash, child))); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task Empty_AncestorSwapDuringTrustCheck_PreservesOutsideEntries(bool shared) + { + var item = Single(await _service.TrashAsync([WriteFile(_home, "entry")])).Item!; + var parent = shared ? Path.Combine(_usb, ".Trash") : _dataHome; + var leaf = shared ? Uid.ToString(System.Globalization.CultureInfo.InvariantCulture) : "Trash"; + if (shared) + { + PlantEntry(Path.Combine(parent, leaf), "entry"); + File.SetUnixFileMode(parent, UnixFileMode.StickyBit | UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + var outside = Path.Combine(_root, "outside"); + var victim = WriteFile(Path.Combine(outside, leaf, "files"), "victim"); + Directory.CreateDirectory(Path.Combine(outside, leaf, "info")); + _inspector.AfterInspect = path => + { + if (path != Path.Combine(parent, leaf, "info")) return; + _inspector.AfterInspect = null; + Directory.Move(parent, parent + ".old"); + Directory.CreateSymbolicLink(parent, outside); + }; + try { await _service.EmptyAsync(); } catch (IOException) { } + Assert.IsTrue(File.Exists(victim)); + } + [TestMethod] [DataRow("Trash")] [DataRow("files")] diff --git a/tests/Files.Platform.Tests/Trash/LinuxTrashServiceTests.cs b/tests/Files.Platform.Tests/Trash/LinuxTrashServiceTests.cs index 830ca8687cdd..7b0872b0ee2b 100644 --- a/tests/Files.Platform.Tests/Trash/LinuxTrashServiceTests.cs +++ b/tests/Files.Platform.Tests/Trash/LinuxTrashServiceTests.cs @@ -22,7 +22,7 @@ namespace Files.Platform.Tests.Trash [TestClass] public sealed partial class LinuxTrashServiceTests { - private const uint Uid = 4242; + private static readonly uint Uid = ProcessIdentityNative.CurrentUserId; private string _root = null!; private string _home = null!; @@ -671,6 +671,8 @@ public FakeOwnershipInspector(uint defaultOwner) _defaultOwner = defaultOwner; } + public Action? AfterInspect { get; set; } + public Dictionary Owners { get; } = []; public bool TryGetInfo(string path, out FileEntryInfo info) @@ -683,6 +685,7 @@ public bool TryGetInfo(string path, out FileEntryInfo info) var owner = Owners.TryGetValue(path, out var o) ? o : _defaultOwner; info = new FileEntryInfo(!link, link, owner, link ? 0 : File.GetUnixFileMode(path)); + AfterInspect?.Invoke(path); return true; } } From 22ab7dcb30931f50f15b1930d667cf2fb5cf9c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Tue, 13 Oct 2026 00:47:45 +0300 Subject: [PATCH 3/4] Address re-review findings Co-Authored-By: Codex Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../Launching/DesktopExecExpander.cs | 32 +-------- .../Trash/LinuxTrashService.cs | 30 ++++++--- .../Mime/ServiceMenuTests.cs | 15 ----- .../Trash/LinuxTrashHardeningTests.cs | 65 +++++++++++++++++++ 4 files changed, 88 insertions(+), 54 deletions(-) diff --git a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs index 777646ccbce0..5bdc356b6839 100644 --- a/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs +++ b/src/Files.Platform.Linux/Launching/DesktopExecExpander.cs @@ -14,7 +14,7 @@ namespace Files.Platform.Linux.Launching /// public static class DesktopExecExpander { - private static readonly HashSet Shells = ["sh", "bash", "dash", "zsh", "ksh", "fish", "ash", "csh", "tcsh", "rbash"]; + private static readonly HashSet Shells = ["sh", "bash", "dash", "zsh", "ksh", "fish", "ash", "csh", "tcsh"]; /// /// Splits an Exec value into arguments using shell quoting rules (double quotes, single quotes, backslash). @@ -115,7 +115,7 @@ public static IReadOnlyList> ExpandServiceMenu(DesktopAppl if (app.Exec.Any(c => c is '|' or '&' or ';' or '<' or '>' or '`' or '$' or '*' or '?' or '~' or '#' || char.IsControl(c))) return []; var tokens = Tokenize(app.Exec); if (tokens is null || tokens.Count == 0 || tokens[0].Contains('%') || tokens[0].Contains('=') || - InvokesShell(tokens) || + tokens.Any(t => Shells.Contains(System.IO.Path.GetFileName(t))) || (System.IO.Path.GetFileName(tokens[0]) == "env" && tokens.Any(t => t.StartsWith("-S", StringComparison.Ordinal) || t.StartsWith("--split-string", StringComparison.Ordinal)))) return []; var targetCodes = tokens.Select(t => t is "%f" or "%F" or "%u" or "%U" ? t[1] : LongOptionTargetCode(t)).ToArray(); if (targetCodes.Count(c => c != '\0') > 1 || @@ -165,34 +165,6 @@ public static IReadOnlyList> ExpandServiceMenu(DesktopAppl return result; } - private static bool InvokesShell(IReadOnlyList tokens) - { - for (var i = 0; i < tokens.Count; i++) - { - var executable = tokens[i]; - if (executable.Contains('=') || executable.StartsWith('-')) continue; - var name = System.IO.Path.GetFileName(executable); - try - { - var path = executable.Contains('/') ? executable : - (Environment.GetEnvironmentVariable("PATH") ?? "").Split(':') - .Select(p => System.IO.Path.Combine(p, executable)).FirstOrDefault(System.IO.File.Exists); - if (path is not null) - name = System.IO.Path.GetFileName(Trash.LinuxTrashService.RealPath(path)); - } - catch (Exception ex) when (ex is System.IO.IOException or UnauthorizedAccessException) { } - if (!Shells.Contains(name) && !Shells.Contains(System.IO.Path.GetFileName(executable))) continue; - for (var j = i + 1; j < tokens.Count; j++) - { - var arg = tokens[j]; - if (arg == "--") return j + 1 < tokens.Count; - if (arg.StartsWith('-') && !arg.StartsWith("--", StringComparison.Ordinal) && arg.Contains('c')) return true; - if (!arg.StartsWith('-')) return true; - } - } - return false; - } - // Accept only --name=%x with an ASCII option name and no surrounding value text. private static char LongOptionTargetCode(string token) { diff --git a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs index c3f73f375376..564385a7ed91 100644 --- a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs +++ b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs @@ -666,8 +666,9 @@ private List GetLocations() { var locations = new List(); - if (Directory.Exists(HomeRoot) && IsTrusted(new TrashLocation(HomeRoot, null))) - locations.Add(new TrashLocation(HomeRoot, null)); + var home = new TrashLocation(HomeRoot, null); + if (Directory.Exists(HomeRoot) && IsTrusted(home)) + locations.Add(home); string homeMount; try @@ -688,12 +689,14 @@ private List GetLocations() try { var shared = GetSharedTrashRoot(mount.MountPoint); - if (shared is not null && Directory.Exists(shared) && IsTrusted(new TrashLocation(shared, mount.MountPoint))) - locations.Add(new TrashLocation(shared, mount.MountPoint)); + var sharedLocation = shared is null ? null : new TrashLocation(shared, mount.MountPoint); + if (sharedLocation is not null && Directory.Exists(shared) && IsTrusted(sharedLocation)) + locations.Add(sharedLocation); var perUser = GetPerUserTrashRoot(mount.MountPoint); - if (Directory.Exists(perUser) && IsTrusted(new TrashLocation(perUser, mount.MountPoint))) - locations.Add(new TrashLocation(perUser, mount.MountPoint)); + var perUserLocation = new TrashLocation(perUser, mount.MountPoint); + if (Directory.Exists(perUser) && IsTrusted(perUserLocation)) + locations.Add(perUserLocation); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -901,9 +904,13 @@ private static void TryDelete(string path) private DirectoryHandle OpenTrashRoot(TrashLocation location) { - // Walk every component with openat/O_NOFOLLOW; never resolve a checked ancestor again. - using var parent = DirectoryHandle.OpenPath(Path.GetDirectoryName(location.Root)!); - if (location.TopDirectory is not null && Path.GetFileName(parent.Path) == ".Trash") + var parentPath = Path.GetDirectoryName(location.Root)!; + var sharedTrash = location.TopDirectory is not null && Path.GetFileName(parentPath) == ".Trash"; + // Resolve ordinary ancestors once; trash containers remain no-follow relative to the pinned fd. + using var ancestor = DirectoryHandle.OpenPath(location.ResolvedAncestor); + using var sharedDirectory = sharedTrash ? DirectoryHandle.OpenChild(ancestor.Descriptor, ".Trash", parentPath) : null; + var parent = sharedDirectory ?? ancestor; + if (sharedTrash) { if (!PosixNative.TryStat(parent.Descriptor, out var shared) || (shared.Mode & 0x200) == 0) throw new IOException("Shared trash directory is not sticky."); @@ -1012,6 +1019,11 @@ internal static string RealPath(string path) private sealed record TrashLocation(string Root, string? TopDirectory) { + // Capture ordinary ancestors before path-based trust checks can observe a replacement. + public string ResolvedAncestor { get; } = RealPath(TopDirectory is not null && Path.GetFileName(Path.GetDirectoryName(Root)) == ".Trash" + ? Path.GetDirectoryName(Path.GetDirectoryName(Root)!)! + : Path.GetDirectoryName(Root)!); + public string FilesDirectory => Path.Combine(Root, FilesDirectoryName); public string InfoDirectory => Path.Combine(Root, InfoDirectoryName); diff --git a/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs b/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs index 0bf11028ccc9..a5489484af0b 100644 --- a/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs +++ b/tests/Files.Platform.Tests/Mime/ServiceMenuTests.cs @@ -28,21 +28,6 @@ private static string Text(string extra = "", string exec = "tool %F") => private static LinuxServiceMenuService Service(XdgFixture fx) => new(fx.Directories, new LinuxMimeTypeService(fx.Directories, CultureInfo.InvariantCulture), CultureInfo.GetCultureInfo("fr-FR")); - [TestMethod] - public void Expansion_RejectsResolvedShellAliasesInWrappers() - { - var path = Path.Combine(Path.GetTempPath(), "shell-alias-" + Guid.NewGuid().ToString("N")); - File.CreateSymbolicLink(path, "/bin/sh"); - try - { - foreach (var exec in new[] { path + " -c code", "env VAR=bash " + path + " script", "env rbash -c code", "env tcsh script" }) - Assert.AreEqual(0, DesktopExecExpander.ExpandServiceMenu(new DesktopApplication("run", "Run", exec, "/menu.desktop"), ["/a"]).Count, exec); - foreach (var exec in new[] { "env VAR=bash tool %f", "bash --version", "tool VAR=bash %f" }) - Assert.AreEqual(1, DesktopExecExpander.ExpandServiceMenu(new DesktopApplication("run", "Run", exec, "/menu.desktop"), ["/a"]).Count, exec); - } - finally { File.Delete(path); } - } - [TestMethod] public async Task Cache_ReusesParsingButRefiltersAndInvalidatesChanges() { diff --git a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs index 3d1c1bd49776..7b01db8a923a 100644 --- a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs +++ b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs @@ -7,6 +7,7 @@ using Files.Platform.Abstractions.Trash; using Files.Platform.Linux.Native; using Files.Platform.Linux.Trash; +using System.Collections.Generic; using Microsoft.VisualStudio.TestTools.UnitTesting; using System; using System.IO; @@ -17,6 +18,70 @@ namespace Files.Platform.Tests.Trash { public sealed partial class LinuxTrashServiceTests { + [TestMethod] + [DataRow("home", "restore")] + [DataRow("data", "delete")] + [DataRow("mount", "empty")] + [DataRow("shared", "restore")] + public async Task Trash_SymlinkedAncestor_RemainsAccessible(string ancestor, string operation) + { + _service.Watcher.Dispose(); + if (ancestor is "home" or "data") + { + var path = ancestor == "home" ? _home : _dataHome; + Directory.CreateDirectory(path); + Directory.Move(path, path + ".real"); + Directory.CreateSymbolicLink(path, path + ".real"); + } + else + { + if (ancestor == "shared") + { + var shared = Path.Combine(_usb, ".Trash"); + Directory.CreateDirectory(shared); + File.SetUnixFileMode(shared, UnixFileMode.StickyBit | UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + } + _service = new LinuxTrashService(new LinuxTrashOptions + { + DataHome = _dataHome, + UserId = Uid, + MountResolver = new SymlinkAncestorMountResolver((_root, _root), (_home, ancestor == "home" ? _home + ".real" : _home), + (_usb, ancestor is "mount" or "shared" ? Path.Combine(Path.GetDirectoryName(_usb)! + ".real", "usb") : _usb)), + }); + var original = WriteFile(ancestor is "home" or "data" ? _home : _usb, "linked.txt"); + var result = Single(await _service.TrashAsync([original])); + Assert.IsTrue(result.Succeeded); + if (ancestor is "mount" or "shared") + { + var mountParent = Path.GetDirectoryName(_usb)!; + Directory.Move(mountParent, mountParent + ".real"); + Directory.CreateSymbolicLink(mountParent, mountParent + ".real"); + } + var item = (await _service.ListAsync()).Single(); + if (operation == "restore") + { + Assert.IsTrue(Single(await _service.RestoreAsync([item])).Succeeded); + Assert.AreEqual("hello", File.ReadAllText(original)); + } + else if (operation == "delete") + Assert.IsTrue(Single(await _service.DeletePermanentlyAsync([item])).Succeeded); + else + await _service.EmptyAsync(); + Assert.IsEmpty(await _service.ListAsync()); + Assert.IsFalse(File.Exists(item.TrashedPath)); + Assert.IsFalse(File.Exists(item.TrashId)); + } + + private sealed class SymlinkAncestorMountResolver(params (string Path, string Resolved)[] mountPoints) : IMountResolver + { + public IReadOnlyList GetMounts() => mountPoints.Select(m => new MountEntry(m.Path, "ext4")).ToList(); + + public string GetMountPoint(string realPath) => mountPoints + .Where(m => MountInfoMountResolver.IsUnder(realPath, Directory.Exists(m.Resolved) ? m.Resolved : m.Path)) + .OrderByDescending(m => m.Resolved.Length).Select(m => m.Path).FirstOrDefault() ?? "/"; + } + [TestMethod] [DataRow("files")] [DataRow("info")] From 9f61e2488111ad89ca3a18c56e519e8c180c6bd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kov=C3=A1cs=20B=C3=A1lint=20Hunor?= Date: Tue, 13 Oct 2026 00:55:19 +0300 Subject: [PATCH 4/4] Tolerate missing info directory when deleting from trash Co-Authored-By: Codex Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016FAtbHHLQSiJiYcw2vnKiq --- .../Trash/LinuxTrashService.cs | 10 ++++- .../Trash/LinuxTrashHardeningTests.cs | 45 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs index 564385a7ed91..584d0c48af14 100644 --- a/src/Files.Platform.Linux/Trash/LinuxTrashService.cs +++ b/src/Files.Platform.Linux/Trash/LinuxTrashService.cs @@ -555,13 +555,13 @@ private TrashOperationResult DeleteOne(TrashItem item) using var root = OpenTrashRoot(location); using var files = OpenTrustedChild(root.Descriptor, FilesDirectoryName, location.FilesDirectory); - using var info = OpenTrustedChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); + using var info = OpenOptionalTrustedChild(root.Descriptor, InfoDirectoryName, location.InfoDirectory); if (files.EntryExists(name)) { _options.FileOperationsHooks?.BeforeDeleteEntry?.Invoke(item.TrashedPath); files.DeleteEntry(name); } - if (info.EntryExists(name + TrashInfoFile.Extension)) + if (info is not null && info.EntryExists(name + TrashInfoFile.Extension)) info.DeleteEntry(name + TrashInfoFile.Extension); new DirectorySizesCache(location.Root, root).Remove([name]); @@ -925,6 +925,12 @@ private DirectoryHandle OpenTrustedChild(int descriptor, string name, string pat catch { directory.Dispose(); throw; } } + private DirectoryHandle? OpenOptionalTrustedChild(int descriptor, string name, string path) + { + try { return OpenTrustedChild(descriptor, name, path); } + catch (FileNotFoundException) { return null; } + } + private void ValidateTrustedDirectory(DirectoryHandle directory) { if (!PosixNative.TryStat(directory.Descriptor, out var stat) || !stat.IsDirectory || diff --git a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs index 7b01db8a923a..ab8ffb4c2871 100644 --- a/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs +++ b/tests/Files.Platform.Tests/Trash/LinuxTrashHardeningTests.cs @@ -93,6 +93,51 @@ public async Task Empty_PartialLayout_DeletesExistingChild(string child) Assert.IsEmpty(Directory.GetFileSystemEntries(Path.Combine(HomeTrash, child))); } + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task DeletePermanently_MissingInfoDirectory_DeletesPayload(bool directory) + { + var original = directory ? Path.Combine(_home, "folder") : WriteFile(_home, "entry"); + if (directory) + WriteFile(original, "child"); + Assert.IsTrue(Single(await _service.TrashAsync([original])).Succeeded); + var item = (await _service.ListAsync()).Single(); + Directory.Delete(Path.Combine(HomeTrash, "info"), true); + + Assert.IsTrue(Single(await _service.DeletePermanentlyAsync([item])).Succeeded); + Assert.IsFalse(Path.Exists(item.TrashedPath)); + Assert.IsFalse(Directory.Exists(Path.Combine(HomeTrash, "info"))); + } + + [TestMethod] + [DataRow("symlink")] + [DataRow("dangling-symlink")] + [DataRow("writable")] + [DataRow("file")] + public async Task DeletePermanently_UntrustedInfoDirectory_PreservesPayload(string replacement) + { + Assert.IsTrue(Single(await _service.TrashAsync([WriteFile(_home, "entry")])).Succeeded); + var item = (await _service.ListAsync()).Single(); + var info = Path.Combine(HomeTrash, "info"); + var outside = Path.Combine(_root, "outside"); + var metadata = WriteFile(outside, Path.GetFileName(item.TrashId), "untouched"); + Directory.Delete(info, true); + if (replacement is "symlink" or "dangling-symlink") + Directory.CreateSymbolicLink(info, replacement == "symlink" ? outside : outside + "-missing"); + else if (replacement == "file") + File.WriteAllText(info, "untouched"); + else + { + Directory.CreateDirectory(info); + File.SetUnixFileMode(info, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute | UnixFileMode.OtherWrite); + } + + Assert.IsFalse(Single(await _service.DeletePermanentlyAsync([item])).Succeeded); + Assert.AreEqual("hello", File.ReadAllText(item.TrashedPath)); + Assert.AreEqual("untouched", File.ReadAllText(metadata)); + } + [TestMethod] [DataRow(false)] [DataRow(true)]